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.

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.May 28 2019, 5:14 PM
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.