Page MenuHomePhabricator

D13229.diff
No OneTemporary

D13229.diff

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 @@
-<?php
-
-final class PhabricatorSpacesControl extends AphrontFormControl {
-
- private $object;
-
- protected function shouldRender() {
- // Render this control only if some Spaces exist.
- return PhabricatorSpacesNamespaceQuery::getAllSpaces();
- }
-
- public function setObject(PhabricatorSpacesInterface $object) {
- $this->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;
+ }
}

File Metadata

Mime Type
text/plain
Expires
Tue, Nov 19, 1:13 AM (17 h, 11 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6725657
Default Alt Text
D13229.diff (9 KB)

Event Timeline