From c8d20d3c66f2dbf243aed4fdbbffdf126d36ff75 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 7 Nov 2014 15:36:58 -0800 Subject: [PATCH] Phriction - policy front end changes Summary: Ref T4029. Fixes T6034. Various front-end miscellania here. See D10814#96251. This more or less makes policy work but I am not going to call it "fixed" here since we need D10814 to be deployed too and will do that manually. Test Plan: - changed document policy from web ui and changes persisted - changed document policy from web and had form error and changes persisted - created a structure like users/users/justmyuserpolicy and made sure another user could delete the users/users/ doc - moved a doc from a to b and verified policy persisted - verified stub documents inherited policy of the document that stub them...! - uploaded a file and verified that it 1) had the permissions of the page it was added to and 2) had an "attached" tab linking back to the page on the file page (this means T6034 is fixed with this) Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T6034, T4029 Differential Revision: https://secure.phabricator.com/D10816 --- src/__phutil_library_map__.php | 2 - .../PhabricatorPhrictionApplication.php | 6 --- .../controller/PhrictionDiffController.php | 4 ++ .../PhrictionDocumentController.php | 4 ++ .../controller/PhrictionEditController.php | 34 ++++++++++++++- .../controller/PhrictionHistoryController.php | 4 ++ .../editor/PhrictionTransactionEditor.php | 16 +++++++ .../PhrictionActionMenuEventListener.php | 43 ------------------- .../phriction/storage/PhrictionDocument.php | 26 ++++++++--- 9 files changed, 80 insertions(+), 59 deletions(-) delete mode 100644 src/applications/phriction/event/PhrictionActionMenuEventListener.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 79ca74ca44..5f45f74d94 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2756,7 +2756,6 @@ phutil_register_library_map(array( 'PhrequentUserTime' => 'applications/phrequent/storage/PhrequentUserTime.php', 'PhrequentUserTimeQuery' => 'applications/phrequent/query/PhrequentUserTimeQuery.php', 'PhrictionActionConstants' => 'applications/phriction/constants/PhrictionActionConstants.php', - 'PhrictionActionMenuEventListener' => 'applications/phriction/event/PhrictionActionMenuEventListener.php', 'PhrictionChangeType' => 'applications/phriction/constants/PhrictionChangeType.php', 'PhrictionConduitAPIMethod' => 'applications/phriction/conduit/PhrictionConduitAPIMethod.php', 'PhrictionConstants' => 'applications/phriction/constants/PhrictionConstants.php', @@ -5959,7 +5958,6 @@ phutil_register_library_map(array( ), 'PhrequentUserTimeQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhrictionActionConstants' => 'PhrictionConstants', - 'PhrictionActionMenuEventListener' => 'PhabricatorEventListener', 'PhrictionChangeType' => 'PhrictionConstants', 'PhrictionConduitAPIMethod' => 'ConduitAPIMethod', 'PhrictionContent' => array( diff --git a/src/applications/phriction/application/PhabricatorPhrictionApplication.php b/src/applications/phriction/application/PhabricatorPhrictionApplication.php index 1f063857d2..b2cc472341 100644 --- a/src/applications/phriction/application/PhabricatorPhrictionApplication.php +++ b/src/applications/phriction/application/PhabricatorPhrictionApplication.php @@ -36,12 +36,6 @@ final class PhabricatorPhrictionApplication extends PhabricatorApplication { ); } - public function getEventListeners() { - return array( - new PhrictionActionMenuEventListener(), - ); - } - public function getRoutes() { return array( // Match "/w/" with slug "/". diff --git a/src/applications/phriction/controller/PhrictionDiffController.php b/src/applications/phriction/controller/PhrictionDiffController.php index fc2f2a6f33..41f515dfbb 100644 --- a/src/applications/phriction/controller/PhrictionDiffController.php +++ b/src/applications/phriction/controller/PhrictionDiffController.php @@ -4,6 +4,10 @@ final class PhrictionDiffController extends PhrictionController { private $id; + public function shouldAllowPublic() { + return true; + } + public function willProcessRequest(array $data) { $this->id = $data['id']; } diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php index f9dec06d03..2f1cc2f6e5 100644 --- a/src/applications/phriction/controller/PhrictionDocumentController.php +++ b/src/applications/phriction/controller/PhrictionDocumentController.php @@ -5,6 +5,10 @@ final class PhrictionDocumentController private $slug; + public function shouldAllowPublic() { + return true; + } + public function willProcessRequest(array $data) { $this->slug = $data['slug']; } diff --git a/src/applications/phriction/controller/PhrictionEditController.php b/src/applications/phriction/controller/PhrictionEditController.php index e85b7bc788..3892ab9578 100644 --- a/src/applications/phriction/controller/PhrictionEditController.php +++ b/src/applications/phriction/controller/PhrictionEditController.php @@ -131,6 +131,8 @@ final class PhrictionEditController $content_text = $request->getStr('content'); $notes = $request->getStr('description'); $current_version = $request->getInt('contentVersion'); + $v_view = $request->getStr('viewPolicy'); + $v_edit = $request->getStr('editPolicy'); $xactions = array(); $xactions[] = id(new PhrictionTransaction()) @@ -139,6 +141,12 @@ final class PhrictionEditController $xactions[] = id(new PhrictionTransaction()) ->setTransactionType(PhrictionTransaction::TYPE_CONTENT) ->setNewValue($content_text); + $xactions[] = id(new PhrictionTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) + ->setNewValue($v_view); + $xactions[] = id(new PhrictionTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY) + ->setNewValue($v_edit); $editor = id(new PhrictionTransactionEditor()) ->setActor($user) @@ -170,7 +178,8 @@ final class PhrictionEditController $overwrite = true; } - // TODO - remember to set policy to what the user tried to set it to + $document->setViewPolicy($v_view); + $document->setEditPolicy($v_edit); } } @@ -194,6 +203,13 @@ final class PhrictionEditController $cancel_uri = PhrictionDocument::getSlugURI($document->getSlug()); + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($user) + ->setObject($document) + ->execute(); + $view_capability = PhabricatorPolicyCapability::CAN_VIEW; + $edit_capability = PhabricatorPolicyCapability::CAN_EDIT; + $form = id(new AphrontFormView()) ->setUser($user) ->addHiddenInput('slug', $document->getSlug()) @@ -219,6 +235,22 @@ final class PhrictionEditController ->setName('content') ->setID('document-textarea') ->setUser($user)) + ->appendChild( + id(new AphrontFormPolicyControl()) + ->setName('viewPolicy') + ->setPolicyObject($document) + ->setCapability($view_capability) + ->setPolicies($policies) + ->setCaption( + $document->describeAutomaticCapability($view_capability))) + ->appendChild( + id(new AphrontFormPolicyControl()) + ->setName('editPolicy') + ->setPolicyObject($document) + ->setCapability($edit_capability) + ->setPolicies($policies) + ->setCaption( + $document->describeAutomaticCapability($edit_capability))) ->appendChild( id(new AphrontFormTextControl()) ->setLabel(pht('Edit Notes')) diff --git a/src/applications/phriction/controller/PhrictionHistoryController.php b/src/applications/phriction/controller/PhrictionHistoryController.php index 41b827b954..e9daa2be41 100644 --- a/src/applications/phriction/controller/PhrictionHistoryController.php +++ b/src/applications/phriction/controller/PhrictionHistoryController.php @@ -5,6 +5,10 @@ final class PhrictionHistoryController private $slug; + public function shouldAllowPublic() { + return true; + } + public function willProcessRequest(array $data) { $this->slug = $data['slug']; } diff --git a/src/applications/phriction/editor/PhrictionTransactionEditor.php b/src/applications/phriction/editor/PhrictionTransactionEditor.php index bc838c8f03..af40cf117e 100644 --- a/src/applications/phriction/editor/PhrictionTransactionEditor.php +++ b/src/applications/phriction/editor/PhrictionTransactionEditor.php @@ -200,8 +200,18 @@ final class PhrictionTransactionEditor ->setNewValue(true); } break; + case PhrictionTransaction::TYPE_MOVE_TO: + $document = $xaction->getNewValue(); + $xactions[] = id(new PhrictionTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) + ->setNewValue($document->getViewPolicy()); + $xactions[] = id(new PhrictionTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY) + ->setNewValue($document->getEditPolicy()); + break; default: break; + } return $xactions; @@ -298,6 +308,12 @@ final class PhrictionTransactionEditor ->setTransactionType(PhrictionTransaction::TYPE_CONTENT) ->setNewValue('') ->setMetadataValue('stub:create:phid', $object->getPHID()); + $stub_xactions[] = id(new PhrictionTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) + ->setNewValue($object->getViewPolicy()); + $stub_xactions[] = id(new PhrictionTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY) + ->setNewValue($object->getEditPolicy()); $sub_editor = id(new PhrictionTransactionEditor()) ->setActor($this->getActor()) ->setContentSource($this->getContentSource()) diff --git a/src/applications/phriction/event/PhrictionActionMenuEventListener.php b/src/applications/phriction/event/PhrictionActionMenuEventListener.php deleted file mode 100644 index c83f2e4247..0000000000 --- a/src/applications/phriction/event/PhrictionActionMenuEventListener.php +++ /dev/null @@ -1,43 +0,0 @@ -listen(PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS); - } - - public function handleEvent(PhutilEvent $event) { - switch ($event->getType()) { - case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS: - $this->handleActionsEvent($event); - break; - } - } - - private function handleActionsEvent(PhutilEvent $event) { - $object = $event->getValue('object'); - - $actions = null; - if ($object instanceof PhabricatorProject) { - $actions = $this->buildProjectActions($event); - } - - $this->addActionMenuItems($event, $actions); - } - - private function buildProjectActions(PhutilEvent $event) { - if (!$this->canUseApplication($event->getUser())) { - return null; - } - - $project = $event->getValue('object'); - $slug = PhabricatorSlug::normalize($project->getPhrictionSlug()); - $href = '/w/projects/'.$slug; - - return id(new PhabricatorActionView()) - ->setIcon('fa-book') - ->setName(pht('View Wiki')) - ->setHref($href); - } - -} diff --git a/src/applications/phriction/storage/PhrictionDocument.php b/src/applications/phriction/storage/PhrictionDocument.php index 3a4a80bfe4..070b3106ce 100644 --- a/src/applications/phriction/storage/PhrictionDocument.php +++ b/src/applications/phriction/storage/PhrictionDocument.php @@ -18,10 +18,7 @@ final class PhrictionDocument extends PhrictionDAO private $contentObject = self::ATTACHABLE; private $ancestors = array(); - - // TODO: This should be `self::ATTACHABLE`, but there are still a lot of call - // sites which load PhrictionDocuments directly. - private $project = null; + private $project = self::ATTACHABLE; public function getConfiguration() { return array( @@ -68,9 +65,24 @@ final class PhrictionDocument extends PhrictionDAO $content->setTitle($default_title); $document->attachContent($content); - $default_view_policy = PhabricatorPolicies::getMostOpenPolicy(); - $document->setViewPolicy($default_view_policy); - $document->setEditPolicy(PhabricatorPolicies::POLICY_USER); + $parent_doc = null; + $ancestral_slugs = PhabricatorSlug::getAncestry($slug); + if ($ancestral_slugs) { + $parent = end($ancestral_slugs); + $parent_doc = id(new PhrictionDocumentQuery()) + ->setViewer($actor) + ->withSlugs(array($parent)) + ->executeOne(); + } + + if ($parent_doc) { + $document->setViewPolicy($parent_doc->getViewPolicy()); + $document->setEditPolicy($parent_doc->getEditPolicy()); + } else { + $default_view_policy = PhabricatorPolicies::getMostOpenPolicy(); + $document->setViewPolicy($default_view_policy); + $document->setEditPolicy(PhabricatorPolicies::POLICY_USER); + } return $document; }