Page MenuHomePhabricator

Policy - fix up DifferentialChangesetParser
ClosedPublic

Authored by btrahan on Jan 30 2015, 7:05 PM.
Tags
None
Referenced Files
F14481637: D11579.diff
Mon, Dec 30, 4:33 AM
Unknown Object (File)
Wed, Dec 25, 3:58 PM
Unknown Object (File)
Tue, Dec 24, 7:52 PM
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)
Nov 30 2024, 4:32 AM
Unknown Object (File)
Nov 27 2024, 4:12 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
Branch
T7094_1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 4185
Build 4198: [Placeholder Plan] Wait for 30 Seconds

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.