From c1056f6dab5352ac8d13ab74f4c2be8b88a13ce4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 15 Feb 2018 08:35:35 -0800 Subject: [PATCH] Partially clean up Phriction document status constants; introduce "phriction.document.search" Summary: Depends on D19098. Ref T13077. Phriction status constants currently use the "bag of statuses" approach typical of older code, and store integers in the database. This fixes the "bag of statuses" stuff; a future change will fix the integers. Also adds a skeleton for `phriction.document.search`, but doesn't implement the Conduit interface yet. Test Plan: Searched for documents with various status constraints. Grepped for removed status constants. Viewed document list. Maniphest Tasks: T13077 Differential Revision: https://secure.phabricator.com/D19099 --- src/__phutil_library_map__.php | 8 ++-- ...hrictionDocumentSearchConduitAPIMethod.php | 18 ++++++++ .../constants/PhrictionDocumentStatus.php | 44 ++++++++++++++++++- .../controller/PhrictionController.php | 2 +- .../controller/PhrictionListController.php | 2 +- .../query/PhrictionDocumentQuery.php | 42 ++---------------- ....php => PhrictionDocumentSearchEngine.php} | 44 ++++++++----------- .../search/PhrictionDocumentFerretEngine.php | 2 +- .../phriction/storage/PhrictionDocument.php | 23 ++++++++++ 9 files changed, 114 insertions(+), 71 deletions(-) create mode 100644 src/applications/phriction/conduit/PhrictionDocumentSearchConduitAPIMethod.php rename src/applications/phriction/query/{PhrictionSearchEngine.php => PhrictionDocumentSearchEngine.php} (72%) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3f42c58c66..4c9d40d9bc 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4870,6 +4870,8 @@ phutil_register_library_map(array( 'PhrictionDocumentPHIDType' => 'applications/phriction/phid/PhrictionDocumentPHIDType.php', 'PhrictionDocumentPathHeraldField' => 'applications/phriction/herald/PhrictionDocumentPathHeraldField.php', 'PhrictionDocumentQuery' => 'applications/phriction/query/PhrictionDocumentQuery.php', + 'PhrictionDocumentSearchConduitAPIMethod' => 'applications/phriction/conduit/PhrictionDocumentSearchConduitAPIMethod.php', + 'PhrictionDocumentSearchEngine' => 'applications/phriction/query/PhrictionDocumentSearchEngine.php', 'PhrictionDocumentStatus' => 'applications/phriction/constants/PhrictionDocumentStatus.php', 'PhrictionDocumentTitleHeraldField' => 'applications/phriction/herald/PhrictionDocumentTitleHeraldField.php', 'PhrictionDocumentTitleTransaction' => 'applications/phriction/xaction/PhrictionDocumentTitleTransaction.php', @@ -4886,7 +4888,6 @@ phutil_register_library_map(array( 'PhrictionRemarkupRule' => 'applications/phriction/markup/PhrictionRemarkupRule.php', 'PhrictionReplyHandler' => 'applications/phriction/mail/PhrictionReplyHandler.php', 'PhrictionSchemaSpec' => 'applications/phriction/storage/PhrictionSchemaSpec.php', - 'PhrictionSearchEngine' => 'applications/phriction/query/PhrictionSearchEngine.php', 'PhrictionTransaction' => 'applications/phriction/storage/PhrictionTransaction.php', 'PhrictionTransactionComment' => 'applications/phriction/storage/PhrictionTransactionComment.php', 'PhrictionTransactionEditor' => 'applications/phriction/editor/PhrictionTransactionEditor.php', @@ -10799,7 +10800,9 @@ phutil_register_library_map(array( 'PhrictionDocumentPHIDType' => 'PhabricatorPHIDType', 'PhrictionDocumentPathHeraldField' => 'PhrictionDocumentHeraldField', 'PhrictionDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', - 'PhrictionDocumentStatus' => 'PhrictionConstants', + 'PhrictionDocumentSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', + 'PhrictionDocumentSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PhrictionDocumentStatus' => 'PhabricatorObjectStatus', 'PhrictionDocumentTitleHeraldField' => 'PhrictionDocumentHeraldField', 'PhrictionDocumentTitleTransaction' => 'PhrictionDocumentTransactionType', 'PhrictionDocumentTransactionType' => 'PhabricatorModularTransactionType', @@ -10815,7 +10818,6 @@ phutil_register_library_map(array( 'PhrictionRemarkupRule' => 'PhutilRemarkupRule', 'PhrictionReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', 'PhrictionSchemaSpec' => 'PhabricatorConfigSchemaSpec', - 'PhrictionSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhrictionTransaction' => 'PhabricatorModularTransaction', 'PhrictionTransactionComment' => 'PhabricatorApplicationTransactionComment', 'PhrictionTransactionEditor' => 'PhabricatorApplicationTransactionEditor', diff --git a/src/applications/phriction/conduit/PhrictionDocumentSearchConduitAPIMethod.php b/src/applications/phriction/conduit/PhrictionDocumentSearchConduitAPIMethod.php new file mode 100644 index 0000000000..f51faede5f --- /dev/null +++ b/src/applications/phriction/conduit/PhrictionDocumentSearchConduitAPIMethod.php @@ -0,0 +1,18 @@ +getStatusSpecification($key)); + } + + public static function getStatusMap() { + $map = id(new self())->getStatusSpecifications(); + return ipull($map, 'name', 'key'); + } + + public function isActive() { + return ($this->getKey() == self::STATUS_EXISTS); + } + + protected function newStatusSpecifications() { + return array( + array( + 'key' => self::STATUS_EXISTS, + 'name' => pht('Active'), + 'icon' => 'fa-file-text-o', + 'color' => 'bluegrey', + ), + array( + 'key' => self::STATUS_DELETED, + 'name' => pht('Deleted'), + 'icon' => 'fa-file-text-o', + 'color' => 'grey', + ), + array( + 'key' => self::STATUS_MOVED, + 'name' => pht('Moved'), + 'icon' => 'fa-arrow-right', + 'color' => 'grey', + ), + array( + 'key' => self::STATUS_STUB, + 'name' => pht('Stub'), + 'icon' => 'fa-file-text-o', + 'color' => 'grey', + ), + ); + } } diff --git a/src/applications/phriction/controller/PhrictionController.php b/src/applications/phriction/controller/PhrictionController.php index 277692334f..51ffbcab1f 100644 --- a/src/applications/phriction/controller/PhrictionController.php +++ b/src/applications/phriction/controller/PhrictionController.php @@ -13,7 +13,7 @@ abstract class PhrictionController extends PhabricatorController { $nav->addFilter('/phriction/', pht('Index')); } - id(new PhrictionSearchEngine()) + id(new PhrictionDocumentSearchEngine()) ->setViewer($user) ->addNavigationItems($nav->getMenu()); diff --git a/src/applications/phriction/controller/PhrictionListController.php b/src/applications/phriction/controller/PhrictionListController.php index 44ef95f178..7da29b297d 100644 --- a/src/applications/phriction/controller/PhrictionListController.php +++ b/src/applications/phriction/controller/PhrictionListController.php @@ -12,7 +12,7 @@ final class PhrictionListController $controller = id(new PhabricatorApplicationSearchController()) ->setQueryKey($querykey) - ->setSearchEngine(new PhrictionSearchEngine()) + ->setSearchEngine(new PhrictionDocumentSearchEngine()) ->setNavigation($this->buildSideNavView()); return $this->delegateToController($controller); diff --git a/src/applications/phriction/query/PhrictionDocumentQuery.php b/src/applications/phriction/query/PhrictionDocumentQuery.php index b5118146d3..f6073b2907 100644 --- a/src/applications/phriction/query/PhrictionDocumentQuery.php +++ b/src/applications/phriction/query/PhrictionDocumentQuery.php @@ -12,12 +12,7 @@ final class PhrictionDocumentQuery private $needContent; - private $status = 'status-any'; - const STATUS_ANY = 'status-any'; - const STATUS_OPEN = 'status-open'; - const STATUS_NONSTUB = 'status-nonstub'; - - const ORDER_HIERARCHY = 'order-hierarchy'; + const ORDER_HIERARCHY = 'hierarchy'; public function withIDs(array $ids) { $this->ids = $ids; @@ -49,11 +44,6 @@ final class PhrictionDocumentQuery return $this; } - public function withStatus($status) { - $this->status = $status; - return $this; - } - public function needContent($need_content) { $this->needContent = $need_content; return $this; @@ -224,42 +214,16 @@ final class PhrictionDocumentQuery $this->depths); } - switch ($this->status) { - case self::STATUS_OPEN: - $where[] = qsprintf( - $conn, - 'd.status NOT IN (%Ld)', - array( - PhrictionDocumentStatus::STATUS_DELETED, - PhrictionDocumentStatus::STATUS_MOVED, - PhrictionDocumentStatus::STATUS_STUB, - )); - break; - case self::STATUS_NONSTUB: - $where[] = qsprintf( - $conn, - 'd.status NOT IN (%Ld)', - array( - PhrictionDocumentStatus::STATUS_MOVED, - PhrictionDocumentStatus::STATUS_STUB, - )); - break; - case self::STATUS_ANY: - break; - default: - throw new Exception(pht("Unknown status '%s'!", $this->status)); - } - return $where; } public function getBuiltinOrders() { - return array( + return parent::getBuiltinOrders() + array( self::ORDER_HIERARCHY => array( 'vector' => array('depth', 'title', 'updated'), 'name' => pht('Hierarchy'), ), - ) + parent::getBuiltinOrders(); + ); } public function getOrderableColumns() { diff --git a/src/applications/phriction/query/PhrictionSearchEngine.php b/src/applications/phriction/query/PhrictionDocumentSearchEngine.php similarity index 72% rename from src/applications/phriction/query/PhrictionSearchEngine.php rename to src/applications/phriction/query/PhrictionDocumentSearchEngine.php index 245500e0ff..e578b48346 100644 --- a/src/applications/phriction/query/PhrictionSearchEngine.php +++ b/src/applications/phriction/query/PhrictionDocumentSearchEngine.php @@ -1,6 +1,6 @@ needContent(true) - ->withStatus(PhrictionDocumentQuery::STATUS_NONSTUB); + ->needContent(true); } protected function buildQueryFromParameters(array $map) { $query = $this->newQuery(); - if ($map['status']) { - $query->withStatus($map['status']); + if ($map['statuses']) { + $query->withStatuses($map['statuses']); } return $query; @@ -29,10 +28,10 @@ final class PhrictionSearchEngine protected function buildCustomSearchFields() { return array( - id(new PhabricatorSearchSelectField()) - ->setKey('status') + id(new PhabricatorSearchCheckboxesField()) + ->setKey('statuses') ->setLabel(pht('Status')) - ->setOptions($this->getStatusOptions()), + ->setOptions(PhrictionDocumentStatus::getStatusMap()), ); } @@ -59,19 +58,15 @@ final class PhrictionSearchEngine return $query; case 'active': return $query->setParameter( - 'status', PhrictionDocumentQuery::STATUS_OPEN); + 'statuses', + array( + PhrictionDocumentStatus::STATUS_EXISTS, + )); } return parent::buildSavedQueryFromBuiltin($query_key); } - private function getStatusOptions() { - return array( - PhrictionDocumentQuery::STATUS_OPEN => pht('Show Active Documents'), - PhrictionDocumentQuery::STATUS_NONSTUB => pht('Show All Documents'), - ); - } - protected function getRequiredHandlePHIDsForResultList( array $documents, PhabricatorSavedQuery $query) { @@ -118,15 +113,14 @@ final class PhrictionSearchEngine $item->addAttribute($slug_uri); - switch ($document->getStatus()) { - case PhrictionDocumentStatus::STATUS_DELETED: - $item->setDisabled(true); - $item->addIcon('delete', pht('Deleted')); - break; - case PhrictionDocumentStatus::STATUS_MOVED: - $item->setDisabled(true); - $item->addIcon('arrow-right', pht('Moved Away')); - break; + $icon = $document->getStatusIcon(); + $color = $document->getStatusColor(); + $label = $document->getStatusDisplayName(); + + $item->setStatusIcon("{$icon} {$color}", $label); + + if (!$document->isActive()) { + $item->setDisabled(true); } $list->addItem($item); diff --git a/src/applications/phriction/search/PhrictionDocumentFerretEngine.php b/src/applications/phriction/search/PhrictionDocumentFerretEngine.php index 76802391e7..498b9274f0 100644 --- a/src/applications/phriction/search/PhrictionDocumentFerretEngine.php +++ b/src/applications/phriction/search/PhrictionDocumentFerretEngine.php @@ -12,7 +12,7 @@ final class PhrictionDocumentFerretEngine } public function newSearchEngine() { - return new PhrictionSearchEngine(); + return new PhrictionDocumentSearchEngine(); } } diff --git a/src/applications/phriction/storage/PhrictionDocument.php b/src/applications/phriction/storage/PhrictionDocument.php index 0a9f09afdd..730b8e09bf 100644 --- a/src/applications/phriction/storage/PhrictionDocument.php +++ b/src/applications/phriction/storage/PhrictionDocument.php @@ -148,6 +148,29 @@ final class PhrictionDocument extends PhrictionDAO return $this; } +/* -( Status )------------------------------------------------------------- */ + + + public function getStatusObject() { + return PhrictionDocumentStatus::newStatusObject($this->getStatus()); + } + + public function getStatusIcon() { + return $this->getStatusObject()->getIcon(); + } + + public function getStatusColor() { + return $this->getStatusObject()->getColor(); + } + + public function getStatusDisplayName() { + return $this->getStatusObject()->getDisplayName(); + } + + public function isActive() { + return $this->getStatusObject()->isActive(); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */