Page MenuHomePhabricator

Remove some defunct old-style transaction policy checks
ClosedPublic

Authored by epriestley on Nov 6 2017, 7:07 PM.
Tags
None
Referenced Files
F15547286: D18763.id45028.diff
Sat, Apr 26, 6:32 PM
F15546748: D18763.id45028.diff
Sat, Apr 26, 4:46 PM
F15545112: D18763.id45028.diff
Sat, Apr 26, 9:00 AM
F15521876: D18763.id45034.diff
Sun, Apr 20, 4:07 PM
F15521284: D18763.id45028.diff
Sun, Apr 20, 12:48 PM
F15512317: D18763.id.diff
Thu, Apr 17, 9:48 AM
F15508808: D18763.diff
Wed, Apr 16, 6:40 AM
F15504256: D18763.id.diff
Mon, Apr 14, 5:01 PM
Subscribers
None

Details

Summary

Ref PHI193. This method of enforcing policy checks is now (mostly) obsolete, and they're generally checked at the Controller/API level instead.

Notably, this method does not call adjustObjectForPolicyChecks(...) properly, so it can not handle special cases like "creating a project and taking its newly created members into account" for object policies like "Project Members".

Just remove these checks, which are redundant with checks elsewhere.

Test Plan
  • Set Project application default edit policy to "Administrators and Project Members".
  • Tried to create a project as a non-administrator, adding myself.
  • Before patch: policy fatal on a VOID object (the project with no PHID generated yet).
  • After patch: object created properly. Got a sensible policy error if I didn't include myself as a member.
  • Also verified that other edit rules are still enforced/respected (I can't edit stuff I shouldn't be able to edit).
  • There's at least a bit of unit test coverage of this, too, which I updated to work via API (which hits the new broad capability checks) instead of via low-level transactions (which enforce only a subset of policy operations now).

Diff Detail

Repository
rP Phabricator
Branch
proj1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 18806
Build 25349: Run Core Tests
Build 25348: arc lint + arc unit