mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-02 02:40:58 +01:00
Call didRejectResult() in DiffusionCommitQuery properly
Summary: Ref T4345. This error is per object-type in the query implementations, not a mail/permissions issue. Without `didRejectResult()`, we can't distinguish between "restricted" and "unknown" for objects filtered by `willFilterPage()`. - Call `didRejectResult()` on commits. - Make `didRejectResult()` handle both existing policy exceptions and filtering. - Recover from partial objects (like commits) which are missing attached data required to figure out policies. Test Plan: Saw "Restricted Diffusion Commit" instead of "Unknown Object (Diffusion Commit)" when viewing nonvisible commit handle in Maniphest. Reviewers: btrahan, joshuaspence Reviewed By: joshuaspence Subscribers: epriestley Maniphest Tasks: T4345 Differential Revision: https://secure.phabricator.com/D13289
This commit is contained in:
parent
175799fab9
commit
2fbc65e396
2 changed files with 18 additions and 1 deletions
|
@ -193,6 +193,7 @@ final class DiffusionCommitQuery
|
||||||
if ($repo) {
|
if ($repo) {
|
||||||
$commit->attachRepository($repo);
|
$commit->attachRepository($repo);
|
||||||
} else {
|
} else {
|
||||||
|
$this->didRejectResult($commit);
|
||||||
unset($commits[$key]);
|
unset($commits[$key]);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
|
@ -338,9 +338,25 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function didRejectResult(PhabricatorPolicyInterface $object) {
|
protected function didRejectResult(PhabricatorPolicyInterface $object) {
|
||||||
|
// Some objects (like commits) may be rejected because related objects
|
||||||
|
// (like repositories) can not be loaded. In some cases, we may need these
|
||||||
|
// related objects to determine the object policy, so it's expected that
|
||||||
|
// we may occasionally be unable to determine the policy.
|
||||||
|
|
||||||
|
try {
|
||||||
|
$policy = $object->getPolicy(PhabricatorPolicyCapability::CAN_VIEW);
|
||||||
|
} catch (Exception $ex) {
|
||||||
|
$policy = null;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Mark this object as filtered so handles can render "Restricted" instead
|
||||||
|
// of "Unknown".
|
||||||
|
$phid = $object->getPHID();
|
||||||
|
$this->addPolicyFilteredPHIDs(array($phid => $phid));
|
||||||
|
|
||||||
$this->getPolicyFilter()->rejectObject(
|
$this->getPolicyFilter()->rejectObject(
|
||||||
$object,
|
$object,
|
||||||
$object->getPolicy(PhabricatorPolicyCapability::CAN_VIEW),
|
$policy,
|
||||||
PhabricatorPolicyCapability::CAN_VIEW);
|
PhabricatorPolicyCapability::CAN_VIEW);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue