recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Use extended policies in Differential diffs

Summary:
Fixes T9648. Diffs currently use `return $this->getRevision()->getViewPolicy();` to inherit their revision's view policy.

After the introduction of object policies, this is wrong for policies like "Subscribers", because it means "Subscribers to this object, the diff". Since Diffs have no subscribers, this always fails.

Instead, use extended policies so that the object policy evaluates in the context of the correct object (the revision).

Test Plan:
- Create a revision.
- Subscribe `alice` to it.
- Set view policy to "Subscribers".
- View revision as `alice`.
- Before patch: nonsense fatal about missing diff because of policy error.
- After patch: `alice` can see the revision.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9648

Differential Revision: https://secure.phabricator.com/D17123

epriestley 65c1c758 81e2a1cf

+27 -5
+1
src/__phutil_library_map__.php
··· 5058 5058 'DifferentialDiff' => array( 5059 5059 'DifferentialDAO', 5060 5060 'PhabricatorPolicyInterface', 5061 + 'PhabricatorExtendedPolicyInterface', 5061 5062 'HarbormasterBuildableInterface', 5062 5063 'HarbormasterCircleCIBuildableInterface', 5063 5064 'PhabricatorApplicationTransactionInterface',
+2 -3
src/applications/differential/controller/DifferentialRevisionViewController.php
··· 16 16 17 17 $revision = id(new DifferentialRevisionQuery()) 18 18 ->withIDs(array($this->revisionID)) 19 - ->setViewer($request->getUser()) 19 + ->setViewer($viewer) 20 20 ->needRelationships(true) 21 21 ->needReviewerStatus(true) 22 22 ->needReviewerAuthority(true) 23 23 ->executeOne(); 24 - 25 24 if (!$revision) { 26 25 return new Aphront404Response(); 27 26 } 28 27 29 28 $diffs = id(new DifferentialDiffQuery()) 30 - ->setViewer($request->getUser()) 29 + ->setViewer($viewer) 31 30 ->withRevisionIDs(array($this->revisionID)) 32 31 ->execute(); 33 32 $diffs = array_reverse($diffs, $preserve_keys = true);
+24 -2
src/applications/differential/storage/DifferentialDiff.php
··· 4 4 extends DifferentialDAO 5 5 implements 6 6 PhabricatorPolicyInterface, 7 + PhabricatorExtendedPolicyInterface, 7 8 HarbormasterBuildableInterface, 8 9 HarbormasterCircleCIBuildableInterface, 9 10 PhabricatorApplicationTransactionInterface, ··· 429 430 430 431 public function getPolicy($capability) { 431 432 if ($this->hasRevision()) { 432 - return $this->getRevision()->getPolicy($capability); 433 + return PhabricatorPolicies::getMostOpenPolicy(); 433 434 } 434 435 435 436 return $this->viewPolicy; ··· 440 441 return $this->getRevision()->hasAutomaticCapability($capability, $viewer); 441 442 } 442 443 443 - return ($this->getAuthorPHID() == $viewer->getPhid()); 444 + return ($this->getAuthorPHID() == $viewer->getPHID()); 444 445 } 445 446 446 447 public function describeAutomaticCapability($capability) { ··· 448 449 return pht( 449 450 'This diff is attached to a revision, and inherits its policies.'); 450 451 } 452 + 451 453 return pht('The author of a diff can see it.'); 452 454 } 453 455 456 + 457 + /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ 458 + 459 + 460 + public function getExtendedPolicy($capability, PhabricatorUser $viewer) { 461 + $extended = array(); 462 + 463 + switch ($capability) { 464 + case PhabricatorPolicyCapability::CAN_VIEW: 465 + if ($this->hasRevision()) { 466 + $extended[] = array( 467 + $this->getRevision(), 468 + PhabricatorPolicyCapability::CAN_VIEW, 469 + ); 470 + } 471 + break; 472 + } 473 + 474 + return $extended; 475 + } 454 476 455 477 456 478 /* -( HarbormasterBuildableInterface )------------------------------------- */