Move the hierarchical edit policy check in Phriction from requireCapabilities() to validateTransactions()

Summary:
Depends on D19583. Ref T13164. This continues the work of getting rid of `requireCapabilities()`.

This check is valid, but can be a `validateTransactions()` check instead. This is generally more consistent with how other applications work (e.g., creating subprojects).

The UI for this isn't terribly great: you get a policy error //after// you try to create the object. But that's how it worked before, so this isn't any worse than it was. The actual policy exception is (very) slightly more clear now (raised against the right object).

Test Plan:
- Created a child as a user with permission to do so to make sure I didn't break that.
- Set edit permission on `a/` to just me, tried to create `a/b/` as another user, got a policy exception since they can't edit the parent.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

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

epriestley 7 years ago 3b92da22 24d44458

+23 -52

2 changed files

expand all

src

applications

phriction

editor

PhrictionTransactionEditor.php

xaction

PhrictionDocumentTitleTransaction.php

-52

src/applications/phriction/editor/PhrictionTransactionEditor.php

··· 516 516 } 517 517 return $error; 518 518 } 519 - protected function requireCapabilities( 520 - PhabricatorLiskDAO $object, 521 - PhabricatorApplicationTransaction $xaction) { 522 - 523 - /* 524 - * New objects have a special case. If a user can't see 525 - * x/y 526 - * then definitely don't let them make some 527 - * x/y/z 528 - * We need to load the direct parent to handle this case. 529 - */ 530 - if ($this->getIsNewObject()) { 531 - $actor = $this->requireActor(); 532 - $parent_doc = null; 533 - $ancestral_slugs = PhabricatorSlug::getAncestry($object->getSlug()); 534 - // No ancestral slugs is "/"; the first person gets to play with "/". 535 - if ($ancestral_slugs) { 536 - $parent = end($ancestral_slugs); 537 - $parent_doc = id(new PhrictionDocumentQuery()) 538 - ->setViewer($actor) 539 - ->withSlugs(array($parent)) 540 - ->executeOne(); 541 - // If the $actor can't see the $parent_doc then they can't create 542 - // the child $object; throw a policy exception. 543 - if (!$parent_doc) { 544 - id(new PhabricatorPolicyFilter()) 545 - ->setViewer($actor) 546 - ->raisePolicyExceptions(true) 547 - ->rejectObject( 548 - $object, 549 - $object->getEditPolicy(), 550 - PhabricatorPolicyCapability::CAN_EDIT); 551 - } 552 - 553 - // If the $actor can't edit the $parent_doc then they can't create 554 - // the child $object; throw a policy exception. 555 - if (!PhabricatorPolicyFilter::hasCapability( 556 - $actor, 557 - $parent_doc, 558 - PhabricatorPolicyCapability::CAN_EDIT)) { 559 - id(new PhabricatorPolicyFilter()) 560 - ->setViewer($actor) 561 - ->raisePolicyExceptions(true) 562 - ->rejectObject( 563 - $object, 564 - $object->getEditPolicy(), 565 - PhabricatorPolicyCapability::CAN_EDIT); 566 - } 567 - } 568 - } 569 - return parent::requireCapabilities($object, $xaction); 570 - } 571 519 572 520 protected function supportsSearch() { 573 521 return true;

+23

src/applications/phriction/xaction/PhrictionDocumentTitleTransaction.php

··· 91 91 pht('Documents must have a title.')); 92 92 } 93 93 94 + if ($this->isNewObject()) { 95 + // No ancestral slugs is "/". No ancestry checks apply when creating the 96 + // root document. 97 + $ancestral_slugs = PhabricatorSlug::getAncestry($object->getSlug()); 98 + if ($ancestral_slugs) { 99 + // You must be able to view and edit the parent document to create a new 100 + // child. 101 + $parent_document = id(new PhrictionDocumentQuery()) 102 + ->setViewer($this->getActor()) 103 + ->withSlugs(array(last($ancestral_slugs))) 104 + ->requireCapabilities( 105 + array( 106 + PhabricatorPolicyCapability::CAN_VIEW, 107 + PhabricatorPolicyCapability::CAN_EDIT, 108 + )) 109 + ->executeOne(); 110 + if (!$parent_document) { 111 + $errors[] = $this->newInvalidError( 112 + pht('You can not create a document which does not have a parent.')); 113 + } 114 + } 115 + } 116 + 94 117 return $errors; 95 118 } 96 119

Configure Feed

Configure Feed