Page MenuHomePhabricator

Test for "CAN_INTERACT" on comment edits in a way that survives objects which only implement "CAN_VIEW"
ClosedPublicDraft

Authored by epriestley on May 28 2019, 5:14 PM.
Tags
None
Referenced Files
F12827578: D20558.id49042.diff
Thu, Mar 28, 10:10 AM
Unknown Object (File)
Sat, Mar 23, 7:21 AM
Unknown Object (File)
Sat, Mar 23, 7:21 AM
Unknown Object (File)
Sat, Mar 23, 7:21 AM
Unknown Object (File)
Sat, Mar 23, 7:21 AM
Unknown Object (File)
Sat, Mar 16, 11:18 AM
Unknown Object (File)
Mon, Mar 11, 2:14 AM
Unknown Object (File)
Fri, Mar 1, 5:31 AM
Subscribers
None
Tokens
"Manufacturing Defect?" token, awarded by amckinley.

Details

Summary

Ref T13289. See D20551. In D20551, I implemented some "CAN_INTERACT" checks against certain edits, but these checks end up testing "CAN_INTERACT" against objects like Conpherence threads which do not support a distinct "CAN_INTERACT" permission. I misrembered how the "CAN_INTERACT" fallback to "CAN_VIEW" actually works: it's not fully automatic, and needs some explicit "interact, or view if interact is not available" checks.

Use the "interact" wrappers to test these policies so they fall back to "CAN_VIEW" if an object does not support "CAN_INTERACT". Generally, objects which have a "locked" state have a separate "CAN_INTERACT" permission; objects which don't have a "locked" state do not.

Test Plan

Created and edited comments in Conpherence (or most applications other than Maniphest).

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision was not accepted when it landed; it landed in state Draft.May 28 2019, 5:14 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

I just landed this since comments won't work without it, yell if it seems retroactively bad.

Oh, this is also interesting: this never undrafted. We should probably make "land" force things to undraft.