Projects - smooth out scenarios around renaming a project and slugs
ClosedPublic
Actions

Authored by btrahan on Feb 5 2015, 7:31 PM.

Details

Reviewers

Maniphest Tasks

T7092: When a project is renamed, the old hash should be added as an alternate hash
T6909: Unable to rename project and add former primary hashtag as additional hashtag

Commits

Restricted Diffusion Commit
rPac504f232fed: Projects - smooth out scenarios around renaming a project and slugs

Summary

Fixes T7092. When you name project "Foo" which has primary hashtag "foo" to "Foobar", post this patch the hashtag "foo" gets added as a secondary hashtag. Also makes sure we don't normalize the hashtags in the query function as the wikimedia folks were hitting an issue around capitalization on the hashtag.

Note that T6909 remains "broken" in that you get an error that you can't do that, though if you just omit the additional hashtag it would work fine. I think if a fix is necessary here the best bet would be to simply detect this particular scenario and let things proceed; its a bit tricky though since its about two transactions about to be applied and how they interact with one another...

Test Plan

Made project "Foo" which has primary hashtag "foo". Renamed it to "Foobar" and verified "foo" was added as a secondary hashtag and "foobar" was the primary hashtag. Renamed it again to "Foo" and noted that the hashtags all ended up correct.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

btrahan updated this revision to Diff 28144.Feb 5 2015, 7:31 PM

btrahan added tasks: T6909: Unable to rename project and add former primary hashtag as additional hashtag, T7092: When a project is renamed, the old hash should be added as an alternate hash.

btrahan retitled this revision from to Projects - smooth out scenarios around renaming a project and slugs.

btrahan updated this object.

btrahan edited the test plan for this revision. (Show Details)

btrahan added a reviewer: epriestley.

Herald added subscribers: epriestley, Korvin. · View Herald TranscriptFeb 5 2015, 7:31 PM

btrahan updated this object.Feb 5 2015, 7:32 PM

Slug add/remove stuff looks good, less sure about normalization removal. Leaving T6909 "broken" seems fine to me.

src/applications/project/query/PhabricatorProjectQuery.php
281–284	I'd imagine that this might make `/tag/Phabricator/` stop working for `/tag/phabricator/`, etc., but it seems like it should work. Does it still work / was there more detail on the issue that this fixes? I couldn't find anything obvious in the linked tasks. Particularly, right now, `/tag/QuAcK/` will correctly redirect to `/tag/quack/`, and all of `#quack`, `#QUACK`, etc., will work in remarkup -- do they after this patch?

epriestley accepted this revision.Feb 6 2015, 11:49 PM

epriestley edited edge metadata.

This revision is now accepted and ready to land.Feb 6 2015, 11:49 PM

btrahan added inline comments.Feb 7 2015, 12:35 AM

src/applications/project/query/PhabricatorProjectQuery.php
281–284	I see different behavior right now... https://secure.phabricator.com/project/details/2/ (has phabricatorcli, PHABCLI for tags..) WORKS - https://secure.phabricator.com/tag/phabricator-cli/ FAILS - https://secure.phabricator.com/tag/PHABCLI/ - note no redirect or anything ...so what this patch does is make the second bit work. (On one of those issues wikimedia reported secondary hashtags not working and I narrowed it down to this case issue...)

Hrrm, alright. QUACK goes to quack for me locally, but maybe I did something weird. As long as it's working, I think we're good. If we figure out something's broken we can add some unit tests when we fix it.