Page MenuHomePhabricator

Policy - fix up DifferentialChangesetParser
ClosedPublic

Authored by btrahan on Jan 30 2015, 7:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 14, 2:10 AM
Unknown Object (File)
Tue, Dec 10, 2:54 PM
Unknown Object (File)
Sun, Dec 8, 12:45 AM
Unknown Object (File)
Sat, Nov 30, 4:32 AM
Unknown Object (File)
Wed, Nov 27, 4:12 PM
Unknown Object (File)
Tue, Nov 26, 11:59 PM
Unknown Object (File)
Tue, Nov 26, 11:59 PM
Unknown Object (File)
Tue, Nov 26, 11:59 PM
Subscribers
Tokens
"Doubloon" token, awarded by epriestley.

Details

Reviewers
epriestley
Maniphest Tasks
T7094: Clean up T603
Commits
Restricted Diffusion Commit
rP77eae81e1a5c: Policy - fix up DifferentialChangesetParser
Summary

Ref T7094. We should do a policy query on the files IMO because there exists a scenario where the file gets locked down directly. This requires being a bit more disciplined about setting user, which in turn requires deciding whether or not to show edit / reply links as a separate piece of logic, not conditional on user presence.

This is not the best code but I don't think it gets worse with this and is just some other nuance in any larger cleanup we take on someday.

Test Plan

looked at a revision and noted inline comments rendered correctly with reply / edit actions. looked at a diff standalone and noted no reply / edit actions as expected. looked at a "details" link on a transaction and it rendered correctly. looked at a diff in phriction of page edits and it looked good. grepped around and verified the remaining callsite in diffusion already has the setUser call.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Policy - fix up DifferentialChangesetParser.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.

Nice. Consider setViewer() over setUser() for greater consistency with modern classes.

This revision is now accepted and ready to land.Jan 30 2015, 7:07 PM
This revision was automatically updated to reflect the committed changes.