diff --git a/src/applications/paste/controller/PhabricatorPasteEditController.php b/src/applications/paste/controller/PhabricatorPasteEditController.php --- a/src/applications/paste/controller/PhabricatorPasteEditController.php +++ b/src/applications/paste/controller/PhabricatorPasteEditController.php @@ -167,12 +167,6 @@ ->setObject($paste) ->execute(); - $form->appendControl( - id(new PhabricatorSpacesControl()) - ->setObject($paste) - ->setValue($v_space) - ->setName('spacePHID')); - $form->appendChild( id(new AphrontFormPolicyControl()) ->setUser($user) @@ -180,6 +174,7 @@ ->setPolicyObject($paste) ->setPolicies($policies) ->setValue($v_view_policy) + ->setSpacePHID($v_space) ->setName('can_view')); $form->appendChild( diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -758,6 +758,7 @@ // TODO: We might let the user switch which space they're "in" later on; // for now just use the global space if one exists. + // If the viewer has access to the default space, use that. $spaces = PhabricatorSpacesNamespaceQuery::getViewerSpaces($this); foreach ($spaces as $space) { if ($space->getIsDefaultNamespace()) { @@ -765,6 +766,14 @@ } } + // Otherwise, use the space with the lowest ID that they have access to. + // This just tends to keep the default stable and predictable over time, + // so adding a new space won't change behavior for users. + if ($spaces) { + $spaces = msort($spaces, 'getID'); + return head($spaces)->getPHID(); + } + return null; } diff --git a/src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php b/src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php --- a/src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php +++ b/src/applications/spaces/query/PhabricatorSpacesNamespaceQuery.php @@ -90,6 +90,17 @@ return (bool)self::getAllSpaces(); } + public static function getViewerSpacesExist(PhabricatorUser $viewer) { + if (!self::getSpacesExist()) { + return false; + } + + // If the viewer has access to only one space, pretend spaces simply don't + // exist. + $spaces = self::getViewerSpaces($viewer); + return (count($spaces) > 1); + } + public static function getAllSpaces() { $cache = PhabricatorCaches::getRequestCache(); $cache_key = self::KEY_ALL; diff --git a/src/applications/spaces/view/PHUISpacesNamespaceContextView.php b/src/applications/spaces/view/PHUISpacesNamespaceContextView.php --- a/src/applications/spaces/view/PHUISpacesNamespaceContextView.php +++ b/src/applications/spaces/view/PHUISpacesNamespaceContextView.php @@ -21,7 +21,20 @@ return null; } + // If the viewer can't see spaces, pretend they don't exist. $viewer = $this->getUser(); + if (!PhabricatorSpacesNamespaceQuery::getViewerSpacesExist($viewer)) { + return null; + } + + // If this is the default space, don't show a space label. + $default = PhabricatorSpacesNamespaceQuery::getDefaultSpace(); + if ($default) { + if ($default->getPHID() == $space_phid) { + return null; + } + } + return phutil_tag( 'span', array( diff --git a/src/applications/spaces/view/PhabricatorSpacesControl.php b/src/applications/spaces/view/PhabricatorSpacesControl.php deleted file mode 100644 --- a/src/applications/spaces/view/PhabricatorSpacesControl.php +++ /dev/null @@ -1,49 +0,0 @@ -object = $object; - return $this; - } - - protected function getCustomControlClass() { - return ''; - } - - protected function getOptions() { - $viewer = $this->getUser(); - $viewer_spaces = PhabricatorSpacesNamespaceQuery::getViewerSpaces($viewer); - - $map = mpull($viewer_spaces, 'getNamespaceName', 'getPHID'); - asort($map); - - return $map; - } - - protected function renderInput() { - $viewer = $this->getUser(); - - $this->setLabel(pht('Space')); - - $value = $this->getValue(); - if ($value === null) { - $value = $viewer->getDefaultSpacePHID(); - } - - return AphrontFormSelectControl::renderSelectTag( - $value, - $this->getOptions(), - array( - 'name' => $this->getName(), - )); - } - -} diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -360,10 +360,13 @@ case PhabricatorTransactions::TYPE_SPACE: $space_phid = $xaction->getNewValue(); if (!strlen($space_phid)) { - // If an install has no Spaces, we might end up with the empty string - // here instead of a strict `null`. Just make this work like callers - // might reasonably expect. - return null; + // If an install has no Spaces or the Spaces controls are not visible + // to the viewer, we might end up with the empty string here instead + // of a strict `null`, because some controller just used `getStr()` + // to read the space PHID from the request. + // Just make this work like callers might reasonably expect so we + // don't need to handle this specially in every EditController. + return $this->getActor()->getDefaultSpacePHID(); } else { return $space_phid; } @@ -2002,14 +2005,15 @@ $transaction_type) { $errors = array(); - $all_spaces = PhabricatorSpacesNamespaceQuery::getAllSpaces(); - $viewer_spaces = PhabricatorSpacesNamespaceQuery::getViewerSpaces( - $this->getActor()); + $actor = $this->getActor(); + + $has_spaces = PhabricatorSpacesNamespaceQuery::getViewerSpacesExist($actor); + $actor_spaces = PhabricatorSpacesNamespaceQuery::getViewerSpaces($actor); foreach ($xactions as $xaction) { $space_phid = $xaction->getNewValue(); if ($space_phid === null) { - if (!$all_spaces) { + if (!$has_spaces) { // The install doesn't have any spaces, so this is fine. continue; } @@ -2026,7 +2030,7 @@ // If the PHID isn't `null`, it needs to be a valid space that the // viewer can see. - if (empty($viewer_spaces[$space_phid])) { + if (empty($actor_spaces[$space_phid])) { $errors[] = new PhabricatorApplicationTransactionValidationError( $transaction_type, pht('Invalid'), @@ -2045,7 +2049,18 @@ PhabricatorLiskDAO $object, array $xactions) { - return clone $object; + $copy = clone $object; + + foreach ($xactions as $xaction) { + switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_SPACE: + $space_phid = $this->getTransactionNewValue($object, $xaction); + $copy->setSpacePHID($space_phid); + break; + } + } + + return $copy; } protected function validateAllTransactions( diff --git a/src/view/form/control/AphrontFormPolicyControl.php b/src/view/form/control/AphrontFormPolicyControl.php --- a/src/view/form/control/AphrontFormPolicyControl.php +++ b/src/view/form/control/AphrontFormPolicyControl.php @@ -5,6 +5,7 @@ private $object; private $capability; private $policies; + private $spacePHID; public function setPolicyObject(PhabricatorPolicyInterface $object) { $this->object = $object; @@ -17,6 +18,15 @@ return $this; } + public function setSpacePHID($space_phid) { + $this->spacePHID = $space_phid; + return $this; + } + + public function getSpacePHID() { + return $this->spacePHID; + } + public function setCapability($capability) { $this->capability = $capability; @@ -187,11 +197,14 @@ $selected_icon = idx($selected, 'icon'); $selected_name = idx($selected, 'name'); + $spaces_control = $this->buildSpacesControl(); + return phutil_tag( 'div', array( ), array( + $spaces_control, javelin_tag( 'a', array( @@ -231,4 +244,48 @@ return 'custom:placeholder'; } + private function buildSpacesControl() { + if ($this->capability != PhabricatorPolicyCapability::CAN_VIEW) { + return null; + } + + if (!($this->object instanceof PhabricatorSpacesInterface)) { + return null; + } + + $viewer = $this->getUser(); + if (!PhabricatorSpacesNamespaceQuery::getViewerSpacesExist($viewer)) { + return null; + } + + $space_phid = $this->getSpacePHID(); + if ($space_phid === null) { + $space_phid = $viewer->getDefaultSpacePHID(); + } + + $select = AphrontFormSelectControl::renderSelectTag( + $space_phid, + $this->getSpaceOptions(), + array( + 'name' => 'spacePHID', + )); + + return $select; + } + + protected function getSpaceOptions() { + $viewer = $this->getUser(); + $viewer_spaces = PhabricatorSpacesNamespaceQuery::getViewerSpaces($viewer); + + $map = array(); + foreach ($viewer_spaces as $space) { + $map[$space->getPHID()] = pht( + 'Space %s: %s', + $space->getMonogram(), + $space->getNamespaceName()); + } + asort($map); + + return $map; + } }