Page MenuHomePhabricator

Audit - fix profile link
ClosedPublic

Authored by btrahan on May 2 2014, 3:31 PM.
Tags
None
Referenced Files
F14044580: D8951.id.diff
Tue, Nov 12, 6:07 PM
F14042045: D8951.diff
Mon, Nov 11, 11:54 PM
F14034794: D8951.id21236.diff
Sun, Nov 10, 2:11 AM
F14034594: D8951.id21236.diff
Sun, Nov 10, 1:11 AM
F14026847: D8951.id21240.diff
Fri, Nov 8, 2:41 AM
F14025999: D8951.diff
Thu, Nov 7, 9:41 PM
F14008268: D8951.diff
Tue, Oct 29, 6:52 PM
F14007952: D8951.id21237.diff
Tue, Oct 29, 1:25 PM
Subscribers

Details

Summary

forgot to update this with new application search.

Test Plan

verified "View Commits" took me to my commits and the commits of another user from respective profile pages.

Diff Detail

Repository
rP Phabricator
Branch
commitlink
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 197
Build 197: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

btrahan retitled this revision from to Audit - fix profile link.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.
This revision is now accepted and ready to land.May 2 2014, 3:43 PM
src/applications/audit/events/AuditActionMenuEventListener.php
39

Oh, this should be unnecessary I think. Or we need to swap a thing in the SearchEngine. If it is necessary, I'll counter-diff you.

src/applications/audit/events/AuditActionMenuEventListener.php
39

it works without this; I was copying Differential basically because WITHOUT this you get the URI as written in the code and WITH it you get a "pretty" query URI.

I think the cleanest fix is:

  • in the Engine, call the parameter authors instead of commitAuthorPHIDs;
  • pass the readable form here: '/audit/?authors='.$user->getUsername()

This is clean and hints to users that they can construct their own similar query in a sensible way (authors=username).

That is, call the HTTP parameter authors. The storage parameter can/should remain commitAuthorPHIDs. For example:

$saved->setParameter(
  'commitAuthorPHIDs',
  $this->readUsersFromRequest($request, 'authors'));

The narrative here is something like "authors is in a human-preferred format, commitAuthorPHIDs is in a normalized computer-preferred format".

btrahan edited edge metadata.

make this all prettier as suggested

WMF also has some issue with their markup where [] is unreasonably difficult to type in URIs:

http://fab.wmflabs.org/T88

This isn't really a problem on our side, but since the authors=epriestley form is better for humans in general we avoid that issue for free.

Cool, looks great.

src/applications/audit/query/PhabricatorCommitSearchEngine.php
91

This probably needs to be 'authors'.

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

...even prettier (authors[] => authors)

change commitAuthorPHIDs to authors in form UI

(It worked before this change, but this is more clear. Also maybe there's some caching or something that made it fake-work with my smallish dataset...)

btrahan updated this revision to Diff 21240.

Closed by commit rP97f88f468b31 (authored by @btrahan).