From 74b438db136f3b0132096ee2e56e8ee0ba505e1b Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 12 Aug 2012 19:19:46 -0700 Subject: [PATCH] Move "Mailing Lists" to a separate application Summary: There's currently no way to get here from the UI since nav tabs don't exist anymore. It's also always been hard to find this feature even when we had the tabs, since it's surprising that it's inside "MetaMTA". - Move mailing lists to a separate application. - Add `buildApplicationPage()`, since we don't really need `buildStandardPageResponse()` any more -- we can infer all the information from `PhabricatorApplication`. This will let us get rid of a lot of the `PhabricatorXXXController` classes which just define application information. - Add `getApplicationURI()` to reduce code duplication, and in case we want to let you move applications around some day. Test Plan: Looked/edited/saved mailing lists. Reviewers: btrahan, vrana Reviewed By: btrahan CC: aran Maniphest Tasks: T631 Differential Revision: https://secure.phabricator.com/D3248 --- src/__celerity_resource_map__.php | 2 +- src/__phutil_library_map__.php | 14 +++--- ...AphrontDefaultApplicationConfiguration.php | 3 -- .../base/PhabricatorApplication.php | 4 ++ .../base/controller/PhabricatorController.php | 38 +++++++++++++++ .../PhabricatorApplicationMailingLists.php | 46 +++++++++++++++++++ ...PhabricatorMailingListsEditController.php} | 23 +++++----- ...PhabricatorMailingListsListController.php} | 19 ++++---- .../storage/PhabricatorMetaMTAMailingList.php | 0 .../PhabricatorApplicationApplications.php | 4 ++ .../PhabricatorApplicationsListController.php | 4 +- src/view/layout/AphrontSideNavFilterView.php | 12 +++-- src/view/layout/AphrontSideNavView.php | 4 +- 13 files changed, 132 insertions(+), 41 deletions(-) create mode 100644 src/applications/mailinglists/application/PhabricatorApplicationMailingLists.php rename src/applications/{metamta/controller/PhabricatorMetaMTAMailingListEditController.php => mailinglists/controller/PhabricatorMailingListsEditController.php} (88%) rename src/applications/{metamta/controller/PhabricatorMetaMTAMailingListsController.php => mailinglists/controller/PhabricatorMailingListsListController.php} (84%) rename src/applications/{metamta => mailinglists}/storage/PhabricatorMetaMTAMailingList.php (100%) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 0c7ae3b8ea..ac4562e41b 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -2564,7 +2564,7 @@ celerity_register_resource_map(array( ), 'phabricator-nav-view-css' => array( - 'uri' => '/res/82636b80/rsrc/css/aphront/phabricator-nav-view.css', + 'uri' => '/res/051b6ab6/rsrc/css/aphront/phabricator-nav-view.css', 'type' => 'css', 'requires' => array( diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cf538cb35b..d0e9071a85 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -551,6 +551,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationFact' => 'applications/fact/application/PhabricatorApplicationFact.php', 'PhabricatorApplicationFlags' => 'applications/flag/application/PhabricatorApplicationFlags.php', 'PhabricatorApplicationLaunchView' => 'applications/meta/view/PhabricatorApplicationLaunchView.php', + 'PhabricatorApplicationMailingLists' => 'applications/mailinglists/application/PhabricatorApplicationMailingLists.php', 'PhabricatorApplicationManiphest' => 'applications/maniphest/application/PhabricatorApplicationManiphest.php', 'PhabricatorApplicationPeople' => 'applications/people/application/PhabricatorApplicationPeople.php', 'PhabricatorApplicationPhriction' => 'applications/phriction/application/PhabricatorApplicationPhriction.php', @@ -777,6 +778,8 @@ phutil_register_library_map(array( 'PhabricatorMailImplementationSendGridAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationSendGridAdapter.php', 'PhabricatorMailImplementationTestAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php', 'PhabricatorMailReplyHandler' => 'applications/metamta/replyhandler/PhabricatorMailReplyHandler.php', + 'PhabricatorMailingListsEditController' => 'applications/mailinglists/controller/PhabricatorMailingListsEditController.php', + 'PhabricatorMailingListsListController' => 'applications/mailinglists/controller/PhabricatorMailingListsListController.php', 'PhabricatorMainMenuGroupView' => 'view/page/menu/PhabricatorMainMenuGroupView.php', 'PhabricatorMainMenuIconView' => 'view/page/menu/PhabricatorMainMenuIconView.php', 'PhabricatorMainMenuSearchView' => 'view/page/menu/PhabricatorMainMenuSearchView.php', @@ -795,9 +798,7 @@ phutil_register_library_map(array( 'PhabricatorMetaMTAMailBody' => 'applications/metamta/view/PhabricatorMetaMTAMailBody.php', 'PhabricatorMetaMTAMailBodyTestCase' => 'applications/metamta/view/__tests__/PhabricatorMetaMTAMailBodyTestCase.php', 'PhabricatorMetaMTAMailTestCase' => 'applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php', - 'PhabricatorMetaMTAMailingList' => 'applications/metamta/storage/PhabricatorMetaMTAMailingList.php', - 'PhabricatorMetaMTAMailingListEditController' => 'applications/metamta/controller/PhabricatorMetaMTAMailingListEditController.php', - 'PhabricatorMetaMTAMailingListsController' => 'applications/metamta/controller/PhabricatorMetaMTAMailingListsController.php', + 'PhabricatorMetaMTAMailingList' => 'applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php', 'PhabricatorMetaMTAReceiveController' => 'applications/metamta/controller/PhabricatorMetaMTAReceiveController.php', 'PhabricatorMetaMTAReceivedListController' => 'applications/metamta/controller/PhabricatorMetaMTAReceivedListController.php', 'PhabricatorMetaMTAReceivedMail' => 'applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php', @@ -1670,6 +1671,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationFact' => 'PhabricatorApplication', 'PhabricatorApplicationFlags' => 'PhabricatorApplication', 'PhabricatorApplicationLaunchView' => 'AphrontView', + 'PhabricatorApplicationMailingLists' => 'PhabricatorApplication', 'PhabricatorApplicationManiphest' => 'PhabricatorApplication', 'PhabricatorApplicationPeople' => 'PhabricatorApplication', 'PhabricatorApplicationPhriction' => 'PhabricatorApplication', @@ -1678,7 +1680,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationSettings' => 'PhabricatorApplication', 'PhabricatorApplicationStatusView' => 'AphrontView', 'PhabricatorApplicationsController' => 'PhabricatorController', - 'PhabricatorApplicationsListController' => 'PhabricatorApplicationsController', + 'PhabricatorApplicationsListController' => 'PhabricatorController', 'PhabricatorAuditAddCommentController' => 'PhabricatorAuditController', 'PhabricatorAuditComment' => 'PhabricatorAuditDAO', 'PhabricatorAuditCommitListView' => 'AphrontView', @@ -1869,6 +1871,8 @@ phutil_register_library_map(array( 'PhabricatorMailImplementationPHPMailerLiteAdapter' => 'PhabricatorMailImplementationAdapter', 'PhabricatorMailImplementationSendGridAdapter' => 'PhabricatorMailImplementationAdapter', 'PhabricatorMailImplementationTestAdapter' => 'PhabricatorMailImplementationAdapter', + 'PhabricatorMailingListsEditController' => 'PhabricatorController', + 'PhabricatorMailingListsListController' => 'PhabricatorController', 'PhabricatorMainMenuGroupView' => 'AphrontView', 'PhabricatorMainMenuIconView' => 'AphrontView', 'PhabricatorMainMenuSearchView' => 'AphrontView', @@ -1882,8 +1886,6 @@ phutil_register_library_map(array( 'PhabricatorMetaMTAMailBodyTestCase' => 'PhabricatorTestCase', 'PhabricatorMetaMTAMailTestCase' => 'PhabricatorTestCase', 'PhabricatorMetaMTAMailingList' => 'PhabricatorMetaMTADAO', - 'PhabricatorMetaMTAMailingListEditController' => 'PhabricatorMetaMTAController', - 'PhabricatorMetaMTAMailingListsController' => 'PhabricatorMetaMTAController', 'PhabricatorMetaMTAReceiveController' => 'PhabricatorMetaMTAController', 'PhabricatorMetaMTAReceivedListController' => 'PhabricatorMetaMTAController', 'PhabricatorMetaMTAReceivedMail' => 'PhabricatorMetaMTADAO', diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php index faf49ba944..2db83e06e2 100644 --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -85,9 +85,6 @@ class AphrontDefaultApplicationConfiguration '' => 'PhabricatorMetaMTAListController', 'send/' => 'PhabricatorMetaMTASendController', 'view/(?P\d+)/' => 'PhabricatorMetaMTAViewController', - 'lists/' => 'PhabricatorMetaMTAMailingListsController', - 'lists/edit/(?:(?P\d+)/)?' - => 'PhabricatorMetaMTAMailingListEditController', 'receive/' => 'PhabricatorMetaMTAReceiveController', 'received/' => 'PhabricatorMetaMTAReceivedListController', 'sendgrid/' => 'PhabricatorMetaMTASendGridReceiveController', diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index 64d53542ae..8de9359d8f 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -66,6 +66,10 @@ abstract class PhabricatorApplication { return null; } + public function getTitleGlyph() { + return null; + } + /* -( URI Routing )-------------------------------------------------------- */ diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index b0dcb07cc5..c1e70572a1 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -128,6 +128,44 @@ abstract class PhabricatorController extends AphrontController { return $response; } + public function getApplicationURI($path = '') { + if (!$this->getCurrentApplication()) { + throw new Exception("No application!"); + } + return $this->getCurrentApplication()->getBaseURI().ltrim($path, '/'); + } + + public function buildApplicationPage($view, array $options) { + $page = $this->buildStandardPageView(); + + $application = $this->getCurrentApplication(); + if ($application) { + $page->setApplicationName($application->getName()); + $page->setTitle(idx($options, 'title')); + if ($application->getTitleGlyph()) { + $page->setGlyph($application->getTitleGlyph()); + } + } + + if (!($view instanceof AphrontSideNavFilterView)) { + $nav = new AphrontSideNavFilterView(); + if ($application) { + $nav->setCurrentApplication($application); + } + $nav->setUser($this->getRequest()->getUser()); + $nav->setFlexNav(true); + $nav->setShowApplicationMenu(true); + $nav->appendChild($view); + + $view = $nav; + } + + $page->appendChild($view); + + $response = new AphrontWebpageResponse(); + return $response->setContent($page->render()); + } + public function didProcessRequest($response) { $request = $this->getRequest(); $response->setRequest($request); diff --git a/src/applications/mailinglists/application/PhabricatorApplicationMailingLists.php b/src/applications/mailinglists/application/PhabricatorApplicationMailingLists.php new file mode 100644 index 0000000000..5fa1a82bea --- /dev/null +++ b/src/applications/mailinglists/application/PhabricatorApplicationMailingLists.php @@ -0,0 +1,46 @@ + array( + '' => 'PhabricatorMailingListsListController', + 'edit/(?:(?P\d+)/)?' => 'PhabricatorMailingListsEditController', + ), + ); + } + + public function getTitleGlyph() { + return '@'; + } + +} diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailingListEditController.php b/src/applications/mailinglists/controller/PhabricatorMailingListsEditController.php similarity index 88% rename from src/applications/metamta/controller/PhabricatorMetaMTAMailingListEditController.php rename to src/applications/mailinglists/controller/PhabricatorMailingListsEditController.php index b90dc60a20..eecc9b1db2 100644 --- a/src/applications/metamta/controller/PhabricatorMetaMTAMailingListEditController.php +++ b/src/applications/mailinglists/controller/PhabricatorMailingListsEditController.php @@ -16,8 +16,8 @@ * limitations under the License. */ -final class PhabricatorMetaMTAMailingListEditController - extends PhabricatorMetaMTAController { +final class PhabricatorMailingListsEditController + extends PhabricatorController { private $id; @@ -74,7 +74,7 @@ final class PhabricatorMetaMTAMailingListEditController try { $list->save(); return id(new AphrontRedirectResponse()) - ->setURI('/mail/lists/'); + ->setURI($this->getApplicationURI()); } catch (AphrontQueryDuplicateKeyException $ex) { $e_email = 'Duplicate'; $errors[] = 'Another mailing list already uses that address.'; @@ -92,9 +92,9 @@ final class PhabricatorMetaMTAMailingListEditController $form = new AphrontFormView(); $form->setUser($request->getUser()); if ($list->getID()) { - $form->setAction('/mail/lists/edit/'.$list->getID().'/'); + $form->setAction($this->getApplicationURI('/edit/'.$list->getID().'/')); } else { - $form->setAction('/mail/lists/edit/'); + $form->setAction($this->getApplicationURI('/edit/')); } $form @@ -119,10 +119,6 @@ final class PhabricatorMetaMTAMailingListEditController ->setError($e_uri) ->setCaption('Optional link to mailing list archives or info.') ->setValue($list->getURI())) - ->appendChild( - id(new AphrontFormStaticControl()) - ->setLabel('ID') - ->setValue(nonempty($list->getID(), '-'))) ->appendChild( id(new AphrontFormStaticControl()) ->setLabel('PHID') @@ -130,7 +126,7 @@ final class PhabricatorMetaMTAMailingListEditController ->appendChild( id(new AphrontFormSubmitControl()) ->setValue('Save') - ->addCancelButton('/mail/lists/')); + ->addCancelButton($this->getApplicationURI())); $panel = new AphrontPanelView(); if ($list->getID()) { @@ -142,8 +138,11 @@ final class PhabricatorMetaMTAMailingListEditController $panel->appendChild($form); $panel->setWidth(AphrontPanelView::WIDTH_FORM); - return $this->buildStandardPageResponse( - array($error_view, $panel), + return $this->buildApplicationPage( + array( + $error_view, + $panel, + ), array( 'title' => 'Edit Mailing List', )); diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailingListsController.php b/src/applications/mailinglists/controller/PhabricatorMailingListsListController.php similarity index 84% rename from src/applications/metamta/controller/PhabricatorMetaMTAMailingListsController.php rename to src/applications/mailinglists/controller/PhabricatorMailingListsListController.php index 6dbb57117b..2660aec3e3 100644 --- a/src/applications/metamta/controller/PhabricatorMetaMTAMailingListsController.php +++ b/src/applications/mailinglists/controller/PhabricatorMailingListsListController.php @@ -16,8 +16,8 @@ * limitations under the License. */ -final class PhabricatorMetaMTAMailingListsController - extends PhabricatorMetaMTAController { +final class PhabricatorMailingListsListController + extends PhabricatorController { public function processRequest() { $request = $this->getRequest(); @@ -25,7 +25,7 @@ final class PhabricatorMetaMTAMailingListsController $offset = $request->getInt('offset', 0); $pager = new AphrontPagerView(); - $pager->setPageSize(1000); + $pager->setPageSize(250); $pager->setOffset($offset); $pager->setURI($request->getRequestURI(), 'offset'); @@ -45,14 +45,13 @@ final class PhabricatorMetaMTAMailingListsController $rows = array(); foreach ($lists as $list) { $rows[] = array( - phutil_escape_html($list->getPHID()), - phutil_escape_html($list->getEmail()), phutil_escape_html($list->getName()), + phutil_escape_html($list->getEmail()), phutil_render_tag( 'a', array( 'class' => 'button grey small', - 'href' => '/mail/lists/edit/'.$list->getID().'/', + 'href' => $this->getApplicationURI('/edit/'.$list->getID().'/'), ), 'Edit'), ); @@ -61,14 +60,12 @@ final class PhabricatorMetaMTAMailingListsController $table = new AphrontTableView($rows); $table->setHeaders( array( - 'PHID', - 'Email', 'Name', + 'Email', '', )); $table->setColumnClasses( array( - null, null, 'wide', 'action', @@ -77,10 +74,10 @@ final class PhabricatorMetaMTAMailingListsController $panel = new AphrontPanelView(); $panel->appendChild($table); $panel->setHeader('Mailing Lists'); - $panel->setCreateButton('Add New List', '/mail/lists/edit/'); + $panel->setCreateButton('Add New List', $this->getApplicationURI('/edit/')); $panel->appendChild($pager); - return $this->buildStandardPageResponse( + return $this->buildApplicationPage( $panel, array( 'title' => 'Mailing Lists', diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMailingList.php b/src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php similarity index 100% rename from src/applications/metamta/storage/PhabricatorMetaMTAMailingList.php rename to src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php diff --git a/src/applications/meta/application/PhabricatorApplicationApplications.php b/src/applications/meta/application/PhabricatorApplicationApplications.php index 60f3e106f0..d454c79054 100644 --- a/src/applications/meta/application/PhabricatorApplicationApplications.php +++ b/src/applications/meta/application/PhabricatorApplicationApplications.php @@ -38,6 +38,10 @@ final class PhabricatorApplicationApplications extends PhabricatorApplication { ); } + public function getTitleGlyph() { + return "\xE0\xBC\x84"; + } + public function shouldAppearInLaunchView() { return false; } diff --git a/src/applications/meta/controller/PhabricatorApplicationsListController.php b/src/applications/meta/controller/PhabricatorApplicationsListController.php index 59ad41fec5..2d161e5fec 100644 --- a/src/applications/meta/controller/PhabricatorApplicationsListController.php +++ b/src/applications/meta/controller/PhabricatorApplicationsListController.php @@ -17,7 +17,7 @@ */ final class PhabricatorApplicationsListController - extends PhabricatorApplicationsController { + extends PhabricatorController { public function processRequest() { $request = $this->getRequest(); @@ -52,7 +52,7 @@ final class PhabricatorApplicationsListController ), id(new AphrontNullView())->appendChild($views)->render()); - return $this->buildStandardPageResponse( + return $this->buildApplicationPage( $view, array( 'title' => 'Applications', diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index a8ebba4c53..cb9e9354cf 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/src/view/layout/AphrontSideNavFilterView.php @@ -137,11 +137,13 @@ final class AphrontSideNavFilterView extends AphrontView { } public function render() { - if (!$this->baseURI) { - throw new Exception("Call setBaseURI() before render()!"); - } - if ($this->selectedFilter === false) { - throw new Exception("Call selectFilter() before render()!"); + if ($this->items) { + if (!$this->baseURI) { + throw new Exception("Call setBaseURI() before render()!"); + } + if ($this->selectedFilter === false) { + throw new Exception("Call selectFilter() before render()!"); + } } $view = new AphrontSideNavView(); diff --git a/src/view/layout/AphrontSideNavView.php b/src/view/layout/AphrontSideNavView.php index f4a0bedb2f..4172bf321d 100644 --- a/src/view/layout/AphrontSideNavView.php +++ b/src/view/layout/AphrontSideNavView.php @@ -73,6 +73,8 @@ final class AphrontSideNavView extends AphrontView { $content_id = celerity_generate_unique_node_id(); $collapse_id = null; $expand_id = null; + $local_id = null; + $local_menu = null; $main_id = celerity_generate_unique_node_id(); $apps = $this->renderApplications(); @@ -227,7 +229,7 @@ final class AphrontSideNavView extends AphrontView { $core = mpull($core, null, 'getPHID'); if ($current && empty($core[$current->getPHID()])) { - array_unshift($core, $this->current); + array_unshift($core, $current); } $apps = array();