From 88d9366701c5b73b20db16f442bf92a676110dca Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 28 Jun 2014 16:37:36 -0700 Subject: [PATCH] Allow users to search for signatures across documents Summary: Ref T3116. You can already search for sigatures on a specific document, but allow them to be searched across documents too. In particular, this lets users answer questions like "Which of these 5 documents has alincoln signed?" / "Has alincoln signed all the stuff I care about?" / "who has signed either L5 or equivalent document L22?", etc. Test Plan: {F171658} Reviewers: chad Reviewed By: chad Subscribers: epriestley Maniphest Tasks: T3116 Differential Revision: https://secure.phabricator.com/D9770 --- .../PhabricatorApplicationLegalpad.php | 4 +- .../controller/LegalpadController.php | 3 + ...egalpadDocumentSignatureListController.php | 59 +++++++++++------- .../PhabricatorLegalpadPHIDTypeDocument.php | 4 +- .../LegalpadDocumentSignatureSearchEngine.php | 62 ++++++++++++++++--- 5 files changed, 99 insertions(+), 33 deletions(-) diff --git a/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php b/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php index 9710d6eef8..f723f1b081 100644 --- a/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php +++ b/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php @@ -49,8 +49,8 @@ final class PhabricatorApplicationLegalpad extends PhabricatorApplication { 'done/' => 'LegalpadDocumentDoneController', 'verify/(?P[^/]+)/' => 'LegalpadDocumentSignatureVerificationController', - 'signatures/(?P\d+)/(?:query/(?P[^/]+)/)?' - => 'LegalpadDocumentSignatureListController', + 'signatures/(?:(?P\d+)/)?(?:query/(?P[^/]+)/)?' => + 'LegalpadDocumentSignatureListController', 'document/' => array( 'preview/' => 'PhabricatorMarkupPreviewController'), )); diff --git a/src/applications/legalpad/controller/LegalpadController.php b/src/applications/legalpad/controller/LegalpadController.php index 1715b4c400..a438bca36e 100644 --- a/src/applications/legalpad/controller/LegalpadController.php +++ b/src/applications/legalpad/controller/LegalpadController.php @@ -19,6 +19,9 @@ abstract class LegalpadController extends PhabricatorController { ->setViewer($user) ->addNavigationItems($nav->getMenu()); + $nav->addLabel(pht('Signatures')); + $nav->addFilter('signatures/', pht('Find Signatures')); + return $nav; } diff --git a/src/applications/legalpad/controller/LegalpadDocumentSignatureListController.php b/src/applications/legalpad/controller/LegalpadDocumentSignatureListController.php index 0be217813e..f7e9b4464b 100644 --- a/src/applications/legalpad/controller/LegalpadDocumentSignatureListController.php +++ b/src/applications/legalpad/controller/LegalpadDocumentSignatureListController.php @@ -7,7 +7,7 @@ final class LegalpadDocumentSignatureListController extends LegalpadController { private $document; public function willProcessRequest(array $data) { - $this->documentID = $data['id']; + $this->documentID = idx($data, 'id'); $this->queryKey = idx($data, 'queryKey'); } @@ -15,23 +15,28 @@ final class LegalpadDocumentSignatureListController extends LegalpadController { $request = $this->getRequest(); $user = $request->getUser(); - $document = id(new LegalpadDocumentQuery()) - ->setViewer($user) - ->withIDs(array($this->documentID)) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->executeOne(); - if (!$document) { - return new Aphront404Response(); + if ($this->documentID) { + $document = id(new LegalpadDocumentQuery()) + ->setViewer($user) + ->withIDs(array($this->documentID)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + if (!$document) { + return new Aphront404Response(); + } + + $this->document = $document; } - $this->document = $document; + $engine = id(new LegalpadDocumentSignatureSearchEngine()); - $engine = id(new LegalpadDocumentSignatureSearchEngine()) - ->setDocument($document); + if ($this->document) { + $engine->setDocument($this->document); + } $controller = id(new PhabricatorApplicationSearchController($request)) ->setQueryKey($this->queryKey) @@ -47,10 +52,14 @@ final class LegalpadDocumentSignatureListController extends LegalpadController { $nav = new AphrontSideNavFilterView(); $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); - id(new LegalpadDocumentSignatureSearchEngine()) - ->setViewer($user) - ->setDocument($this->document) - ->addNavigationItems($nav->getMenu()); + $engine = id(new LegalpadDocumentSignatureSearchEngine()) + ->setViewer($user); + + if ($this->document) { + $engine->setDocument($this->document); + } + + $engine->addNavigationItems($nav->getMenu()); return $nav; } @@ -58,9 +67,15 @@ final class LegalpadDocumentSignatureListController extends LegalpadController { public function buildApplicationCrumbs() { $crumbs = parent::buildApplicationCrumbs(); - $crumbs->addTextCrumb( - $this->document->getMonogram(), - '/'.$this->document->getMonogram()); + if ($this->document) { + $crumbs->addTextCrumb( + $this->document->getMonogram(), + '/'.$this->document->getMonogram()); + } else { + $crumbs->addTextCrumb( + pht('Signatures'), + '/legalpad/signatures/'); + } return $crumbs; } diff --git a/src/applications/legalpad/phid/PhabricatorLegalpadPHIDTypeDocument.php b/src/applications/legalpad/phid/PhabricatorLegalpadPHIDTypeDocument.php index ad76e1f334..3640181407 100644 --- a/src/applications/legalpad/phid/PhabricatorLegalpadPHIDTypeDocument.php +++ b/src/applications/legalpad/phid/PhabricatorLegalpadPHIDTypeDocument.php @@ -37,8 +37,8 @@ final class PhabricatorLegalpadPHIDTypeDocument extends PhabricatorPHIDType { $document = $objects[$phid]; $name = $document->getDocumentBody()->getTitle(); $handle->setName($name); - $handle->setFullName($name); - $handle->setURI('/legalpad/view/'.$document->getID().'/'); + $handle->setFullName($document->getMonogram().' '.$name); + $handle->setURI('/'.$document->getMonogram()); } } diff --git a/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php b/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php index b75fe0c649..83d91e8930 100644 --- a/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php +++ b/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php @@ -25,6 +25,15 @@ final class LegalpadDocumentSignatureSearchEngine 'signerPHIDs', $this->readUsersFromRequest($request, 'signers')); + $saved->setParameter( + 'documentPHIDs', + $this->readPHIDsFromRequest( + $request, + 'documents', + array( + PhabricatorLegalpadPHIDTypeDocument::TYPECONST, + ))); + return $saved; } @@ -38,6 +47,11 @@ final class LegalpadDocumentSignatureSearchEngine if ($this->document) { $query->withDocumentPHIDs(array($this->document->getPHID())); + } else { + $document_phids = $saved->getParameter('documentPHIDs', array()); + if ($document_phids) { + $query->withDocumentPHIDs($document_phids); + } } return $query; @@ -47,14 +61,25 @@ final class LegalpadDocumentSignatureSearchEngine AphrontFormView $form, PhabricatorSavedQuery $saved_query) { + $document_phids = $saved_query->getParameter('documentPHIDs', array()); $signer_phids = $saved_query->getParameter('signerPHIDs', array()); - $phids = array_merge($signer_phids); + $phids = array_merge($document_phids, $signer_phids); $handles = id(new PhabricatorHandleQuery()) ->setViewer($this->requireViewer()) ->withPHIDs($phids) ->execute(); + if (!$this->document) { + $form + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setDatasource('/typeahead/common/legalpaddocuments/') + ->setName('documents') + ->setLabel(pht('Documents')) + ->setValue(array_select_keys($handles, $document_phids))); + } + $form ->appendChild( id(new AphrontFormTokenizerControl()) @@ -68,10 +93,7 @@ final class LegalpadDocumentSignatureSearchEngine if ($this->document) { return '/legalpad/signatures/'.$this->document->getID().'/'.$path; } else { - throw new Exception( - pht( - 'Searching for signatures outside of a document context is not '. - 'currently supported.')); + return '/legalpad/signatures/'.$path; } } @@ -97,9 +119,12 @@ final class LegalpadDocumentSignatureSearchEngine } protected function getRequiredHandlePHIDsForResultList( - array $documents, + array $signatures, PhabricatorSavedQuery $query) { - return mpull($documents, 'getSignerPHID'); + + return array_merge( + mpull($signatures, 'getSignerPHID'), + mpull($signatures, 'getDocumentPHID')); } protected function renderResultList( @@ -150,6 +175,7 @@ final class LegalpadDocumentSignatureSearchEngine $rows[] = array( $sig_icon, + $handles[$document->getPHID()]->renderLink(), $handles[$signature->getSignerPHID()]->renderLink(), $name, phutil_tag( @@ -166,16 +192,26 @@ final class LegalpadDocumentSignatureSearchEngine ->setHeaders( array( '', + pht('Document'), pht('Account'), pht('Name'), pht('Email'), pht('Signed'), )) + ->setColumnVisibility( + array( + true, + + // Only show the "Document" column if we aren't scoped to a + // particular document. + !$this->document, + )) ->setColumnClasses( array( '', '', '', + '', 'wide', 'right', )); @@ -184,6 +220,18 @@ final class LegalpadDocumentSignatureSearchEngine ->setHeaderText(pht('Signatures')) ->appendChild($table); + if (!$this->document) { + $policy_notice = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setErrors( + array( + pht( + 'NOTE: You can only see your own signatures and signatures on '. + 'documents you have permission to edit.'), + )); + $box->setErrorView($policy_notice); + } + return $box; }