From 11861265fe94fa97e4d0563c5bdb7b8cac27282d Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 11 Jan 2017 15:09:57 -0800 Subject: [PATCH] Merge "Audit" more completely into "Diffusion" Summary: Fixes T6630. Long ago, "Audit", "Diffusion" and "Repositories" were three totally separate applications. This separation isn't useful and the three rapidly became intertwined. Ideally, they would all be one application. This doesn't take us quite that far, but Audit no longer has any controllers and has little actual behavior. The "Audit" screen has always just been a SearchEngine view of commits with some filters on it, and this formalizes that and puts a link to it in Diffusion. (This view has other uses, too.) Test Plan: - Accessed audit from home page. - Accessed audit/commits from Diffusion. - Could no longer uninstall Audit on its own. - Grepped for `/audit/` and `AuditApplication`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T6630 Differential Revision: https://secure.phabricator.com/D17186 --- src/__phutil_library_map__.php | 4 +-- .../PhabricatorAuditApplication.php | 29 +++++++------------ .../audit/conduit/AuditConduitAPIMethod.php | 3 +- .../PhabricatorAuditListController.php | 19 ------------ .../audit/editor/PhabricatorAuditEditor.php | 2 +- .../mail/PhabricatorAuditMailReceiver.php | 2 +- .../query/PhabricatorCommitSearchEngine.php | 2 +- .../PhabricatorDiffusionApplication.php | 6 ++++ .../DiffusionCommitListController.php | 26 +++++++++++++++++ .../DiffusionRepositoryListController.php | 11 +++++++ .../PhabricatorOwnersDetailController.php | 2 +- .../PhabricatorPeopleProfileMenuEngine.php | 2 +- .../engine/PhabricatorJumpNavHandler.php | 2 +- src/docs/book/phabricator.book | 4 --- src/docs/user/userguide/audit.diviner | 4 +-- 15 files changed, 65 insertions(+), 53 deletions(-) delete mode 100644 src/applications/audit/controller/PhabricatorAuditListController.php create mode 100644 src/applications/diffusion/controller/DiffusionCommitListController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index bba2d53d45..a644231520 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -642,6 +642,7 @@ phutil_register_library_map(array( 'DiffusionCommitHintQuery' => 'applications/diffusion/query/DiffusionCommitHintQuery.php', 'DiffusionCommitHookEngine' => 'applications/diffusion/engine/DiffusionCommitHookEngine.php', 'DiffusionCommitHookRejectException' => 'applications/diffusion/exception/DiffusionCommitHookRejectException.php', + 'DiffusionCommitListController' => 'applications/diffusion/controller/DiffusionCommitListController.php', 'DiffusionCommitMergeHeraldField' => 'applications/diffusion/herald/DiffusionCommitMergeHeraldField.php', 'DiffusionCommitMessageHeraldField' => 'applications/diffusion/herald/DiffusionCommitMessageHeraldField.php', 'DiffusionCommitPackageAuditHeraldField' => 'applications/diffusion/herald/DiffusionCommitPackageAuditHeraldField.php', @@ -1873,7 +1874,6 @@ phutil_register_library_map(array( 'PhabricatorAuditController' => 'applications/audit/controller/PhabricatorAuditController.php', 'PhabricatorAuditEditor' => 'applications/audit/editor/PhabricatorAuditEditor.php', 'PhabricatorAuditInlineComment' => 'applications/audit/storage/PhabricatorAuditInlineComment.php', - 'PhabricatorAuditListController' => 'applications/audit/controller/PhabricatorAuditListController.php', 'PhabricatorAuditListView' => 'applications/audit/view/PhabricatorAuditListView.php', 'PhabricatorAuditMailReceiver' => 'applications/audit/mail/PhabricatorAuditMailReceiver.php', 'PhabricatorAuditManagementDeleteWorkflow' => 'applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php', @@ -5346,6 +5346,7 @@ phutil_register_library_map(array( 'DiffusionCommitHintQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DiffusionCommitHookEngine' => 'Phobject', 'DiffusionCommitHookRejectException' => 'Exception', + 'DiffusionCommitListController' => 'DiffusionController', 'DiffusionCommitMergeHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitMessageHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitPackageAuditHeraldField' => 'DiffusionCommitHeraldField', @@ -6764,7 +6765,6 @@ phutil_register_library_map(array( 'Phobject', 'PhabricatorInlineCommentInterface', ), - 'PhabricatorAuditListController' => 'PhabricatorAuditController', 'PhabricatorAuditListView' => 'AphrontView', 'PhabricatorAuditMailReceiver' => 'PhabricatorObjectMailReceiver', 'PhabricatorAuditManagementDeleteWorkflow' => 'PhabricatorAuditManagementWorkflow', diff --git a/src/applications/audit/application/PhabricatorAuditApplication.php b/src/applications/audit/application/PhabricatorAuditApplication.php index 5c7ae66f0d..9eb9cd0e09 100644 --- a/src/applications/audit/application/PhabricatorAuditApplication.php +++ b/src/applications/audit/application/PhabricatorAuditApplication.php @@ -3,7 +3,7 @@ final class PhabricatorAuditApplication extends PhabricatorApplication { public function getBaseURI() { - return '/audit/'; + return '/diffusion/commit/'; } public function getIcon() { @@ -18,25 +18,16 @@ final class PhabricatorAuditApplication extends PhabricatorApplication { return pht('Browse and Audit Commits'); } + public function canUninstall() { + // Audit was once a separate application, but has largely merged with + // Diffusion. + return false; + } + public function isPinnedByDefault(PhabricatorUser $viewer) { - return true; - } - - public function getHelpDocumentationArticles(PhabricatorUser $viewer) { - return array( - array( - 'name' => pht('Audit User Guide'), - 'href' => PhabricatorEnv::getDoclink('Audit User Guide'), - ), - ); - } - - public function getRoutes() { - return array( - '/audit/' => array( - '(?:query/(?P[^/]+)/)?' => 'PhabricatorAuditListController', - ), - ); + return parent::isClassInstalledForViewer( + 'PhabricatorDiffusionApplication', + $viewer); } public function getApplicationOrder() { diff --git a/src/applications/audit/conduit/AuditConduitAPIMethod.php b/src/applications/audit/conduit/AuditConduitAPIMethod.php index 3a11b27698..a5a7957b1f 100644 --- a/src/applications/audit/conduit/AuditConduitAPIMethod.php +++ b/src/applications/audit/conduit/AuditConduitAPIMethod.php @@ -3,7 +3,8 @@ abstract class AuditConduitAPIMethod extends ConduitAPIMethod { final public function getApplication() { - return PhabricatorApplication::getByClass('PhabricatorAuditApplication'); + return PhabricatorApplication::getByClass( + 'PhabricatorDiffusionApplication'); } } diff --git a/src/applications/audit/controller/PhabricatorAuditListController.php b/src/applications/audit/controller/PhabricatorAuditListController.php deleted file mode 100644 index 4d2d7bf37e..0000000000 --- a/src/applications/audit/controller/PhabricatorAuditListController.php +++ /dev/null @@ -1,19 +0,0 @@ -setQueryKey($request->getURIData('queryKey')) - ->setSearchEngine(new PhabricatorCommitSearchEngine()) - ->setNavigation($this->buildSideNavView()); - - return $this->delegateToController($controller); - } - -} diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 608939df5a..4b2d9bec29 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -42,7 +42,7 @@ final class PhabricatorAuditEditor } public function getEditorApplicationClass() { - return 'PhabricatorAuditApplication'; + return 'PhabricatorDiffusionApplication'; } public function getEditorObjectsDescription() { diff --git a/src/applications/audit/mail/PhabricatorAuditMailReceiver.php b/src/applications/audit/mail/PhabricatorAuditMailReceiver.php index 36e68c76a9..9dc70e9d8a 100644 --- a/src/applications/audit/mail/PhabricatorAuditMailReceiver.php +++ b/src/applications/audit/mail/PhabricatorAuditMailReceiver.php @@ -4,7 +4,7 @@ final class PhabricatorAuditMailReceiver extends PhabricatorObjectMailReceiver { public function isEnabled() { return PhabricatorApplication::isClassInstalled( - 'PhabricatorAuditApplication'); + 'PhabricatorDiffusionApplication'); } protected function getObjectPattern() { diff --git a/src/applications/audit/query/PhabricatorCommitSearchEngine.php b/src/applications/audit/query/PhabricatorCommitSearchEngine.php index b0c5ec1d2c..40bab34264 100644 --- a/src/applications/audit/query/PhabricatorCommitSearchEngine.php +++ b/src/applications/audit/query/PhabricatorCommitSearchEngine.php @@ -73,7 +73,7 @@ final class PhabricatorCommitSearchEngine } protected function getURI($path) { - return '/audit/'.$path; + return '/diffusion/commit/'.$path; } protected function getBuiltinQueryNames() { diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php index 0e2b669eed..8fef05bd88 100644 --- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php +++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php @@ -28,6 +28,10 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication { 'name' => pht('Diffusion User Guide'), 'href' => PhabricatorEnv::getDoclink('Diffusion User Guide'), ), + array( + 'name' => pht('Audit User Guide'), + 'href' => PhabricatorEnv::getDoclink('Audit User Guide'), + ), ); } @@ -131,6 +135,8 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication { 'lint/' => 'DiffusionLintController', 'commit/' => array( + $this->getQueryRoutePattern() => + 'DiffusionCommitListController', $this->getEditRoutePattern('edit/') => 'DiffusionCommitEditController', ), diff --git a/src/applications/diffusion/controller/DiffusionCommitListController.php b/src/applications/diffusion/controller/DiffusionCommitListController.php new file mode 100644 index 0000000000..b8c008c4d6 --- /dev/null +++ b/src/applications/diffusion/controller/DiffusionCommitListController.php @@ -0,0 +1,26 @@ +setController($this) + ->buildResponse(); + } + + protected function buildApplicationCrumbs() { + $crumbs = parent::buildApplicationCrumbs(); + + $crumbs->addTextCrumb( + pht('Commits'), + $this->getApplicationURI('commit/')); + + return $crumbs; + } + +} diff --git a/src/applications/diffusion/controller/DiffusionRepositoryListController.php b/src/applications/diffusion/controller/DiffusionRepositoryListController.php index eebe0bf91f..d886209c89 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryListController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryListController.php @@ -7,8 +7,19 @@ final class DiffusionRepositoryListController extends DiffusionController { } public function handleRequest(AphrontRequest $request) { + $items = array(); + + $items[] = id(new PHUIListItemView()) + ->setType(PHUIListItemView::TYPE_LABEL) + ->setName(pht('Commits')); + + $items[] = id(new PHUIListItemView()) + ->setName('Browse Commits') + ->setHref($this->getApplicationURI('commit/')); + return id(new PhabricatorRepositorySearchEngine()) ->setController($this) + ->setNavigationItems($items) ->buildResponse(); } diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index c91de51cc6..d7d249d8a5 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -65,7 +65,7 @@ final class PhabricatorOwnersDetailController $commit_views = array(); - $commit_uri = id(new PhutilURI('/audit/')) + $commit_uri = id(new PhutilURI('/diffusion/commit/')) ->setQueryParams( array( 'auditorPHIDs' => $package->getPHID(), diff --git a/src/applications/people/engine/PhabricatorPeopleProfileMenuEngine.php b/src/applications/people/engine/PhabricatorPeopleProfileMenuEngine.php index aab3896721..9965147a12 100644 --- a/src/applications/people/engine/PhabricatorPeopleProfileMenuEngine.php +++ b/src/applications/people/engine/PhabricatorPeopleProfileMenuEngine.php @@ -63,7 +63,7 @@ final class PhabricatorPeopleProfileMenuEngine $viewer); if ($have_diffusion) { $uri = urisprintf( - '/audit/?authors=%s#R', + '/diffusion/commit/?authors=%s#R', $object->getPHID()); $items[] = $this->newItem() diff --git a/src/applications/search/engine/PhabricatorJumpNavHandler.php b/src/applications/search/engine/PhabricatorJumpNavHandler.php index 6c30af98dd..6f6ea3d0a9 100644 --- a/src/applications/search/engine/PhabricatorJumpNavHandler.php +++ b/src/applications/search/engine/PhabricatorJumpNavHandler.php @@ -6,7 +6,7 @@ final class PhabricatorJumpNavHandler extends Phobject { $jump = trim($jump); $patterns = array( - '/^a$/i' => 'uri:/audit/', + '/^a$/i' => 'uri:/diffusion/commit/', '/^f$/i' => 'uri:/feed/', '/^d$/i' => 'uri:/differential/', '/^r$/i' => 'uri:/diffusion/', diff --git a/src/docs/book/phabricator.book b/src/docs/book/phabricator.book index 2b362853b4..2beef23023 100644 --- a/src/docs/book/phabricator.book +++ b/src/docs/book/phabricator.book @@ -37,10 +37,6 @@ "name": "Arcanist Integration", "include": "(^src/applications/arcanist/)" }, - "audit": { - "name": "Audit", - "include": "(^src/applications/audit/)" - }, "auth": { "name": "Auth", "include": "(^src/applications/auth/)" diff --git a/src/docs/user/userguide/audit.diviner b/src/docs/user/userguide/audit.diviner index 9c2f752b08..2ebc26a687 100644 --- a/src/docs/user/userguide/audit.diviner +++ b/src/docs/user/userguide/audit.diviner @@ -23,8 +23,8 @@ track of two things: - **Audit Requests** which ask a user (or some other entity) to audit a commit. These can be triggered in a number of ways (see below). -In the Audit tool's home screen (at `/audit/`) and on the homepage you can see -commits and requests that require your action: +In the Audit tool's home screen and on the homepage you can see commits and +requests that require your action: - **Required Audits** are open audit requests that require you, a project you are a member of, or a package you own to audit a commit. An audit