From e0faa66772ed1afb8b8ea9964992b6c0ddd2dced Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 18 Aug 2015 13:36:05 -0700 Subject: [PATCH] Allow Owners Packages to be archived Summary: Fixes T8428. Adds status to packages, allows setting and application search. I presume though these need checked elsewhere? Test Plan: New package, edit package, archive package, run search queries. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T8428 Differential Revision: https://secure.phabricator.com/D13925 --- .../autopatches/20150815.owners.status.1.sql | 2 ++ .../autopatches/20150815.owners.status.2.sql | 2 ++ .../controller/DifferentialController.php | 2 ++ .../controller/DiffusionBrowseController.php | 1 + .../controller/DiffusionCommitController.php | 2 ++ .../PhabricatorOwnersDetailController.php | 11 +++++++++ .../PhabricatorOwnersEditController.php | 23 +++++++++++++++++-- ...bricatorOwnersPackageTransactionEditor.php | 8 +++++++ .../query/PhabricatorOwnersPackageQuery.php | 13 +++++++++++ .../PhabricatorOwnersPackageSearchEngine.php | 21 +++++++++++++++++ .../storage/PhabricatorOwnersPackage.php | 17 ++++++++++++++ .../PhabricatorOwnersPackageTransaction.php | 11 +++++++++ 12 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 resources/sql/autopatches/20150815.owners.status.1.sql create mode 100644 resources/sql/autopatches/20150815.owners.status.2.sql diff --git a/resources/sql/autopatches/20150815.owners.status.1.sql b/resources/sql/autopatches/20150815.owners.status.1.sql new file mode 100644 index 0000000000..591e9efeef --- /dev/null +++ b/resources/sql/autopatches/20150815.owners.status.1.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_owners.owners_package + ADD status VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20150815.owners.status.2.sql b/resources/sql/autopatches/20150815.owners.status.2.sql new file mode 100644 index 0000000000..8f8c05af06 --- /dev/null +++ b/resources/sql/autopatches/20150815.owners.status.2.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_owners.owners_package + SET status = 'active' WHERE status = ''; diff --git a/src/applications/differential/controller/DifferentialController.php b/src/applications/differential/controller/DifferentialController.php index eed059e265..31cb51eaab 100644 --- a/src/applications/differential/controller/DifferentialController.php +++ b/src/applications/differential/controller/DifferentialController.php @@ -47,6 +47,7 @@ abstract class DifferentialController extends PhabricatorController { if ($viewer->getPHID()) { $packages = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withAuthorityPHIDs(array($viewer->getPHID())) ->execute(); $toc_view->setAuthorityPackages($packages); @@ -58,6 +59,7 @@ abstract class DifferentialController extends PhabricatorController { $control_query = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withControl($repository_phid, $paths); $control_query->execute(); } diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index 7450c3921a..e8bd46b735 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -167,6 +167,7 @@ abstract class DiffusionBrowseController extends DiffusionController { if (PhabricatorApplication::isClassInstalled($owners)) { $package_query = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withControl( $repository->getPHID(), array( diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index b91a7dc804..627150b682 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -1089,6 +1089,7 @@ final class DiffusionCommitController extends DiffusionController { if ($viewer->getPHID()) { $packages = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withAuthorityPHIDs(array($viewer->getPHID())) ->execute(); $toc_view->setAuthorityPackages($packages); @@ -1099,6 +1100,7 @@ final class DiffusionCommitController extends DiffusionController { $control_query = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withControl($repository_phid, mpull($changesets, 'getFilename')); $control_query->execute(); } diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index 008cb715e5..19f305dc09 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -41,9 +41,20 @@ final class PhabricatorOwnersDetailController $properties = $this->buildPackagePropertyView($package); $properties->setActionList($actions); + if ($package->isArchived()) { + $header_icon = 'fa-ban'; + $header_name = pht('Archived'); + $header_color = 'dark'; + } else { + $header_icon = 'fa-check'; + $header_name = pht('Active'); + $header_color = 'bluegrey'; + } + $header = id(new PHUIHeaderView()) ->setUser($viewer) ->setHeader($package->getName()) + ->setStatus($header_icon, $header_color, $header_name) ->setPolicyObject($package); $panel = id(new PHUIObjectBoxView()) diff --git a/src/applications/owners/controller/PhabricatorOwnersEditController.php b/src/applications/owners/controller/PhabricatorOwnersEditController.php index f601cd3115..af5ae29be8 100644 --- a/src/applications/owners/controller/PhabricatorOwnersEditController.php +++ b/src/applications/owners/controller/PhabricatorOwnersEditController.php @@ -34,6 +34,7 @@ final class PhabricatorOwnersEditController $v_owners = mpull($package->getOwners(), 'getUserPHID'); $v_auditing = $package->getAuditingEnabled(); $v_description = $package->getDescription(); + $v_status = $package->getStatus(); $errors = array(); @@ -44,11 +45,13 @@ final class PhabricatorOwnersEditController $v_owners = $request->getArr('owners'); $v_auditing = ($request->getStr('auditing') == 'enabled'); $v_description = $request->getStr('description'); + $v_status = $request->getStr('status'); $type_name = PhabricatorOwnersPackageTransaction::TYPE_NAME; $type_owners = PhabricatorOwnersPackageTransaction::TYPE_OWNERS; $type_auditing = PhabricatorOwnersPackageTransaction::TYPE_AUDITING; $type_description = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION; + $type_status = PhabricatorOwnersPackageTransaction::TYPE_STATUS; $xactions[] = id(new PhabricatorOwnersPackageTransaction()) ->setTransactionType($type_name) @@ -66,6 +69,12 @@ final class PhabricatorOwnersEditController ->setTransactionType($type_description) ->setNewValue($v_description); + if (!$is_new) { + $xactions[] = id(new PhabricatorOwnersPackageTransaction()) + ->setTransactionType($type_status) + ->setNewValue($v_status); + } + $editor = id(new PhabricatorOwnersPackageTransactionEditor()) ->setActor($viewer) ->setContentSourceFromRequest($request) @@ -115,8 +124,18 @@ final class PhabricatorOwnersEditController ->setDatasource(new PhabricatorProjectOrUserDatasource()) ->setLabel(pht('Owners')) ->setName('owners') - ->setValue($v_owners)) - ->appendChild( + ->setValue($v_owners)); + + if (!$is_new) { + $form->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Status')) + ->setName('status') + ->setValue($v_status) + ->setOptions($package->getStatusNameMap())); + } + + $form->appendChild( id(new AphrontFormSelectControl()) ->setName('auditing') ->setLabel(pht('Auditing')) diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php index 79cce9e9af..8feeeb1408 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php @@ -19,6 +19,7 @@ final class PhabricatorOwnersPackageTransactionEditor $types[] = PhabricatorOwnersPackageTransaction::TYPE_AUDITING; $types[] = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION; $types[] = PhabricatorOwnersPackageTransaction::TYPE_PATHS; + $types[] = PhabricatorOwnersPackageTransaction::TYPE_STATUS; return $types; } @@ -42,6 +43,8 @@ final class PhabricatorOwnersPackageTransactionEditor case PhabricatorOwnersPackageTransaction::TYPE_PATHS: $paths = $object->getPaths(); return mpull($paths, 'getRef'); + case PhabricatorOwnersPackageTransaction::TYPE_STATUS: + return $object->getStatus(); } } @@ -53,6 +56,7 @@ final class PhabricatorOwnersPackageTransactionEditor case PhabricatorOwnersPackageTransaction::TYPE_NAME: case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: case PhabricatorOwnersPackageTransaction::TYPE_PATHS: + case PhabricatorOwnersPackageTransaction::TYPE_STATUS: return $xaction->getNewValue(); case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: return (int)$xaction->getNewValue(); @@ -99,6 +103,9 @@ final class PhabricatorOwnersPackageTransactionEditor case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: case PhabricatorOwnersPackageTransaction::TYPE_PATHS: return; + case PhabricatorOwnersPackageTransaction::TYPE_STATUS: + $object->setStatus($xaction->getNewValue()); + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -112,6 +119,7 @@ final class PhabricatorOwnersPackageTransactionEditor case PhabricatorOwnersPackageTransaction::TYPE_NAME: case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: + case PhabricatorOwnersPackageTransaction::TYPE_STATUS: return; case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: $old = $xaction->getOldValue(); diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php index 74f77f79b7..c049e7406e 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php @@ -10,6 +10,7 @@ final class PhabricatorOwnersPackageQuery private $repositoryPHIDs; private $paths; private $namePrefix; + private $statuses; private $controlMap = array(); private $controlResults; @@ -57,6 +58,11 @@ final class PhabricatorOwnersPackageQuery return $this; } + public function withStatuses(array $statuses) { + $this->statuses = $statuses; + return $this; + } + public function withControl($repository_phid, array $paths) { if (empty($this->controlMap[$repository_phid])) { $this->controlMap[$repository_phid] = array(); @@ -200,6 +206,13 @@ final class PhabricatorOwnersPackageQuery $this->getFragmentsForPaths($this->paths)); } + if ($this->statuses !== null) { + $where[] = qsprintf( + $conn, + 'p.status IN (%Ls)', + $this->statuses); + } + if (strlen($this->namePrefix)) { // NOTE: This is a hacky mess, but this column is currently case // sensitive and unique. diff --git a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php index a26a9b3c56..d3131bba47 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php @@ -31,6 +31,12 @@ final class PhabricatorOwnersPackageSearchEngine ->setLabel(pht('Paths')) ->setKey('paths') ->setAliases(array('path')), + id(new PhabricatorSearchCheckboxesField()) + ->setKey('statuses') + ->setLabel(pht('Status')) + ->setOptions( + id(new PhabricatorOwnersPackage()) + ->getStatusNameMap()), ); } @@ -49,6 +55,10 @@ final class PhabricatorOwnersPackageSearchEngine $query->withPaths($map['paths']); } + if ($map['statuses']) { + $query->withStatuses($map['statuses']); + } + return $query; } @@ -64,6 +74,7 @@ final class PhabricatorOwnersPackageSearchEngine } $names += array( + 'active' => pht('Active Packages'), 'all' => pht('All Packages'), ); @@ -77,6 +88,12 @@ final class PhabricatorOwnersPackageSearchEngine switch ($query_key) { case 'all': return $query; + case 'active': + return $query->setParameter( + 'statuses', + array( + PhabricatorOwnersPackage::STATUS_ACTIVE, + )); case 'authority': return $query->setParameter( 'authorityPHIDs', @@ -105,6 +122,10 @@ final class PhabricatorOwnersPackageSearchEngine ->setHeader($package->getName()) ->setHref('/owners/package/'.$id.'/'); + if ($package->isArchived()) { + $item->setDisabled(true); + } + $list->addItem($item); } diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php index 20aff7bd88..d9a24b179a 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -12,14 +12,19 @@ final class PhabricatorOwnersPackage protected $description; protected $primaryOwnerPHID; protected $mailKey; + protected $status; private $paths = self::ATTACHABLE; private $owners = self::ATTACHABLE; + const STATUS_ACTIVE = 'active'; + const STATUS_ARCHIVED = 'archived'; + public static function initializeNewPackage(PhabricatorUser $actor) { return id(new PhabricatorOwnersPackage()) ->setAuditingEnabled(0) ->attachPaths(array()) + ->setStatus(self::STATUS_ACTIVE) ->attachOwners(array()); } @@ -41,6 +46,13 @@ final class PhabricatorOwnersPackage return null; } + public static function getStatusNameMap() { + return array( + self::STATUS_ACTIVE => pht('Active'), + self::STATUS_ARCHIVED => pht('Archived'), + ); + } + protected function getConfiguration() { return array( // This information is better available from the history table. @@ -53,6 +65,7 @@ final class PhabricatorOwnersPackage 'primaryOwnerPHID' => 'phid?', 'auditingEnabled' => 'bool', 'mailKey' => 'bytes20', + 'status' => 'text32', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, @@ -81,6 +94,10 @@ final class PhabricatorOwnersPackage return parent::save(); } + public function isArchived() { + return ($this->getStatus() == self::STATUS_ARCHIVED); + } + public function setName($name) { $this->name = $name; if (!$this->getID()) { diff --git a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php index bf676f941b..edb4f5db8f 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php @@ -9,6 +9,7 @@ final class PhabricatorOwnersPackageTransaction const TYPE_AUDITING = 'owners.auditing'; const TYPE_DESCRIPTION = 'owners.description'; const TYPE_PATHS = 'owners.paths'; + const TYPE_STATUS = 'owners.status'; public function getApplicationName() { return 'owners'; @@ -115,6 +116,16 @@ final class PhabricatorOwnersPackageTransaction return pht( '%s updated paths for this package.', $this->renderHandleLink($author_phid)); + case self::TYPE_STATUS: + if ($new == PhabricatorOwnersPackage::STATUS_ACTIVE) { + return pht( + '%s activated this package.', + $this->renderHandleLink($author_phid)); + } else if ($new == PhabricatorOwnersPackage::STATUS_ARCHIVED) { + return pht( + '%s archived this package.', + $this->renderHandleLink($author_phid)); + } } return parent::getTitle();