Page MenuHomePhabricator

Make policy violation dialog more flexible
ClosedPublic

Authored by epriestley on Jun 4 2015, 7:49 PM.

Details

Summary

Ref T8424. When users are rejected because they can't see the space an object is in, this isn't really a capability rejection. Don't require a capability when rejecting objects.

This mostly simplifies upcoming changes.

Test Plan
  • Viewed a capability exception dialog, it looked the same as always.
  • (After additional changes, viewed a space exception dialog.)

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Make policy violation dialog more flexible.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
btrahan added inline comments.
src/applications/policy/filter/PhabricatorPolicyFilter.php
656

I don't think this really matters / I assume what's actually getting passed in here makes sense, but when I was wondering about correctness I looked up PhabricatorPolicyInterface and it doesn't have getPHID. Ergo / I guess at this anyway, isn't a method_exists sort of check maybe? Part of this is I think getPHID() usually returns something unless the object hasn't been saved yet.

This revision is now accepted and ready to land.Jun 4 2015, 9:50 PM
src/applications/policy/filter/PhabricatorPolicyFilter.php
656

PhabricatorPolicyInterface extends PhabricatorPHIDInterface which does guarantee that getPHID() exists, so this shouldn't ever fatal, at least.

However, this is probably at least somewhat wrong/sketchy/out-of-date; I don't know which types of objects the comment was talking about offhand. Two cases I can think of:

  • New objects getting policy checked by TransactionEditor before they're saved.
  • PhabricatorPolicyTestObjects in unit tests.
This revision was automatically updated to reflect the committed changes.