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
Branch
interact1
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 22915
Build 31438: Run Core Tests
Build 31437: arc lint + arc unit

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.