From c7079b52a2f3d1ff13804384f286917dbd63c2aa Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 14 Mar 2014 11:22:00 -0700 Subject: [PATCH] Subscriptions - make a dialog for massive subscription lists Summary: Ref T4430. This just deploys it on the property lists. (Help on how to do translations better? I tried a more traditional pht('%s, %s, %s, and %d other(s)') but I think the string lookup assumes the %d comes as the second param or something?) Test Plan: Made a Maniphest Task with a hojillion subscribers and noted the working dialogue. Also made a Pholio Mock with lots of subscribers and it worked. Reviewers: epriestley Reviewed By: epriestley Subscribers: aran, epriestley, Korvin, chad Maniphest Tasks: T4430 Differential Revision: https://secure.phabricator.com/D8525 --- resources/celerity/map.php | 2 + src/__phutil_library_map__.php | 4 + .../ManiphestTaskDetailController.php | 12 +-- .../PhabricatorApplicationSubscriptions.php | 1 + ...PhabricatorSubscriptionsListController.php | 49 ++++++++++++ ...habricatorSubscriptionsUIEventListener.php | 11 ++- .../view/SubscriptionListDialogBuilder.php | 78 +++++++++++++++++++ .../view/SubscriptionListStringBuilder.php | 64 +++++++++++++++ .../PhabricatorBaseEnglishTranslation.php | 5 ++ .../subscriptions/subscribers-list.css | 15 ++++ 10 files changed, 230 insertions(+), 11 deletions(-) create mode 100644 src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php create mode 100644 src/applications/subscriptions/view/SubscriptionListDialogBuilder.php create mode 100644 src/applications/subscriptions/view/SubscriptionListStringBuilder.php create mode 100644 webroot/rsrc/css/application/subscriptions/subscribers-list.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 7fcc9ac718..d72f069ce7 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -109,6 +109,7 @@ return array( 'rsrc/css/application/search/search-results.css' => 'f240504c', 'rsrc/css/application/settings/settings.css' => 'ea8f5915', 'rsrc/css/application/slowvote/slowvote.css' => '266df6a1', + 'rsrc/css/application/subscriptions/subscribers-list.css' => '5bb30c78', 'rsrc/css/application/tokens/tokens.css' => 'fb286311', 'rsrc/css/application/uiexample/example.css' => '4741b891', 'rsrc/css/core/core.css' => 'da26ddb2', @@ -804,6 +805,7 @@ return array( 'sprite-projects-css' => '7578fa56', 'sprite-status-css' => '8bce1c97', 'sprite-tokens-css' => '1706b943', + 'subscribers-list-css' => '5bb30c78', 'syntax-highlighting-css' => '3c18c1cb', 'tokens-css' => 'fb286311', ), diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d6dda6bf31..ce2caee3ea 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2092,6 +2092,7 @@ phutil_register_library_map(array( 'PhabricatorSubscribersQuery' => 'applications/subscriptions/query/PhabricatorSubscribersQuery.php', 'PhabricatorSubscriptionsEditController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php', 'PhabricatorSubscriptionsEditor' => 'applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php', + 'PhabricatorSubscriptionsListController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsListController.php', 'PhabricatorSubscriptionsUIEventListener' => 'applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php', 'PhabricatorSymbolNameLinter' => 'infrastructure/lint/hook/PhabricatorSymbolNameLinter.php', 'PhabricatorSyntaxHighlighter' => 'infrastructure/markup/PhabricatorSyntaxHighlighter.php', @@ -2521,6 +2522,8 @@ phutil_register_library_map(array( 'SleepBuildStepImplementation' => 'applications/harbormaster/step/SleepBuildStepImplementation.php', 'SlowvoteEmbedView' => 'applications/slowvote/view/SlowvoteEmbedView.php', 'SlowvoteRemarkupRule' => 'applications/slowvote/remarkup/SlowvoteRemarkupRule.php', + 'SubscriptionListDialogBuilder' => 'applications/subscriptions/view/SubscriptionListDialogBuilder.php', + 'SubscriptionListStringBuilder' => 'applications/subscriptions/view/SubscriptionListStringBuilder.php', 'UploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/UploadArtifactBuildStepImplementation.php', 'VariableBuildStepImplementation' => 'applications/harbormaster/step/VariableBuildStepImplementation.php', 'WaitForPreviousBuildStepImplementation' => 'applications/harbormaster/step/WaitForPreviousBuildStepImplementation.php', @@ -4889,6 +4892,7 @@ phutil_register_library_map(array( 'PhabricatorSubscribersQuery' => 'PhabricatorQuery', 'PhabricatorSubscriptionsEditController' => 'PhabricatorController', 'PhabricatorSubscriptionsEditor' => 'PhabricatorEditor', + 'PhabricatorSubscriptionsListController' => 'PhabricatorController', 'PhabricatorSubscriptionsUIEventListener' => 'PhabricatorEventListener', 'PhabricatorSymbolNameLinter' => 'ArcanistXHPASTLintNamingHook', 'PhabricatorSyntaxHighlightingConfigOptions' => 'PhabricatorApplicationConfigOptions', diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index a93846a615..d3987c5115 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -516,11 +516,13 @@ final class ManiphestTaskDetailController extends ManiphestController { pht('Priority'), ManiphestTaskPriority::getTaskPriorityName($task->getPriority())); - $view->addProperty( - pht('Subscribers'), - $task->getCCPHIDs() - ? $this->renderHandlesForPHIDs($task->getCCPHIDs(), ',') - : phutil_tag('em', array(), pht('None'))); + $handles = $this->getLoadedHandles(); + $cc_handles = array_select_keys($handles, $task->getCCPHIDs()); + $subscriber_html = id(new SubscriptionListStringBuilder()) + ->setObjectPHID($task->getPHID()) + ->setHandles($cc_handles) + ->buildPropertyString(); + $view->addProperty(pht('Subscribers'), $subscriber_html); $view->addProperty( pht('Author'), diff --git a/src/applications/subscriptions/application/PhabricatorApplicationSubscriptions.php b/src/applications/subscriptions/application/PhabricatorApplicationSubscriptions.php index e569be0d8f..f78b6ae1df 100644 --- a/src/applications/subscriptions/application/PhabricatorApplicationSubscriptions.php +++ b/src/applications/subscriptions/application/PhabricatorApplicationSubscriptions.php @@ -21,6 +21,7 @@ final class PhabricatorApplicationSubscriptions extends PhabricatorApplication { '/subscriptions/' => array( '(?Padd|delete)/'. '(?P[^/]+)/' => 'PhabricatorSubscriptionsEditController', + 'list/(?P[^/]+)/' => 'PhabricatorSubscriptionsListController', ), ); } diff --git a/src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php b/src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php new file mode 100644 index 0000000000..14a1195638 --- /dev/null +++ b/src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php @@ -0,0 +1,49 @@ +phid = idx($data, 'phid'); + } + + public function processRequest() { + $request = $this->getRequest(); + + $viewer = $request->getUser(); + $phid = $this->phid; + + $object = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withPHIDs(array($phid)) + ->executeOne(); + + if ($object instanceof PhabricatorSubscribableInterface) { + $subscriber_phids = PhabricatorSubscribersQuery::loadSubscribersForPHID( + $phid); + } else if ($object instanceof ManiphestTask) { + $subscriber_phids = $object->getCCPHIDs(); + } + + $handle_phids = $subscriber_phids; + $handle_phids[] = $phid; + + $handles = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs($handle_phids) + ->execute(); + $object_handle = $handles[$phid]; + + $dialog = id(new SubscriptionListDialogBuilder()) + ->setViewer($viewer) + ->setTitle(pht('Subscribers for %s', $object_handle->getFullName())) + ->setObjectPHID($phid) + ->setHandles($handles) + ->buildDialog(); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } + +} diff --git a/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php b/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php index 0dd106efff..ebe9892cb7 100644 --- a/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php +++ b/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php @@ -113,14 +113,13 @@ final class PhabricatorSubscriptionsUIEventListener ->setViewer($user) ->withPHIDs($subscribers) ->execute(); - $sub_view = array(); - foreach ($subscribers as $subscriber) { - $sub_view[] = $handles[$subscriber]->renderLink(); - } - $sub_view = phutil_implode_html(', ', $sub_view); } else { - $sub_view = phutil_tag('em', array(), pht('None')); + $handles = array(); } + $sub_view = id(new SubscriptionListStringBuilder()) + ->setObjectPHID($object->getPHID()) + ->setHandles($handles) + ->buildPropertyString(); $view = $event->getValue('view'); $view->addProperty(pht('Subscribers'), $sub_view); diff --git a/src/applications/subscriptions/view/SubscriptionListDialogBuilder.php b/src/applications/subscriptions/view/SubscriptionListDialogBuilder.php new file mode 100644 index 0000000000..ba3c1ec8f8 --- /dev/null +++ b/src/applications/subscriptions/view/SubscriptionListDialogBuilder.php @@ -0,0 +1,78 @@ +viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + + public function setHandles(array $handles) { + assert_instances_of($handles, 'PhabricatorObjectHandle'); + $this->handles = $handles; + return $this; + } + + public function getHandles() { + return $this->handles; + } + + public function setObjectPHID($object_phid) { + $this->objectPHID = $object_phid; + return $this; + } + + public function getObjectPHID() { + return $this->objectPHID; + } + + public function setTitle($title) { + $this->title = $title; + return $this; + } + + public function getTitle() { + return $this->title; + } + + public function buildDialog() { + $phid = $this->getObjectPHID(); + $handles = $this->getHandles(); + $object_handle = $handles[$phid]; + unset($handles[$phid]); + + require_celerity_resource('subscribers-list-css'); + return id(new AphrontDialogView()) + ->setUser($this->getViewer()) + ->setClass('subscriber-list-dialog') + ->setTitle($this->getTitle()) + ->appendChild($this->buildBody($this->getViewer(), $handles)) + ->addCancelButton($object_handle->getURI(), pht('Dismiss')); + } + + private function buildBody(PhabricatorUser $viewer, array $handles) { + + $list = id(new PHUIObjectItemListView()) + ->setUser($viewer); + foreach ($handles as $handle) { + // TODO - include $handle image - T4400 + $item = id(new PHUIObjectItemView()) + ->setHeader($handle->getFullName()) + ->setHref($handle->getURI()) + ->setDisabled($handle->isDisabled()); + $list->addItem($item); + } + + return $list; + } + +} diff --git a/src/applications/subscriptions/view/SubscriptionListStringBuilder.php b/src/applications/subscriptions/view/SubscriptionListStringBuilder.php new file mode 100644 index 0000000000..37bdbe468a --- /dev/null +++ b/src/applications/subscriptions/view/SubscriptionListStringBuilder.php @@ -0,0 +1,64 @@ +handles = $handles; + return $this; + } + + public function getHandles() { + return $this->handles; + } + + public function setObjectPHID($object_phid) { + $this->objectPHID = $object_phid; + return $this; + } + + public function getObjectPHID() { + return $this->objectPHID; + } + + public function buildPropertyString() { + $phid = $this->getObjectPHID(); + $handles = $this->getHandles(); + + if (!$handles) { + return phutil_tag('em', array(), pht('None')); + } + + $html = array(); + $show_count = 3; + $subscribers_count = count($handles); + if ($subscribers_count <= $show_count) { + return phutil_implode_html(', ', mpull($handles, 'renderLink')); + } + + $args = array('%s, %s, %s, and %s'); + $shown = 0; + foreach ($handles as $handle) { + $shown++; + if ($shown > $show_count) { + break; + } + $args[] = $handle->renderLink(); + } + $not_shown_count = $subscribers_count - $show_count; + $not_shown_txt = pht('%d other(s)', $not_shown_count); + $args[] = javelin_tag( + 'a', + array( + 'href' => '/subscriptions/list/'.$phid.'/', + 'sigil' => 'workflow' + ), + $not_shown_txt); + + return call_user_func_array('pht', $args); + } + +} diff --git a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php index 666f674e01..5f09477f7d 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php @@ -541,6 +541,11 @@ abstract class PhabricatorBaseEnglishTranslation ), ), + '%d other(s)' => array( + '1 other', + '%d others', + ), + '%s edited subscriber(s), added %d: %s; removed %d: %s.' => '%s edited subscribers, added: %3$s; removed: %5$s', diff --git a/webroot/rsrc/css/application/subscriptions/subscribers-list.css b/webroot/rsrc/css/application/subscriptions/subscribers-list.css new file mode 100644 index 0000000000..0c5ed9ac52 --- /dev/null +++ b/webroot/rsrc/css/application/subscriptions/subscribers-list.css @@ -0,0 +1,15 @@ +/** + * @provides subscribers-list-css + */ + +.subscriber-list-dialog { + width: 400px; +} + +.subscriber-list-dialog .aphront-dialog-body { + padding: 0; +} + +.subscriber-list-dialog .phui-object-item-list-view { + margin: 0; +}