From e1566bef63c273404d3e8a3e8b4b4e23e387d772 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 5 Nov 2016 09:24:40 -0700 Subject: [PATCH 01/31] Fix a Calendar import issue where we looked up attendees by object instead of name Summary: Ref T11801. This issue led to the stack trace in T11801#199042. It wasn't obvious that this was wrong because the recover-on-duplicate-key code made it work correctly. Test Plan: Imported an event with external attendees with no warnings in the log. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11801 Differential Revision: https://secure.phabricator.com/D16804 --- .../calendar/import/PhabricatorCalendarImportEngine.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php index eafa8e332f..46bc95cdfc 100644 --- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php @@ -259,7 +259,7 @@ abstract class PhabricatorCalendarImportEngine if ($attendee_names) { $external_invitees = id(new PhabricatorCalendarExternalInviteeQuery()) ->setViewer($viewer) - ->withNames($attendee_names) + ->withNames(array_keys($attendee_names)) ->execute(); $external_invitees = mpull($external_invitees, null, 'getName'); From 17bd48320774a848a9e01c1972c14a851151a986 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 6 Nov 2016 07:36:25 -0800 Subject: [PATCH 02/31] Queue large ICS files for background import Summary: Ref T11801. When a file is larger than 512KB, queue it for background import instead of trying to do it in the foreground, sinc we risk hitting `max_execution_time`. Test Plan: {F1906943} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11801 Differential Revision: https://secure.phabricator.com/D16805 --- src/__phutil_library_map__.php | 2 + .../PhabricatorCalendarImportEditor.php | 3 +- ...PhabricatorCalendarICSFileImportEngine.php | 8 +++- .../PhabricatorCalendarICSURIImportEngine.php | 7 +++- .../PhabricatorCalendarImportEngine.php | 37 ++++++++++++++++- .../PhabricatorCalendarImportQueueLogType.php | 40 +++++++++++++++++++ ...habricatorCalendarImportTriggerLogType.php | 10 ++++- .../PhabricatorCalendarImportReloadWorker.php | 10 ++++- 8 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 src/applications/calendar/importlog/PhabricatorCalendarImportQueueLogType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4c6fb41333..1427739390 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2145,6 +2145,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarImportOrphanLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportOrphanLogType.php', 'PhabricatorCalendarImportPHIDType' => 'applications/calendar/phid/PhabricatorCalendarImportPHIDType.php', 'PhabricatorCalendarImportQuery' => 'applications/calendar/query/PhabricatorCalendarImportQuery.php', + 'PhabricatorCalendarImportQueueLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportQueueLogType.php', 'PhabricatorCalendarImportReloadController' => 'applications/calendar/controller/PhabricatorCalendarImportReloadController.php', 'PhabricatorCalendarImportReloadTransaction' => 'applications/calendar/xaction/PhabricatorCalendarImportReloadTransaction.php', 'PhabricatorCalendarImportReloadWorker' => 'applications/calendar/worker/PhabricatorCalendarImportReloadWorker.php', @@ -7012,6 +7013,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarImportOrphanLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportPHIDType' => 'PhabricatorPHIDType', 'PhabricatorCalendarImportQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorCalendarImportQueueLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportReloadController' => 'PhabricatorCalendarController', 'PhabricatorCalendarImportReloadTransaction' => 'PhabricatorCalendarImportTransactionType', 'PhabricatorCalendarImportReloadWorker' => 'PhabricatorWorker', diff --git a/src/applications/calendar/editor/PhabricatorCalendarImportEditor.php b/src/applications/calendar/editor/PhabricatorCalendarImportEditor.php index 2631dc1774..f83869d7b9 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarImportEditor.php +++ b/src/applications/calendar/editor/PhabricatorCalendarImportEditor.php @@ -54,7 +54,7 @@ final class PhabricatorCalendarImportEditor if ($should_reload) { $import_engine = $object->getEngine(); - $import_engine->importEventsFromSource($actor, $object); + $import_engine->importEventsFromSource($actor, $object, true); } if ($should_trigger) { @@ -107,6 +107,7 @@ final class PhabricatorCalendarImportEditor 'class' => 'PhabricatorCalendarImportReloadWorker', 'data' => array( 'importPHID' => $object->getPHID(), + 'via' => PhabricatorCalendarImportReloadWorker::VIA_TRIGGER, ), 'options' => array( 'objectPHID' => $object->getPHID(), diff --git a/src/applications/calendar/import/PhabricatorCalendarICSFileImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarICSFileImportEngine.php index 2bc4ac37ff..c37ba6c944 100644 --- a/src/applications/calendar/import/PhabricatorCalendarICSFileImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarICSFileImportEngine.php @@ -65,7 +65,8 @@ final class PhabricatorCalendarICSFileImportEngine public function importEventsFromSource( PhabricatorUser $viewer, - PhabricatorCalendarImport $import) { + PhabricatorCalendarImport $import, + $should_queue) { $phid_key = PhabricatorCalendarImportICSFileTransaction::PARAMKEY_FILE; $file_phid = $import->getParameter($phid_key); @@ -83,10 +84,13 @@ final class PhabricatorCalendarICSFileImportEngine $data = $file->loadFileData(); + if ($should_queue && $this->shouldQueueDataImport($data)) { + return $this->queueDataImport($import, $data); + } + return $this->importICSData($viewer, $import, $data); } - public function canDisable( PhabricatorUser $viewer, PhabricatorCalendarImport $import) { diff --git a/src/applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php index 82f6191c24..6174603c84 100644 --- a/src/applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php @@ -77,7 +77,8 @@ final class PhabricatorCalendarICSURIImportEngine public function importEventsFromSource( PhabricatorUser $viewer, - PhabricatorCalendarImport $import) { + PhabricatorCalendarImport $import, + $should_queue) { $uri_key = PhabricatorCalendarImportICSURITransaction::PARAMKEY_URI; $uri = $import->getParameter($uri_key); @@ -103,6 +104,10 @@ final class PhabricatorCalendarICSURIImportEngine $data = $file->loadFileData(); + if ($should_queue && $this->shouldQueueDataImport($data)) { + return $this->queueDataImport($import, $data); + } + return $this->importICSData($viewer, $import, $data); } diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php index 46bc95cdfc..ca1885a4af 100644 --- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php @@ -3,11 +3,12 @@ abstract class PhabricatorCalendarImportEngine extends Phobject { + const QUEUE_BYTE_LIMIT = 524288; + final public function getImportEngineType() { return $this->getPhobjectClassConstant('ENGINETYPE', 64); } - abstract public function getImportEngineName(); abstract public function getImportEngineTypeName(); abstract public function getImportEngineHint(); @@ -27,7 +28,8 @@ abstract class PhabricatorCalendarImportEngine abstract public function importEventsFromSource( PhabricatorUser $viewer, - PhabricatorCalendarImport $import); + PhabricatorCalendarImport $import, + $should_queue); abstract public function canDisable( PhabricatorUser $viewer, @@ -568,4 +570,35 @@ abstract class PhabricatorCalendarImportEngine return (bool)$any_event; } + final protected function shouldQueueDataImport($data) { + return (strlen($data) > self::QUEUE_BYTE_LIMIT); + } + + final protected function queueDataImport( + PhabricatorCalendarImport $import, + $data) { + + $import->newLogMessage( + PhabricatorCalendarImportQueueLogType::LOGTYPE, + array( + 'data.size' => strlen($data), + 'data.limit' => self::QUEUE_BYTE_LIMIT, + )); + + // When we queue on this pathway, we're queueing in response to an explicit + // user action (like uploading a big `.ics` file), so we queue at normal + // priority instead of bulk/import priority. + + PhabricatorWorker::scheduleTask( + 'PhabricatorCalendarImportReloadWorker', + array( + 'importPHID' => $import->getPHID(), + 'via' => PhabricatorCalendarImportReloadWorker::VIA_BACKGROUND, + ), + array( + 'objectPHID' => $import->getPHID(), + )); + } + + } diff --git a/src/applications/calendar/importlog/PhabricatorCalendarImportQueueLogType.php b/src/applications/calendar/importlog/PhabricatorCalendarImportQueueLogType.php new file mode 100644 index 0000000000..2ab0b5be91 --- /dev/null +++ b/src/applications/calendar/importlog/PhabricatorCalendarImportQueueLogType.php @@ -0,0 +1,40 @@ +getParameter('data.size'); + $limit = $log->getParameter('data.limit'); + + return pht( + 'Queued for background import: data size (%s) exceeds limit for '. + 'immediate processing (%s).', + phutil_format_bytes($size), + phutil_format_bytes($limit)); + } + + public function getDisplayIcon( + PhabricatorUser $viewer, + PhabricatorCalendarImportLog $log) { + return 'fa-sort-amount-desc'; + } + + public function getDisplayColor( + PhabricatorUser $viewer, + PhabricatorCalendarImportLog $log) { + return 'blue'; + } + +} diff --git a/src/applications/calendar/importlog/PhabricatorCalendarImportTriggerLogType.php b/src/applications/calendar/importlog/PhabricatorCalendarImportTriggerLogType.php index b41a893850..8441a9f86b 100644 --- a/src/applications/calendar/importlog/PhabricatorCalendarImportTriggerLogType.php +++ b/src/applications/calendar/importlog/PhabricatorCalendarImportTriggerLogType.php @@ -14,7 +14,15 @@ final class PhabricatorCalendarImportTriggerLogType public function getDisplayDescription( PhabricatorUser $viewer, PhabricatorCalendarImportLog $log) { - return pht('Triggered a periodic update.'); + + $via = $log->getParameter('via'); + switch ($via) { + case PhabricatorCalendarImportReloadWorker::VIA_BACKGROUND: + return pht('Started background processing.'); + case PhabricatorCalendarImportReloadWorker::VIA_TRIGGER: + default: + return pht('Triggered a periodic update.'); + } } public function getDisplayIcon( diff --git a/src/applications/calendar/worker/PhabricatorCalendarImportReloadWorker.php b/src/applications/calendar/worker/PhabricatorCalendarImportReloadWorker.php index 3abc9479c6..8f27a814ab 100644 --- a/src/applications/calendar/worker/PhabricatorCalendarImportReloadWorker.php +++ b/src/applications/calendar/worker/PhabricatorCalendarImportReloadWorker.php @@ -2,6 +2,9 @@ final class PhabricatorCalendarImportReloadWorker extends PhabricatorWorker { + const VIA_TRIGGER = 'trigger'; + const VIA_BACKGROUND = 'background'; + protected function doWork() { $import = $this->loadImport(); $viewer = PhabricatorUser::getOmnipotentUser(); @@ -18,11 +21,14 @@ final class PhabricatorCalendarImportReloadWorker extends PhabricatorWorker { $import_engine = $import->getEngine(); + $data = $this->getTaskData(); $import->newLogMessage( PhabricatorCalendarImportTriggerLogType::LOGTYPE, - array()); + array( + 'via' => idx($data, 'via', self::VIA_TRIGGER), + )); - $import_engine->importEventsFromSource($author, $import); + $import_engine->importEventsFromSource($author, $import, false); } private function loadImport() { From f4f3b90c871f2c057948a891efbfdd33566aa028 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 6 Nov 2016 08:10:02 -0800 Subject: [PATCH 03/31] On tasks, put Task Graph, Mocks and Mentions into a tabgroup Summary: Fixes T4788. This change: - converts the "Task Graph" into a "Related Objects" tabgroup. - makes "Task Graph" the first tab in the group. - moves "Mocks" to become a tab. - adds a new "Mentions" tab, which shows inbound and outbound mentions. Primary goal of "mocks" is to give us room for a pinboard/thumbnail view after the next Pholio iteration. Might make sense to make it the default tab (if present) at that point, too, since mocks are probably more important than related tasks when they're present. Primary goal of "mentions" is to provide a bit of general support for various freeform relationships between tasks: if you want to treat tasks as "siblings" or "related" or "following" or whatever, you can at least find them all in one place. I don't plan to formalize any of these weird one-off relationships in the upstream, although it's vaguely possible that some far-future update might just let you define arbitrary custom relationships and then you can do whatever you want. Test Plan: {F1906974} Reviewers: chad Reviewed By: chad Maniphest Tasks: T4788 Differential Revision: https://secure.phabricator.com/D16806 --- .../ManiphestTaskDetailController.php | 151 ++++++++++++++---- 1 file changed, 122 insertions(+), 29 deletions(-) diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index 4e27479337..5d1999c2af 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -30,20 +30,19 @@ final class ManiphestTaskDetailController extends ManiphestController { ->setViewer($viewer) ->setTargetObject($task); - $e_commit = ManiphestTaskHasCommitEdgeType::EDGECONST; - $e_rev = ManiphestTaskHasRevisionEdgeType::EDGECONST; - $e_mock = ManiphestTaskHasMockEdgeType::EDGECONST; + $edge_types = array( + ManiphestTaskHasCommitEdgeType::EDGECONST, + ManiphestTaskHasRevisionEdgeType::EDGECONST, + ManiphestTaskHasMockEdgeType::EDGECONST, + PhabricatorObjectMentionedByObjectEdgeType::EDGECONST, + PhabricatorObjectMentionsObjectEdgeType::EDGECONST, + ); $phid = $task->getPHID(); $query = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs(array($phid)) - ->withEdgeTypes( - array( - $e_commit, - $e_rev, - $e_mock, - )); + ->withEdgeTypes($edge_types); $edges = idx($query->execute(), $phid); $phids = array_fill_keys($query->getDestinationPHIDs(), true); @@ -77,15 +76,8 @@ final class ManiphestTaskDetailController extends ManiphestController { $timeline->setQuoteRef($monogram); $comment_view->setTransactionTimeline($timeline); - $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setCurtain($curtain) - ->setMainColumn(array( - $timeline, - $comment_view, - )) - ->addPropertySection(pht('Description'), $description) - ->addPropertySection(pht('Details'), $details); + $related_tabs = array(); + $graph_menu = null; $graph_limit = 100; $task_graph = id(new ManiphestTaskGraph()) @@ -159,13 +151,50 @@ final class ManiphestTaskDetailController extends ManiphestController { ->setText($search_text) ->setDropdownMenu($dropdown_menu); - $graph_header = id(new PHUIHeaderView()) - ->setHeader(pht('Task Graph')) - ->addActionLink($graph_menu); - - $view->addPropertySection($graph_header, $graph_table); + $related_tabs[] = id(new PHUITabView()) + ->setName(pht('Task Graph')) + ->setKey('graph') + ->appendChild($graph_table); } + $related_tabs[] = $this->newMocksTab($task, $query); + $related_tabs[] = $this->newMentionsTab($task, $query); + + $tab_view = null; + + $related_tabs = array_filter($related_tabs); + if ($related_tabs) { + $tab_group = new PHUITabGroupView(); + foreach ($related_tabs as $tab) { + $tab_group->addTab($tab); + } + + $related_header = id(new PHUIHeaderView()) + ->setHeader(pht('Related Objects')); + + if ($graph_menu) { + $related_header->addActionLink($graph_menu); + } + + $tab_view = id(new PHUIObjectBoxView()) + ->setHeader($related_header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->addTabGroup($tab_group); + } + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn( + array( + $tab_view, + $timeline, + $comment_view, + )) + ->addPropertySection(pht('Description'), $description) + ->addPropertySection(pht('Details'), $details); + + return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) @@ -173,10 +202,7 @@ final class ManiphestTaskDetailController extends ManiphestController { array( $task->getPHID(), )) - ->appendChild( - array( - $view, - )); + ->appendChild($view); } @@ -356,8 +382,6 @@ final class ManiphestTaskDetailController extends ManiphestController { $edge_types = array( ManiphestTaskHasRevisionEdgeType::EDGECONST => pht('Differential Revisions'), - ManiphestTaskHasMockEdgeType::EDGECONST - => pht('Pholio Mocks'), ); $revisions_commits = array(); @@ -435,4 +459,73 @@ final class ManiphestTaskDetailController extends ManiphestController { return $section; } + private function newMocksTab( + ManiphestTask $task, + PhabricatorEdgeQuery $edge_query) { + + $mock_type = ManiphestTaskHasMockEdgeType::EDGECONST; + $mock_phids = $edge_query->getDestinationPHIDs(array(), array($mock_type)); + if (!$mock_phids) { + return null; + } + + $viewer = $this->getViewer(); + $handles = $viewer->loadHandles($mock_phids); + + // TODO: It would be nice to render this as pinboard-style thumbnails, + // similar to "{M123}", instead of a list of links. + + $view = id(new PHUIPropertyListView()) + ->addProperty(pht('Mocks'), $handles->renderList()); + + return id(new PHUITabView()) + ->setName(pht('Mocks')) + ->setKey('mocks') + ->appendChild($view); + } + + private function newMentionsTab( + ManiphestTask $task, + PhabricatorEdgeQuery $edge_query) { + + $in_type = PhabricatorObjectMentionedByObjectEdgeType::EDGECONST; + $out_type = PhabricatorObjectMentionsObjectEdgeType::EDGECONST; + + $in_phids = $edge_query->getDestinationPHIDs(array(), array($in_type)); + $out_phids = $edge_query->getDestinationPHIDs(array(), array($out_type)); + + // Filter out any mentioned users from the list. These are not generally + // very interesting to show in a relationship summary since they usually + // end up as subscribers anyway. + + $user_type = PhabricatorPeopleUserPHIDType::TYPECONST; + foreach ($out_phids as $key => $out_phid) { + if (phid_get_type($out_phid) == $user_type) { + unset($out_phids[$key]); + } + } + + if (!$in_phids && !$out_phids) { + return null; + } + + $viewer = $this->getViewer(); + $view = new PHUIPropertyListView(); + + if ($in_phids) { + $in_handles = $viewer->loadHandles($in_phids); + $view->addProperty(pht('Mentioned In'), $in_handles->renderList()); + } + + if ($out_phids) { + $out_handles = $viewer->loadHandles($out_phids); + $view->addProperty(pht('Mentioned Here'), $out_handles->renderList()); + } + + return id(new PHUITabView()) + ->setName(pht('Mentions')) + ->setKey('mentions') + ->appendChild($view); + } + } From 960c0be6898c5502956cfb63f826ae4245739f1c Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 6 Nov 2016 10:45:58 -0800 Subject: [PATCH 04/31] Fix some issues with Phabricator i18n string extraction Summary: Ref T5267. Fix one minor bug (paths were not being resolved properly) and one minor string issue (missing `%d` in a string). Test Plan: Extracted strings, got a cleaner result. Reviewers: chad Reviewed By: chad Maniphest Tasks: T5267 Differential Revision: https://secure.phabricator.com/D16808 --- .../daemon/workers/storage/PhabricatorWorkerActiveTask.php | 2 +- ...PhabricatorInternationalizationManagementExtractWorkflow.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php index 0fcc78db6e..e1a6ef1500 100644 --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php @@ -156,7 +156,7 @@ final class PhabricatorWorkerActiveTask extends PhabricatorWorkerTask { if ($this->getFailureCount() > $maximum_failures) { throw new PhabricatorWorkerPermanentFailureException( pht( - 'Task % has exceeded the maximum number of failures (%d).', + 'Task %d has exceeded the maximum number of failures (%d).', $this->getID(), $maximum_failures)); } diff --git a/src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php b/src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php index 1da2e16e27..35b2fc9bee 100644 --- a/src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php +++ b/src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php @@ -54,7 +54,7 @@ final class PhabricatorInternationalizationManagementExtractWorkflow } foreach ($libraries as $library) { - $targets[] = Filesystem::resolvePath(dirname($library)).'/'; + $targets[] = Filesystem::resolvePath(dirname($path.'/'.$library)).'/'; } } From 2f93ce4c25be997ce862247a58038f9a196843f2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 6 Nov 2016 09:14:22 -0800 Subject: [PATCH 05/31] Don't show "Limited" or "Test" translations unless an install is in developer mode Summary: Ref T5267. Although translations with very few strings are already put into a "Limited Translations" group, this isn't necessarily clear and was empirically confusing to at least one user, who was surprised that selecting "Spanish" had no UI effect. Instead, hide limited and test translations entirely unless the install is in developer mode. Test Plan: In a non-developer-mode install, viewed translations menu. No longer saw translations with very few strings. Reviewers: chad Reviewed By: chad Maniphest Tasks: T5267 Differential Revision: https://secure.phabricator.com/D16807 --- .../setting/PhabricatorTranslationSetting.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/applications/settings/setting/PhabricatorTranslationSetting.php b/src/applications/settings/setting/PhabricatorTranslationSetting.php index a30ded554d..6c0bec8799 100644 --- a/src/applications/settings/setting/PhabricatorTranslationSetting.php +++ b/src/applications/settings/setting/PhabricatorTranslationSetting.php @@ -32,7 +32,6 @@ final class PhabricatorTranslationSetting } protected function getSelectOptionGroups() { - $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); $locales = PhutilLocale::loadAllLocales(); $group_labels = array( @@ -56,10 +55,6 @@ final class PhabricatorTranslationSetting unset($raw_scope); if ($locale->isSillyLocale()) { - if ($is_serious) { - // Omit silly locales on serious business installs. - continue; - } $groups['silly'][$code] = $name; continue; } @@ -89,6 +84,20 @@ final class PhabricatorTranslationSetting $groups[$type][$code] = $name; } + // Omit silly locales on serious business installs. + $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); + if ($is_serious) { + unset($groups['silly']); + } + + // Omit limited and test translations if Phabricator is not in developer + // mode. + $is_dev = PhabricatorEnv::getEnvConfig('phabricator.developer-mode'); + if (!$is_dev) { + unset($groups['limited']); + unset($groups['test']); + } + $results = array(); foreach ($groups as $key => $group) { $label = $group_labels[$key]; From 7ddd570fa535f0b6f296402f3684dbcf2e12b8c6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 Nov 2016 10:38:36 -0800 Subject: [PATCH 06/31] Provide a standalone `bin/calendar reload ...` workflow for testing/debugging Summary: Ref T11801. This makes testing/debugging a little easier. Also fix some inconsistencies with `importAuthorPHID` handling -- it should be the import's author PHID in all cases, so we update imported events properly. Test Plan: Imported a French holiday with `bin/calendar reload ...`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11801 Differential Revision: https://secure.phabricator.com/D16814 --- src/__phutil_library_map__.php | 2 + .../PhabricatorCalendarImportEngine.php | 4 +- ...icatorCalendarManagementReloadWorkflow.php | 68 +++++++++++++++++++ .../storage/PhabricatorCalendarEvent.php | 9 ++- 4 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 src/applications/calendar/management/PhabricatorCalendarManagementReloadWorkflow.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 1427739390..a8901dadc1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2157,6 +2157,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarImportUpdateLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportUpdateLogType.php', 'PhabricatorCalendarImportViewController' => 'applications/calendar/controller/PhabricatorCalendarImportViewController.php', 'PhabricatorCalendarManagementNotifyWorkflow' => 'applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php', + 'PhabricatorCalendarManagementReloadWorkflow' => 'applications/calendar/management/PhabricatorCalendarManagementReloadWorkflow.php', 'PhabricatorCalendarManagementWorkflow' => 'applications/calendar/management/PhabricatorCalendarManagementWorkflow.php', 'PhabricatorCalendarNotification' => 'applications/calendar/storage/PhabricatorCalendarNotification.php', 'PhabricatorCalendarNotificationEngine' => 'applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php', @@ -7025,6 +7026,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarImportUpdateLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportViewController' => 'PhabricatorCalendarController', 'PhabricatorCalendarManagementNotifyWorkflow' => 'PhabricatorCalendarManagementWorkflow', + 'PhabricatorCalendarManagementReloadWorkflow' => 'PhabricatorCalendarManagementWorkflow', 'PhabricatorCalendarManagementWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorCalendarNotification' => 'PhabricatorCalendarDAO', 'PhabricatorCalendarNotificationEngine' => 'Phobject', diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php index ca1885a4af..35e94634fe 100644 --- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php @@ -199,7 +199,7 @@ abstract class PhabricatorCalendarImportEngine if ($node_map) { $events = id(new PhabricatorCalendarEventQuery()) ->setViewer($viewer) - ->withImportAuthorPHIDs(array($viewer->getPHID())) + ->withImportAuthorPHIDs(array($import->getAuthorPHID())) ->withImportUIDs(array_keys($node_map)) ->execute(); $events = mpull($events, null, 'getImportUID'); @@ -218,7 +218,7 @@ abstract class PhabricatorCalendarImportEngine } $event - ->setImportAuthorPHID($viewer->getPHID()) + ->setImportAuthorPHID($import->getAuthorPHID()) ->setImportSourcePHID($import->getPHID()) ->setImportUID($full_uid) ->attachImportSource($import); diff --git a/src/applications/calendar/management/PhabricatorCalendarManagementReloadWorkflow.php b/src/applications/calendar/management/PhabricatorCalendarManagementReloadWorkflow.php new file mode 100644 index 0000000000..b1020c9e2f --- /dev/null +++ b/src/applications/calendar/management/PhabricatorCalendarManagementReloadWorkflow.php @@ -0,0 +1,68 @@ +setName('reload') + ->setExamples('**reload** [options] __id__ ...') + ->setSynopsis( + pht( + 'Reload event imports from the command line. Useful for '. + 'testing and debugging importers.')) + ->setArguments( + array( + array( + 'name' => 'ids', + 'wildcard' => true, + 'help' => pht('List of import IDs to reload.'), + ), + )); + } + + public function execute(PhutilArgumentParser $args) { + $viewer = $this->getViewer(); + + $ids = $args->getArg('ids'); + if (!$ids) { + throw new PhutilArgumentUsageException( + pht('Specify at least one import ID to reload.')); + } + + $imports = id(new PhabricatorCalendarImportQuery()) + ->setViewer($viewer) + ->withIDs($ids) + ->execute(); + $imports = mpull($imports, null, 'getID'); + foreach ($ids as $id) { + if (empty($imports[$id])) { + throw new PhutilArgumentUsageException( + pht( + 'Unable to load Calendar import with ID "%s".', + $id)); + } + } + + $imports = array_select_keys($imports, $ids); + + foreach ($imports as $import) { + echo tsprintf( + "%s\n", + pht( + 'Importing "%s"...', + $import->getDisplayName())); + + $engine = $import->getEngine(); + + $engine->importEventsFromSource($viewer, $import, false); + } + + echo tsprintf( + "%s\n", + pht('Done.')); + + return 0; + } + +} diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index b36ff628d4..2834228ec9 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -75,9 +75,16 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO $now); list($datetime_start, $datetime_end) = $datetime_defaults; + // When importing events from a context like "bin/calendar reload", we may + // be acting as the omnipotent user. + $host_phid = $actor->getPHID(); + if (!$host_phid) { + $host_phid = $app->getPHID(); + } + return id(new PhabricatorCalendarEvent()) ->setDescription('') - ->setHostPHID($actor->getPHID()) + ->setHostPHID($host_phid) ->setIsCancelled(0) ->setIsAllDay(0) ->setIsStub(0) From 87c4efdb634b0384a5a39b1adc4c355da3ff0625 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 Nov 2016 10:38:42 -0800 Subject: [PATCH 07/31] Probably fix some display issues with all-day events? Summary: Ref T11801. These are pretty fiddly because users expect to see the end time for timed events ("10 AM - 11 AM" is ONE hour long) but not for all-day events ("Nov 2 - Nov 3" is TWO days long!) We also want to store the thing the user actually entered so we don't lose data if they un-all-day the event later. This may take a little more fiddling since it feels a little shaky, but I couldn't break this version immediately. Test Plan: Imported a French holiday, got proper display in the UI. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11801 Differential Revision: https://secure.phabricator.com/D16815 --- .../calendar/storage/PhabricatorCalendarEvent.php | 9 ++++++++- src/view/phui/calendar/PHUICalendarListView.php | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 2834228ec9..58e3cf1dea 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -593,6 +593,9 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO $min_date = $start->newPHPDateTime(); $max_date = $end->newPHPDateTime(); + // Subtract one second since the stored date is exclusive. + $max_date = $max_date->modify('-1 second'); + $min_day = $min_date->format('Y m d'); $max_day = $max_date->format('Y m d'); @@ -849,7 +852,11 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO // If this is an all day event, we move the end date time forward to the // first second of the following day. This is consistent with what users // expect: an all day event from "Nov 1" to "Nov 1" lasts the entire day. - if ($this->getIsAllDay()) { + + // For imported events, the end date is already stored with this + // adjustment. + + if ($this->getIsAllDay() && !$this->isImportedEvent()) { $datetime = $datetime ->newAbsoluteDateTime() ->setHour(0) diff --git a/src/view/phui/calendar/PHUICalendarListView.php b/src/view/phui/calendar/PHUICalendarListView.php index 88bbde351a..be167fc023 100644 --- a/src/view/phui/calendar/PHUICalendarListView.php +++ b/src/view/phui/calendar/PHUICalendarListView.php @@ -198,8 +198,11 @@ final class PHUICalendarListView extends AphrontTagView { $viewer, $event->getEpochEnd())); + $end_date = $end->getDateTime(); + $end_date = $end_date->modify('-1 second'); + $start_date = $start->getDateTime()->format('m d Y'); - $end_date = $end->getDateTime()->format('m d Y'); + $end_date = $end_date->format('m d Y'); if ($event->getIsAllDay()) { if ($start_date == $end_date) { From a8866c0b314f2a6a5414d98bf1aed9a03e676de0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 Nov 2016 11:05:43 -0800 Subject: [PATCH 08/31] In Maniphest, don't render the task graph drawing if we're only showing parents/children Summary: Ref T4788. I thought I implemented this, but actualy didn't. When we're in the "mid-sized" fallback mode (graph has more than 100 nodes, but not more than than 100 parents/children), don't actually draw the graph. It's almost always uninteresting and huge. Instead, this just renders a list of direct parents, then the task, then the direct children, which is pretty straightforward. Test Plan: Set limit to 5, saw mid-sized fallback graph with no actual graph drawing. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4788 Differential Revision: https://secure.phabricator.com/D16816 --- src/infrastructure/graph/ManiphestTaskGraph.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/infrastructure/graph/ManiphestTaskGraph.php b/src/infrastructure/graph/ManiphestTaskGraph.php index 6e92feb241..a306241f85 100644 --- a/src/infrastructure/graph/ManiphestTaskGraph.php +++ b/src/infrastructure/graph/ManiphestTaskGraph.php @@ -128,6 +128,11 @@ final class ManiphestTaskGraph 'graph-status', null, 'wide pri object-link', + )) + ->setColumnVisibility( + array( + true, + !$this->getRenderOnlyAdjacentNodes(), )); } From 729492a8ff855d09ed1fe410f319c0f1106858ed Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 Nov 2016 11:30:02 -0800 Subject: [PATCH 09/31] Allow transactions to specialize their mail headers for diff sections Summary: Ref T7643. When we send mail about a change to a package description, allow it to say "CHANGES TO PACKAGE DESCRIPTION" instead of "EDIT DETAILS". Smooth! Test Plan: {F1909417} Reviewers: chad Reviewed By: chad Maniphest Tasks: T7643 Differential Revision: https://secure.phabricator.com/D16818 --- ...torCalendarEventDescriptionTransaction.php | 4 ++++ ...torOwnersPackageDescriptionTransaction.php | 4 ++++ .../PhabricatorPasteContentTransaction.php | 4 ++++ ...habricatorApplicationTransactionEditor.php | 20 ++++++++++++++++++- .../PhabricatorModularTransactionType.php | 4 ++++ 5 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventDescriptionTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventDescriptionTransaction.php index 343ffa2adf..0de19989a7 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventDescriptionTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventDescriptionTransaction.php @@ -30,6 +30,10 @@ final class PhabricatorCalendarEventDescriptionTransaction return true; } + public function getMailDiffSectionHeader() { + return pht('CHANGES TO EVENT DESCRIPTION'); + } + public function newChangeDetailView() { $viewer = $this->getViewer(); diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php index 8b2effe80c..235262e947 100644 --- a/src/applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php @@ -19,6 +19,10 @@ final class PhabricatorOwnersPackageDescriptionTransaction $this->renderAuthor()); } + public function getMailDiffSectionHeader() { + return pht('CHANGES TO PACKAGE DESCRIPTION'); + } + public function newChangeDetailView() { return id(new PhabricatorApplicationTransactionTextDiffDetailView()) ->setViewer($this->getViewer()) diff --git a/src/applications/paste/xaction/PhabricatorPasteContentTransaction.php b/src/applications/paste/xaction/PhabricatorPasteContentTransaction.php index b04ed61642..0c3fa7e25b 100644 --- a/src/applications/paste/xaction/PhabricatorPasteContentTransaction.php +++ b/src/applications/paste/xaction/PhabricatorPasteContentTransaction.php @@ -104,6 +104,10 @@ final class PhabricatorPasteContentTransaction return true; } + public function getMailDiffSectionHeader() { + return pht('CHANGES TO PASTE CONTENT'); + } + public function newChangeDetailView() { $viewer = $this->getViewer(); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 1a3dd1c89e..10db361837 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -1141,9 +1141,15 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorLiskDAO $object, array $xactions) { + $this->object = $object; + $this->xactions = $xactions; + // Hook for edges or other properties that may need (re-)loading $object = $this->willPublish($object, $xactions); + // The object might have changed, so reassign it. + $this->object = $object; + $messages = array(); if (!$this->getDisableEmail()) { if ($this->shouldSendMail($object, $xactions)) { @@ -2871,12 +2877,24 @@ abstract class PhabricatorApplicationTransactionEditor foreach ($details as $xaction) { $details = $xaction->renderChangeDetailsForMail($body->getViewer()); if ($details !== null) { - $body->addHTMLSection(pht('EDIT DETAILS'), $details); + $label = $this->getMailDiffSectionHeader($xaction); + $body->addHTMLSection($label, $details); } } } + private function getMailDiffSectionHeader($xaction) { + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + return $xtype->getMailDiffSectionHeader(); + } + + return pht('EDIT DETAILS'); + } + /** * @task mail */ diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index 01a02f7fd3..3c9ef6b182 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -67,6 +67,10 @@ abstract class PhabricatorModularTransactionType throw new PhutilMethodNotImplementedException(); } + public function getMailDiffSectionHeader() { + return pht('EDIT DETAILS'); + } + public function newRemarkupChanges() { return array(); } From 6a7dde03ccfb1300339e6474c4a6098cabf01925 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 Nov 2016 12:23:36 -0800 Subject: [PATCH 10/31] On `@username` mentions in remarkup, show the "busy" dot color Summary: Ref T11809. I missed this when adding a "Busy" status. Also the other dot is orange? Just make them all orange for consistency. Test Plan: Viewed `@username` of busy users (orange), away users (red). Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16819 --- .../calendar/storage/PhabricatorCalendarEventInvitee.php | 2 +- .../people/markup/PhabricatorMentionRemarkupRule.php | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/applications/calendar/storage/PhabricatorCalendarEventInvitee.php b/src/applications/calendar/storage/PhabricatorCalendarEventInvitee.php index 34d4c85fcf..b238b614ab 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEventInvitee.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEventInvitee.php @@ -76,7 +76,7 @@ final class PhabricatorCalendarEventInvitee extends PhabricatorCalendarDAO 'name' => pht('Available'), ), self::AVAILABILITY_BUSY => array( - 'color' => 'yellow', + 'color' => 'orange', 'name' => pht('Busy'), ), self::AVAILABILITY_AWAY => array( diff --git a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php index 8049866118..92697f018c 100644 --- a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php +++ b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php @@ -154,7 +154,12 @@ final class PhabricatorMentionRemarkupRule extends PhutilRemarkupRule { $tag->setDotColor(PHUITagView::COLOR_GREY); } else { if ($user->getAwayUntil()) { - $tag->setDotColor(PHUITagView::COLOR_RED); + $away = PhabricatorCalendarEventInvitee::AVAILABILITY_AWAY; + if ($user->getDisplayAvailability() == $away) { + $tag->setDotColor(PHUITagView::COLOR_RED); + } else { + $tag->setDotColor(PHUITagView::COLOR_ORANGE); + } } } } From 3f5109b66864b5ae98af6451e8c4d1c54816859e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 Nov 2016 11:19:08 -0800 Subject: [PATCH 11/31] In prose diff dialogs (like "Show Details" in transactions), show "old", "new" and "diff" tabs Summary: Ref T7643. When you do something like this: - Edit a task description. - Click "Show Details" on the resulting transaction. - Get a prose diff dialog showing the change. ...now add some "Old" and "New" tabs. These are useful for: - reverting to the old text by copy/pasting; - reading just the new/old text if the diff is noisy; - sometimes just nice to have? (This looks a little rough but I didn't want to put a negative margin on tab groups inside dialogs? Not sure what the best fix here is.) Test Plan: {F1909390} Reviewers: chad Reviewed By: chad Maniphest Tasks: T7643 Differential Revision: https://secure.phabricator.com/D16817 --- resources/celerity/map.php | 18 +++++----- ...ApplicationTransactionDetailController.php | 1 + ...plicationTransactionTextDiffDetailView.php | 34 ++++++++++++++++++- webroot/rsrc/css/aphront/dialog-view.css | 4 +++ .../differential/changeset-view.css | 1 + 5 files changed, 48 insertions(+), 10 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index b7f6c9042d..24a1aba29e 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,10 +9,10 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', - 'core.pkg.css' => '46d588e4', + 'core.pkg.css' => '4fc9469e', 'core.pkg.js' => '035325a7', 'darkconsole.pkg.js' => 'e7393ebb', - 'differential.pkg.css' => 'e1d704ce', + 'differential.pkg.css' => 'a4ba74b5', 'differential.pkg.js' => '634399e9', 'diffusion.pkg.css' => '91c5d3a6', 'diffusion.pkg.js' => '84c8f8fd', @@ -21,7 +21,7 @@ return array( 'maniphest.pkg.js' => '949a7498', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', 'rsrc/css/aphront/dark-console.css' => 'f54bf286', - 'rsrc/css/aphront/dialog-view.css' => '593d3f67', + 'rsrc/css/aphront/dialog-view.css' => '84f1e6a6', 'rsrc/css/aphront/lightbox-attachment.css' => '7acac05d', 'rsrc/css/aphront/list-filter-view.css' => '5d6f0526', 'rsrc/css/aphront/multi-column.css' => '84cc6640', @@ -60,7 +60,7 @@ return array( 'rsrc/css/application/dashboard/dashboard.css' => 'bc6f2127', 'rsrc/css/application/diff/inline-comment-summary.css' => '51efda3a', 'rsrc/css/application/differential/add-comment.css' => 'c47f8c40', - 'rsrc/css/application/differential/changeset-view.css' => '9ef7d354', + 'rsrc/css/application/differential/changeset-view.css' => 'b158cc46', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => '5953c28e', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -552,7 +552,7 @@ return array( 'almanac-css' => 'dbb9b3af', 'aphront-bars' => '231ac33c', 'aphront-dark-console-css' => 'f54bf286', - 'aphront-dialog-view-css' => '593d3f67', + 'aphront-dialog-view-css' => '84f1e6a6', 'aphront-list-filter-view-css' => '5d6f0526', 'aphront-multi-column-view-css' => '84cc6640', 'aphront-panel-view-css' => '8427b78d', @@ -576,7 +576,7 @@ return array( 'conpherence-thread-manager' => '358c717b', 'conpherence-transaction-css' => '85129c68', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => '9ef7d354', + 'differential-changeset-view-css' => 'b158cc46', 'differential-core-view-css' => '5b7b8ff4', 'differential-inline-comment-editor' => '64a5550f', 'differential-revision-add-comment-css' => 'c47f8c40', @@ -1733,9 +1733,6 @@ return array( 'javelin-workflow', 'javelin-stratcom', ), - '9ef7d354' => array( - 'phui-inline-comment-view-css', - ), '9f36c42d' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1838,6 +1835,9 @@ return array( 'javelin-util', 'phabricator-shaped-request', ), + 'b158cc46' => array( + 'phui-inline-comment-view-css', + ), 'b1f0ccee' => array( 'javelin-install', 'javelin-dom', diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php index 5849ae0c5e..cfa5935903 100644 --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php @@ -42,6 +42,7 @@ final class PhabricatorApplicationTransactionDetailController return $this->newDialog() ->setTitle(pht('Change Details')) ->setWidth(AphrontDialogView::WIDTH_FORM) + ->setClass('aphront-dialog-tab-group') ->appendChild($details) ->addCancelButton($cancel_uri, $button_text); } diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php index ee4e96fd04..5caf681b32 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php @@ -122,12 +122,44 @@ final class PhabricatorApplicationTransactionTextDiffDetailView } } - return phutil_tag( + $diff_view = phutil_tag( 'div', array( 'class' => 'prose-diff', ), $result); + + $old_view = phutil_tag( + 'div', + array( + 'class' => 'prose-diff', + ), + $this->oldText); + + $new_view = phutil_tag( + 'div', + array( + 'class' => 'prose-diff', + ), + $this->newText); + + return id(new PHUITabGroupView()) + ->addTab( + id(new PHUITabView()) + ->setKey('old') + ->setName(pht('Old')) + ->appendChild($old_view)) + ->addTab( + id(new PHUITabView()) + ->setKey('new') + ->setName(pht('New')) + ->appendChild($new_view)) + ->addTab( + id(new PHUITabView()) + ->setKey('diff') + ->setName(pht('Diff')) + ->appendChild($diff_view)) + ->selectTab('diff'); } private function buildDiff() { diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css index 3122fd7e92..dd746dd5ee 100644 --- a/webroot/rsrc/css/aphront/dialog-view.css +++ b/webroot/rsrc/css/aphront/dialog-view.css @@ -165,3 +165,7 @@ .aphront-dialog-object-list .aphront-dialog-body { padding: 0 12px; } + +.aphront-dialog-tab-group .aphront-dialog-body { + padding: 0 12px; +} diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 9474553f28..c2cb55ea18 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -94,6 +94,7 @@ } .prose-diff { + padding: 12px 0; white-space: pre-wrap; color: {$greytext}; } From 9803674525228e1fe319447d3627da0ce6e77c81 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 8 Nov 2016 08:13:46 -0800 Subject: [PATCH 12/31] Extract variable type information from pht() calls Summary: Ref T5267. When extrating data from `pht()` calls, also extract the argument types and export them into the map so they can be used by consumers. We recognize plurals (`phutil_count()`, `new PhutilNumber`) and genders (`phutil_person()`). We'll need to annotate the codebase for those, since they're currently runtime-only. Test Plan: Rebuilt extraction maps, got data like this (note "number" type annotation). ``` "Scaling pool \"%s\" up to %s daemon(s).": { "uses": [ { "file": "/daemon/PhutilDaemonOverseer.php", "line": 378 } ], "types": [ null, "number" ] }, ``` Reviewers: chad Reviewed By: chad Maniphest Tasks: T5267 Differential Revision: https://secure.phabricator.com/D16823 --- .../people/storage/PhabricatorUser.php | 2 +- ...tionalizationManagementExtractWorkflow.php | 54 ++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index ef2046c21e..e20c68f782 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -572,7 +572,7 @@ final class PhabricatorUser return $this; } - public function getSex() { + public function getGender() { return $this->getUserSetting(PhabricatorPronounSetting::SETTINGKEY); } diff --git a/src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php b/src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php index 35b2fc9bee..8ba4efc8a1 100644 --- a/src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php +++ b/src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php @@ -175,10 +175,49 @@ final class PhabricatorInternationalizationManagementExtractWorkflow try { $string_value = $string_node->evalStatic(); + $args = $params->getChildren(); + $args = array_slice($args, 1); + + $types = array(); + foreach ($args as $child) { + $type = null; + + switch ($child->getTypeName()) { + case 'n_FUNCTION_CALL': + $call = $child->getChildByIndex(0); + if ($call->getTypeName() == 'n_SYMBOL_NAME') { + switch ($call->getConcreteString()) { + case 'phutil_count': + $type = 'number'; + break; + case 'phutil_person': + $type = 'person'; + break; + } + } + break; + case 'n_NEW': + $class = $child->getChildByIndex(0); + if ($class->getTypeName() == 'n_CLASS_NAME') { + switch ($class->getConcreteString()) { + case 'PhutilNumber': + $type = 'number'; + break; + } + } + break; + default: + break; + } + + $types[] = $type; + } + $results[$hash][] = array( 'string' => $string_value, 'file' => Filesystem::readablePath($full_path, $root_path), 'line' => $string_line, + 'types' => $types, ); } catch (Exception $ex) { $messages[] = pht( @@ -207,10 +246,23 @@ final class PhabricatorInternationalizationManagementExtractWorkflow $map = array(); foreach ($strings as $hash => $string_list) { foreach ($string_list as $string_info) { - $map[$string_info['string']]['uses'][] = array( + $string = $string_info['string']; + + $map[$string]['uses'][] = array( 'file' => $string_info['file'], 'line' => $string_info['line'], ); + + if (!isset($map[$string]['types'])) { + $map[$string]['types'] = $string_info['types']; + } else if ($map[$string]['types'] !== $string_info['types']) { + echo tsprintf( + "** %s ** %s\n", + pht('WARNING'), + pht( + 'Inferred types for string "%s" vary across callsites.', + $string_info['string'])); + } } } From afa1bb28604416381cc362d78bad7b935db46428 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 8 Nov 2016 08:42:34 -0800 Subject: [PATCH 13/31] Fix some grammatical gender constants Summary: Ref T5267. I missed these in the variable types conversion. Test Plan: `arc unit --everything` Reviewers: chad Reviewed By: chad Maniphest Tasks: T5267 Differential Revision: https://secure.phabricator.com/D16824 --- .../settings/setting/PhabricatorPronounSetting.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/applications/settings/setting/PhabricatorPronounSetting.php b/src/applications/settings/setting/PhabricatorPronounSetting.php index a1e01d2fd6..2ab30f0ab0 100644 --- a/src/applications/settings/setting/PhabricatorPronounSetting.php +++ b/src/applications/settings/setting/PhabricatorPronounSetting.php @@ -22,7 +22,7 @@ final class PhabricatorPronounSetting } public function getSettingDefaultValue() { - return PhutilPerson::SEX_UNKNOWN; + return PhutilPerson::GENDER_UNKNOWN; } protected function getSelectOptions() { @@ -37,9 +37,9 @@ final class PhabricatorPronounSetting $label_his = pht('%s updated his profile', $username); return array( - PhutilPerson::SEX_UNKNOWN => $label_unknown, - PhutilPerson::SEX_MALE => $label_his, - PhutilPerson::SEX_FEMALE => $label_her, + PhutilPerson::GENDER_UNKNOWN => $label_unknown, + PhutilPerson::GENDER_MASCULINE => $label_his, + PhutilPerson::GENDER_FEMININE => $label_her, ); } From 9a10413dbc884214cd88a7b1d087d6ecb43e4a9d Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 8 Nov 2016 11:31:06 -0800 Subject: [PATCH 14/31] Improve typeahead behavior with mixed-case matches Summary: Ref T8510. We had two issues with mixed-case result sorting, like typing `@joe` to match user `Joe`. - The fallback sort was not normalized properly, so "J" could sort after "j". Instead, normalize values for sorting. - The `prefix_hits` and older `priority_hits` mechanisms were competing destructively. The `prefix_hits` mechanism completely replaces the `priority_hits` mechanism. Instead, use only the `prefix_hits` mechanism. Test Plan: - Copied results for "joe" from WMF. - Hard-coded the controller to return them. - Searched for `@joe`. - After patches, first hit is user "Joe". Reviewers: chad Reviewed By: chad Maniphest Tasks: T8510 Differential Revision: https://secure.phabricator.com/D16826 --- resources/celerity/map.php | 46 +++++++++---------- .../typeahead/source/TypeaheadSource.js | 6 +-- webroot/rsrc/js/core/Prefab.js | 19 +------- 3 files changed, 28 insertions(+), 43 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 24a1aba29e..47590fb389 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,7 +10,7 @@ return array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', 'core.pkg.css' => '4fc9469e', - 'core.pkg.js' => '035325a7', + 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', 'differential.pkg.js' => '634399e9', @@ -264,7 +264,7 @@ return array( 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadCompositeSource.js' => '503e17fd', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js' => '013ffff9', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadPreloadedSource.js' => '54f314a0', - 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js' => 'b25d5444', + 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js' => '0fcf201c', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadStaticSource.js' => '6c0e62fa', 'rsrc/favicons/apple-touch-icon-114x114.png' => '12a24178', 'rsrc/favicons/apple-touch-icon-120x120.png' => '0d1543c7', @@ -487,7 +487,7 @@ return array( 'rsrc/js/core/KeyboardShortcutManager.js' => '4a021c10', 'rsrc/js/core/MultirowRowManager.js' => 'b5d57730', 'rsrc/js/core/Notification.js' => 'ccf1cbf8', - 'rsrc/js/core/Prefab.js' => 'cfd23f37', + 'rsrc/js/core/Prefab.js' => '8d40ae75', 'rsrc/js/core/ShapedRequest.js' => '7cbe244b', 'rsrc/js/core/TextAreaUtils.js' => '320810c8', 'rsrc/js/core/Title.js' => '485aaa6c', @@ -758,7 +758,7 @@ return array( 'javelin-typeahead-normalizer' => 'e6e25838', 'javelin-typeahead-ondemand-source' => '013ffff9', 'javelin-typeahead-preloaded-source' => '54f314a0', - 'javelin-typeahead-source' => 'b25d5444', + 'javelin-typeahead-source' => '0fcf201c', 'javelin-typeahead-static-source' => '6c0e62fa', 'javelin-uri' => 'c989ade3', 'javelin-util' => '93cc50d6', @@ -810,7 +810,7 @@ return array( 'phabricator-notification-menu-css' => '1e055865', 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', - 'phabricator-prefab' => 'cfd23f37', + 'phabricator-prefab' => '8d40ae75', 'phabricator-remarkup-css' => 'cd912f2c', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', @@ -1013,6 +1013,12 @@ return array( 'javelin-install', 'javelin-util', ), + '0fcf201c' => array( + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-typeahead-normalizer', + ), '116cf19b' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1652,6 +1658,18 @@ return array( 'javelin-stratcom', 'javelin-install', ), + '8d40ae75' => array( + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-typeahead', + 'javelin-tokenizer', + 'javelin-typeahead-preloaded-source', + 'javelin-typeahead-ondemand-source', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-util', + ), '8ff5e24c' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1850,12 +1868,6 @@ return array( 'javelin-request', 'phabricator-shaped-request', ), - 'b25d5444' => array( - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-typeahead-normalizer', - ), 'b2b4fbaf' => array( 'javelin-behavior', 'javelin-dom', @@ -1975,18 +1987,6 @@ return array( 'javelin-util', 'phabricator-notification-css', ), - 'cfd23f37' => array( - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-typeahead', - 'javelin-tokenizer', - 'javelin-typeahead-preloaded-source', - 'javelin-typeahead-ondemand-source', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-util', - ), 'd0c516d5' => array( 'javelin-behavior', 'javelin-dom', diff --git a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js index d3dcbdf2b4..91eec515f5 100644 --- a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js +++ b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js @@ -303,9 +303,9 @@ JX.install('TypeaheadSource', { } var default_comparator = function(u, v) { - var key_u = u.sort || u.name; - var key_v = v.sort || v.name; - return key_u.localeCompare(key_v); + var key_u = u.sort || u.name; + var key_v = v.sort || v.name; + return key_u.localeCompare(key_v); }; var filter_handler = this.getFilterHandler() || function(value, list) { diff --git a/webroot/rsrc/js/core/Prefab.js b/webroot/rsrc/js/core/Prefab.js index 7dc2cc35fc..2c3da10102 100644 --- a/webroot/rsrc/js/core/Prefab.js +++ b/webroot/rsrc/js/core/Prefab.js @@ -198,12 +198,6 @@ JX.install('Prefab', { prefix_hits[item.id] = true; } - for (var jj = 0; jj < tokens.length; jj++) { - if (item.name.indexOf(tokens[jj]) === 0) { - priority_hits[item.id] = true; - } - } - if (!item.priority) { continue; } @@ -211,12 +205,6 @@ JX.install('Prefab', { if (config.username && item.priority == config.username) { self_hits[item.id] = true; } - - for (var hh = 0; hh < tokens.length; hh++) { - if (item.priority.substr(0, tokens[hh].length) == tokens[hh]) { - priority_hits[item.id] = true; - } - } } list.sort(function(u, v) { @@ -242,10 +230,6 @@ JX.install('Prefab', { } } - if (priority_hits[u.id] != priority_hits[v.id]) { - return priority_hits[v.id] ? 1 : -1; - } - if (prefix_hits[u.id] != prefix_hits[v.id]) { return prefix_hits[v.id] ? 1 : -1; } @@ -347,7 +331,8 @@ JX.install('Prefab', { color: fields[11], tokenType: fields[12], unique: fields[13] || false, - autocomplete: fields[14] + autocomplete: fields[14], + sort: JX.TypeaheadNormalizer.normalize(fields[0]) }; }, From 999fae524feaf3229aa04cdd9dbb82394fb70258 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 8 Nov 2016 12:21:59 -0800 Subject: [PATCH 15/31] Fix a typo of the word "granularity" Summary: This isn't spelled as well as it could be. Test Plan: O_O Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16827 --- src/docs/user/userguide/multimeter.diviner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/docs/user/userguide/multimeter.diviner b/src/docs/user/userguide/multimeter.diviner index 23e66276d0..6aab740486 100644 --- a/src/docs/user/userguide/multimeter.diviner +++ b/src/docs/user/userguide/multimeter.diviner @@ -20,7 +20,7 @@ To access Multimeter, go to {nav Applications > Multimeter}. By default, Multimeter samples 0.1% of pages. This should be a reasonable rate for most installs, but you can increase or decrease the rate by adjusting `debug.sample-rate`. Increasing the rate (by setting the value to a lower -number, like 100, to sample 1% of pages) will increase the granualrity of the +number, like 100, to sample 1% of pages) will increase the granularity of the data, at a small performance cost. Using Multimeter From d032eea216b4e9e59efb622a86db01d9145ca427 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 8 Nov 2016 13:20:50 -0800 Subject: [PATCH 16/31] Discourage new users from exploring too much Summary: Fixes T11834. Actually adding the step wasn't in the `if (...)` block. Also, typo fix. Test Plan: Saw only one "Explore" on `/guides/`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11834 Differential Revision: https://secure.phabricator.com/D16828 --- .../PhabricatorGuideQuickStartModule.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/applications/guides/module/PhabricatorGuideQuickStartModule.php b/src/applications/guides/module/PhabricatorGuideQuickStartModule.php index 472d8acaa8..c06e320df7 100644 --- a/src/applications/guides/module/PhabricatorGuideQuickStartModule.php +++ b/src/applications/guides/module/PhabricatorGuideQuickStartModule.php @@ -188,18 +188,18 @@ final class PhabricatorGuideQuickStartModule extends PhabricatorGuideModule { $description = pht('Invite the rest of your team to get started on Phabricator.'); } + + $item = id(new PhabricatorGuideItemView()) + ->setTitle($title) + ->setHref($href) + ->setIcon($icon) + ->setIconBackground($icon_bg) + ->setDescription($description); + $guide_items->addItem($item); } - $item = id(new PhabricatorGuideItemView()) - ->setTitle($title) - ->setHref($href) - ->setIcon($icon) - ->setIconBackground($icon_bg) - ->setDescription($description); - $guide_items->addItem($item); - $intro = pht( - 'If your new to Phabricator, these optional steps can help you learn '. + 'If you\'re new to Phabricator, these optional steps can help you learn '. 'the basics. Conceptually, Phabricator is structured as a graph, and '. 'repositories, tasks, and projects are all independent from each other. '. 'Feel free to set up Phabricator for how you work best, and explore '. From 1747b4d31854da53691037668a77c131816542e6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 8 Nov 2016 15:14:00 -0800 Subject: [PATCH 17/31] Use "book" as the "Help" icon in the Remarkup toolbar Summary: See D16811. I missed this while grepping because the other icon has two aliases (`life-buoy`, `life-ring`) and we were using one of each. Test Plan: {F1912167} Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16829 --- src/view/form/control/PhabricatorRemarkupControl.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/view/form/control/PhabricatorRemarkupControl.php b/src/view/form/control/PhabricatorRemarkupControl.php index 5fc275e063..a2fb7fc7a8 100644 --- a/src/view/form/control/PhabricatorRemarkupControl.php +++ b/src/view/form/control/PhabricatorRemarkupControl.php @@ -158,7 +158,7 @@ final class PhabricatorRemarkupControl extends AphrontFormTextAreaControl { 'align' => 'right', ); - $actions['fa-life-bouy'] = array( + $actions['fa-book'] = array( 'tip' => pht('Help'), 'align' => 'right', 'href' => PhabricatorEnv::getDoclink('Remarkup Reference'), From d78802f3ab6572400b799f4aa1d7b21e1885e350 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 9 Nov 2016 10:33:05 -0800 Subject: [PATCH 18/31] Redesign Comment Box Summary: Redesign the action comment box for better use in two column, mobile, nuance. Test Plan: Test in mobile / desktop / tablet, adding and removing actions. Actionless comment boxes, etc. Reviewers: epriestley Reviewed By: epriestley Subscribers: cspeckmim, Korvin Differential Revision: https://secure.phabricator.com/D16811 --- resources/celerity/map.php | 46 +++-- ...catorApplicationTransactionCommentView.php | 66 ++++-- webroot/rsrc/css/core/remarkup.css | 6 +- webroot/rsrc/css/phui/phui-comment-form.css | 194 ++++++++++++++++++ webroot/rsrc/css/phui/phui-form-view.css | 6 +- .../transactions/behavior-comment-actions.js | 3 +- webroot/rsrc/js/phuix/PHUIXFormControl.js | 8 +- 7 files changed, 281 insertions(+), 48 deletions(-) create mode 100644 webroot/rsrc/css/phui/phui-comment-form.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 47590fb389..2462868c0f 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', - 'core.pkg.css' => '4fc9469e', + 'core.pkg.css' => 'c99a9eb4', 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', @@ -109,7 +109,7 @@ return array( 'rsrc/css/application/tokens/tokens.css' => '3d0f239e', 'rsrc/css/application/uiexample/example.css' => '528b19de', 'rsrc/css/core/core.css' => 'd0801452', - 'rsrc/css/core/remarkup.css' => 'cd912f2c', + 'rsrc/css/core/remarkup.css' => '04ab9f17', 'rsrc/css/core/syntax.css' => '769d3498', 'rsrc/css/core/z-index.css' => 'd1270942', 'rsrc/css/diviner/diviner-shared.css' => 'aa3656aa', @@ -132,6 +132,7 @@ return array( 'rsrc/css/phui/phui-button.css' => '4a5fbe3d', 'rsrc/css/phui/phui-chart.css' => '6bf6f78e', 'rsrc/css/phui/phui-cms.css' => 'be43c8a8', + 'rsrc/css/phui/phui-comment-form.css' => '103cdc49', 'rsrc/css/phui/phui-crumbs-view.css' => '195ac419', 'rsrc/css/phui/phui-curtain-view.css' => '947bf1a4', 'rsrc/css/phui/phui-document-pro.css' => 'ca1fed81', @@ -139,7 +140,7 @@ return array( 'rsrc/css/phui/phui-document.css' => 'c32e8dec', 'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9', 'rsrc/css/phui/phui-fontkit.css' => '9cda225e', - 'rsrc/css/phui/phui-form-view.css' => '9e22b190', + 'rsrc/css/phui/phui-form-view.css' => 'b5bfd17a', 'rsrc/css/phui/phui-form.css' => 'aac1d51d', 'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f', 'rsrc/css/phui/phui-header-view.css' => '06385974', @@ -456,7 +457,7 @@ return array( 'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e2e0a072', 'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08', 'rsrc/js/application/slowvote/behavior-slowvote-embed.js' => '887ad43f', - 'rsrc/js/application/transactions/behavior-comment-actions.js' => '0300eae6', + 'rsrc/js/application/transactions/behavior-comment-actions.js' => '1be09f3f', 'rsrc/js/application/transactions/behavior-reorder-configs.js' => 'd7a74243', 'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96', 'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '94c65b72', @@ -545,7 +546,7 @@ return array( 'rsrc/js/phuix/PHUIXActionView.js' => '8cf6d262', 'rsrc/js/phuix/PHUIXAutocomplete.js' => '6d86ce8b', 'rsrc/js/phuix/PHUIXDropdownMenu.js' => '82e270da', - 'rsrc/js/phuix/PHUIXFormControl.js' => 'e15869a8', + 'rsrc/js/phuix/PHUIXFormControl.js' => '301b7812', 'rsrc/js/phuix/PHUIXIconView.js' => 'bff6884b', ), 'symbols' => array( @@ -612,7 +613,7 @@ return array( 'javelin-behavior-bulk-job-reload' => 'edf8a145', 'javelin-behavior-calendar-month-view' => 'fe33e256', 'javelin-behavior-choose-control' => '327a00d1', - 'javelin-behavior-comment-actions' => '0300eae6', + 'javelin-behavior-comment-actions' => '1be09f3f', 'javelin-behavior-config-reorder-fields' => 'b6993408', 'javelin-behavior-conpherence-menu' => '7524fcfa', 'javelin-behavior-conpherence-participant-pane' => '8604caa8', @@ -811,7 +812,7 @@ return array( 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', 'phabricator-prefab' => '8d40ae75', - 'phabricator-remarkup-css' => 'cd912f2c', + 'phabricator-remarkup-css' => '04ab9f17', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', @@ -854,6 +855,7 @@ return array( 'phui-calendar-month-css' => '8e10e92c', 'phui-chart-css' => '6bf6f78e', 'phui-cms-css' => 'be43c8a8', + 'phui-comment-form-css' => '103cdc49', 'phui-crumbs-view-css' => '195ac419', 'phui-curtain-view-css' => '947bf1a4', 'phui-document-summary-view-css' => '9ca48bdf', @@ -863,7 +865,7 @@ return array( 'phui-font-icon-base-css' => '870a7360', 'phui-fontkit-css' => '9cda225e', 'phui-form-css' => 'aac1d51d', - 'phui-form-view-css' => '9e22b190', + 'phui-form-view-css' => 'b5bfd17a', 'phui-head-thing-view-css' => 'fd311e5f', 'phui-header-view-css' => '06385974', 'phui-hovercard' => '1bd28176', @@ -898,7 +900,7 @@ return array( 'phuix-action-view' => '8cf6d262', 'phuix-autocomplete' => '6d86ce8b', 'phuix-dropdown-menu' => '82e270da', - 'phuix-form-control-view' => 'e15869a8', + 'phuix-form-control-view' => '301b7812', 'phuix-icon-view' => 'bff6884b', 'policy-css' => '957ea14c', 'policy-edit-css' => '815c66f7', @@ -941,15 +943,6 @@ return array( 'javelin-dom', 'phabricator-keyboard-shortcut', ), - '0300eae6' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-workflow', - 'javelin-dom', - 'phuix-form-control-view', - 'phuix-icon-view', - 'javelin-behavior-phabricator-gesture', - ), '05270951' => array( 'javelin-util', 'javelin-magical-init', @@ -1081,6 +1074,15 @@ return array( 'javelin-request', 'javelin-uri', ), + '1be09f3f' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-workflow', + 'javelin-dom', + 'phuix-form-control-view', + 'phuix-icon-view', + 'javelin-behavior-phabricator-gesture', + ), '1def2711' => array( 'javelin-install', 'javelin-dom', @@ -1148,6 +1150,10 @@ return array( '2ee659ce' => array( 'javelin-install', ), + '301b7812' => array( + 'javelin-install', + 'javelin-dom', + ), '320810c8' => array( 'javelin-install', 'javelin-dom', @@ -2063,10 +2069,6 @@ return array( 'javelin-dom', 'phabricator-prefab', ), - 'e15869a8' => array( - 'javelin-install', - 'javelin-dom', - ), 'e1d25dfb' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php index 9741e15124..d6963554ce 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -150,7 +150,6 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { ->setQueryParam('next', (string)$this->getRequestURI()); return id(new PHUIObjectBoxView()) ->setFlush(true) - ->setHeaderText(pht('Add Comment')) ->appendChild( javelin_tag( 'a', @@ -183,9 +182,25 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { )); } + require_celerity_resource('phui-comment-form-css'); + $image_uri = $user->getProfileImageURI(); + $image = phutil_tag( + 'div', + array( + 'style' => 'background-image: url('.$image_uri.')', + 'class' => 'phui-comment-image', + )); + $wedge = phutil_tag( + 'div', + array( + 'class' => 'phui-timeline-wedge', + ), + ''); $comment_box = id(new PHUIObjectBoxView()) ->setFlush(true) - ->setHeaderText($this->headerText) + ->addClass('phui-comment-form-view') + ->appendChild($image) + ->appendChild($wedge) ->appendChild($comment); return array($comment_box, $preview); @@ -287,20 +302,31 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { 'id' => $input_id, ))); - $form->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Actions')) - ->setID($action_id) - ->setOptions($options)); + $invisi_bar = phutil_tag( + 'div', + array( + 'id' => $place_id, + 'class' => 'phui-comment-control-stack', + )); - // This is an empty placeholder node so we know where to insert the - // new actions. - $form->appendChild( - phutil_tag( - 'div', - array( - 'id' => $place_id, - ))); + $action_select = id(new AphrontFormSelectControl()) + ->addClass('phui-comment-fullwidth-control') + ->addClass('phui-comment-action-control') + ->setID($action_id) + ->setOptions($options); + + $action_bar = phutil_tag( + 'div', + array( + 'class' => 'phui-comment-action-bar grouped', + ), + array( + $action_select, + )); + + $form->appendChild($action_bar); + $form->appendChild($invisi_bar); + $form->addClass('phui-comment-has-actions'); Javelin::initBehavior( 'comment-actions', @@ -318,16 +344,24 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { )); } + $submit_button = id(new AphrontFormSubmitControl()) + ->addClass('phui-comment-fullwidth-control') + ->addClass('phui-comment-submit-control') + ->setValue($this->getSubmitButtonName()); + $form ->appendChild( id(new PhabricatorRemarkupControl()) ->setID($this->getCommentID()) + ->addClass('phui-comment-fullwidth-control') + ->addClass('phui-comment-textarea-control') ->setName('comment') - ->setLabel(pht('Comment')) ->setUser($this->getUser()) ->setValue($draft_comment)) ->appendChild( id(new AphrontFormSubmitControl()) + ->addClass('phui-comment-fullwidth-control') + ->addClass('phui-comment-submit-control') ->setValue($this->getSubmitButtonName())); return $form; diff --git a/webroot/rsrc/css/core/remarkup.css b/webroot/rsrc/css/core/remarkup.css index a2644d2c37..48d06d4d5f 100644 --- a/webroot/rsrc/css/core/remarkup.css +++ b/webroot/rsrc/css/core/remarkup.css @@ -487,12 +487,8 @@ var.remarkup-assist-textarea { float: left; } -.remarkup-assist-button:hover { - background-color: rgba(100,100,100,.15); -} - .remarkup-assist-button:hover .phui-icon-view.phui-font-fa { - color: {$darkbluetext}; + color: {$sky}; } .remarkup-assist-button:active { diff --git a/webroot/rsrc/css/phui/phui-comment-form.css b/webroot/rsrc/css/phui/phui-comment-form.css new file mode 100644 index 0000000000..1fca450d97 --- /dev/null +++ b/webroot/rsrc/css/phui/phui-comment-form.css @@ -0,0 +1,194 @@ +/** + * @provides phui-comment-form-css + */ + +body .phui-box.phui-object-box.phui-comment-form-view { + background-color: #fff; + margin-left: 62px; + position: relative; +} + +body.device .phui-box.phui-object-box.phui-comment-form-view { + margin-left: 0; +} + +.phui-comment-form-view.phui-object-box { + padding: 0; +} + +.phui-comment-form-view .phui-form-view { + padding: 0; +} + +.phui-comment-form-view .phui-comment-image { + background-repeat: no-repeat; + position: absolute; + border-radius: 3px; + background-size: 100%; + display: block; +} + +.device-desktop .phui-comment-form-view .phui-comment-image { + width: 50px; + height: 50px; + top: 0px; + left: -62px; +} + +.phui-comment-form-view .phui-timeline-wedge { + top: 26px; +} + +.phui-comment-fullwidth-control .aphront-form-input { + margin: 0; + width: auto; +} + +.phui-comment-form-view .remarkup-assist-bar { + height: 32px; + border-color: {$lightblueborder}; + border-top-left-radius: 3px; + border-top-right-radius: 3px; + padding: 0 4px; +} + +.device-phone .phui-comment-form-view .remarkup-assist-bar, +.device-phone .phui-comment-form-view .aphront-form-input + .remarkup-assist-textarea { + border-radius: 0; +} + +.phui-comment-form-view .remarkup-assist-button { + margin-top: 2px; + padding: 4px 5px; + border-radius: 3px; +} + +.phui-comment-form-view .remarkup-assist-separator { + height: 18px; + margin: 7px 6px; +} + +.phui-comment-form-view .aphront-form-input .remarkup-assist-textarea { + border-color: {$lightblueborder}; + border-top: 1px solid {$thinblueborder}; + padding: 8px; + border-bottom-left-radius: 3px; + border-bottom-right-radius: 3px; + height: 10em; + background-color: rgba({$alphablue},.02); +} + +.phui-comment-form-view .aphront-form-input .remarkup-assist-textarea:focus { + background-color: #fff; +} + +.device-phone .phui-comment-form-view .aphront-form-input + .remarkup-assist-textarea { + height: 8em; +} + +.phui-comment-form-view .phui-comment-textarea-control { + padding: 16px 16px 8px 16px; +} + +.phui-comment-form-view .phui-comment-has-actions + .phui-comment-textarea-control { + padding-top: 0; +} + +.device-phone .phui-comment-form-view .phui-comment-textarea-control { + padding: 0; + margin: -1px; +} + +.phui-comment-action { + background-color: rgba(239, 243, 252, .75); + border-radius: 3px; + margin: 0px 16px 8px; + padding: 6px; +} + +.phui-comment-action + .phui-comment-action { + margin-top: -4px; +} + +.device-phone .phui-comment-action { + margin: 8px; +} + +.phui-comment-form-view .phui-comment-action-bar { + border-bottom: 1px solid {$thinblueborder}; + background-color: rgba(239, 243, 252, .75); + padding: 2px 12px 4px 12px; + margin-bottom: 16px; +} + +.device-phone .phui-comment-form-view .phui-comment-action-bar { + margin: 0; + padding: 8px; +} + +.phui-comment-form-view .phui-comment-submit-control { + padding: 0 16px 12px; +} + +.device-phone .phui-comment-form-view .phui-comment-submit-control { + margin: 0px; + padding: 6px 8px 8px; +} + +.phui-comment-form-view .phui-comment-action-control { + float: left; +} + +.device-phone .phui-comment-form-view .phui-comment-action-control { + float: none; + width: 100%; + margin: 0; + padding: 0; +} + +.phui-comment-form-view .phui-comment-action-icon { + float: left; + font-size: 20px; + color: {$lightbluetext}; + height: 40px; + line-height: 40px; + margin: 0 4px; +} + +.device-phone .phui-comment-form-view .phui-comment-action-icon { + display: none; +} + +/* h8rs gonna h8 */ +.phui-comment-form-view select { + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; + + background: #fff url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAUCAMAAACzvE1FAAAADFBMVEUzMzMzMzMzMzMzMzMKAG/3AAAAA3RSTlMAf4C/aSLHAAAAPElEQVR42q3NMQ4AIAgEQTn//2cLdRKppSGzBYwzVXvznNWs8C58CiussPJj8h6NwgorrKRdTvuV9v16Afn0AYFOB7aYAAAAAElFTkSuQmCC") no-repeat right 8px center; + background-size: 8px 10px; + border-radius: 3px; + color: {$darkbluetext}; + height: 28px; + position: relative; + padding: 0 8px; + width: 180px; +} + +.device-phone .aphront-form-control-submit button { + width: 100%; +} + +.phui-comment-form-view .phui-form-view label.aphront-form-label, +.phui-comment-form-view .phui-form-view .aphront-form-error { + height: 26px; + line-height: 26px; + padding: 0; +} + +.phui-comment-form-view .aphront-form-error .phui-icon-view { + padding: 4px; +} diff --git a/webroot/rsrc/css/phui/phui-form-view.css b/webroot/rsrc/css/phui/phui-form-view.css index 19f74c975a..91e7b64dcf 100644 --- a/webroot/rsrc/css/phui/phui-form-view.css +++ b/webroot/rsrc/css/phui/phui-form-view.css @@ -476,7 +476,6 @@ properly, and submit values. */ display: none; } - .login-to-comment { margin: 12px; } @@ -533,10 +532,11 @@ properly, and submit values. */ } .aphront-form-error .phui-icon-view { + float: right; color: {$lightgreyborder}; - font-size: 16px; + font-size: 20px; } .device-desktop .aphront-form-error .phui-icon-view:hover { - color: {$darkgreyborder}; + color: {$red}; } diff --git a/webroot/rsrc/js/application/transactions/behavior-comment-actions.js b/webroot/rsrc/js/application/transactions/behavior-comment-actions.js index a28a6b7ca2..89ca048fe3 100644 --- a/webroot/rsrc/js/application/transactions/behavior-comment-actions.js +++ b/webroot/rsrc/js/application/transactions/behavior-comment-actions.js @@ -58,7 +58,8 @@ JX.behavior('comment-actions', function(config) { var control = new JX.PHUIXFormControl() .setLabel(action.label) .setError(remove) - .setControl(action.type, action.spec); + .setControl(action.type, action.spec) + .setClass('phui-comment-action'); var node = control.getNode(); JX.Stratcom.addSigil(node, 'touchable'); diff --git a/webroot/rsrc/js/phuix/PHUIXFormControl.js b/webroot/rsrc/js/phuix/PHUIXFormControl.js index 5a53bafd7b..264069248f 100644 --- a/webroot/rsrc/js/phuix/PHUIXFormControl.js +++ b/webroot/rsrc/js/phuix/PHUIXFormControl.js @@ -11,6 +11,7 @@ JX.install('PHUIXFormControl', { _labelNode: null, _errorNode: null, _inputNode: null, + _className: null, _valueSetCallback: null, _valueGetCallback: null, @@ -24,6 +25,11 @@ JX.install('PHUIXFormControl', { return this; }, + setClass: function(className) { + this._className = className; + return this; + }, + setControl: function(type, spec) { var node = this._getInputNode(); @@ -67,7 +73,7 @@ JX.install('PHUIXFormControl', { if (!this._node) { var attrs = { - className: 'aphront-form-control grouped' + className: 'aphront-form-control ' + this._className + ' grouped' }; var content = [ From 4811e6e7c1226d3c51649f47c2e7b49beb68aa19 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 9 Nov 2016 10:02:25 -0800 Subject: [PATCH 19/31] Require several advanced postgraduate degrees to understand object policies Summary: Fixes T11836. See some prior discussion in T8376#120613. The policy hint in headers in the UI is not exhaustive, and can not reasonably be exhaustive. For example, on a revision, it may say "All Users", but really mean "All users who can see the space this object is in and the repository it belongs to, plus the revision author and reviewers". These rules are explained if you click (and, often, in the documentation), but "All Users" is still at least somewhat misleading. I don't think there's any perfect solution here that balances the needs of both new and experienced users perfectly, but this change tries to do a bit better about avoiding cases where we say something very open (like "All Users") when the real policy is not very open. Specifically, I've made these changes to the header: - Spaces are now listed in the tag, so it will say `(S3 > All Users)` instead of `(All Users)`. They're already listed in the header, this just makes it more explicit that Spaces are a policy container and part of the view policy. - Extended policy objects are now listed in the tag, so it will say `(S3 > rARC > All Users)` for a revision in the Arcanist repository which is also in Space 3. - Objects can now provide a "Policy Codex", which is an object that represents a rulebook of more sophisticated policy descriptions. This codex can replace the tag with something else. - Imported calendar events now say "Uses Import Policy" instead of, e.g., "All Users". I've made these changes to the policy dialog: - Split it into more visually separate sections. - Added an explicit section for extended policies ("You must also have access to these other objects: ..."). - Broken the object policy rules into a "Special Rules" section (for rules like "you can only see a revision if you can see the repository it is part of") and an "Object Policy" section (for the actual object policy). - Tried to make it a little more readable? - The new policy dialogs are great to curl up with in front of a fire with a nice cup of cocoa. I've made these changes to infrastructure: - Implementing `PhabricatorPolicyInterface` no longer requires you to implement `describeAutomaticCapability()`. - Instead, implement `PhabricatorPolicyCodexInterface` and return a `PhabricatorPolicyCodex` object. - This "codex" is a policy rulebook which can set all the policy icons, labels, colors, rules, etc., to properly explain complex policies. - Broadly, the old method was usually either not useful (most objects have no special rules) or not powerful enough (objects with special rules often need to do more in order to explain them). Test Plan: {F1912860} {F1912861} {F1912862} {F1912863} Reviewers: chad Reviewed By: chad Subscribers: avivey Maniphest Tasks: T11836 Differential Revision: https://secure.phabricator.com/D16830 --- src/__phutil_library_map__.php | 10 + .../PhabricatorCalendarEventPolicyCodex.php | 80 +++++ .../storage/PhabricatorCalendarEvent.php | 19 +- .../storage/DifferentialRevision.php | 8 - .../policy/codex/PhabricatorPolicyCodex.php | 106 +++++++ .../codex/PhabricatorPolicyCodexInterface.php | 18 ++ .../PhabricatorPolicyCodexRuleDescription.php | 37 +++ .../PhabricatorPolicyExplainController.php | 295 +++++++++++++----- .../interface/PhabricatorPolicyInterface.php | 32 -- .../policy/storage/PhabricatorPolicy.php | 4 +- .../policy/view/PHUIPolicySectionView.php | 142 +++++++++ src/view/phui/PHUIHeaderView.php | 36 ++- webroot/rsrc/css/aphront/dialog-view.css | 4 + webroot/rsrc/css/phui/phui-header-view.css | 62 ++++ 14 files changed, 723 insertions(+), 130 deletions(-) create mode 100644 src/applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php create mode 100644 src/applications/policy/codex/PhabricatorPolicyCodex.php create mode 100644 src/applications/policy/codex/PhabricatorPolicyCodexInterface.php create mode 100644 src/applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php create mode 100644 src/applications/policy/view/PHUIPolicySectionView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a8901dadc1..071eef10e3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1668,6 +1668,7 @@ phutil_register_library_map(array( 'PHUIPagerView' => 'view/phui/PHUIPagerView.php', 'PHUIPinboardItemView' => 'view/phui/PHUIPinboardItemView.php', 'PHUIPinboardView' => 'view/phui/PHUIPinboardView.php', + 'PHUIPolicySectionView' => 'applications/policy/view/PHUIPolicySectionView.php', 'PHUIPropertyGroupView' => 'view/phui/PHUIPropertyGroupView.php', 'PHUIPropertyListExample' => 'applications/uiexample/examples/PHUIPropertyListExample.php', 'PHUIPropertyListView' => 'view/phui/PHUIPropertyListView.php', @@ -2071,6 +2072,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventNameTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventNameTransaction.php', 'PhabricatorCalendarEventNotificationView' => 'applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php', 'PhabricatorCalendarEventPHIDType' => 'applications/calendar/phid/PhabricatorCalendarEventPHIDType.php', + 'PhabricatorCalendarEventPolicyCodex' => 'applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php', 'PhabricatorCalendarEventQuery' => 'applications/calendar/query/PhabricatorCalendarEventQuery.php', 'PhabricatorCalendarEventRSVPEmailCommand' => 'applications/calendar/command/PhabricatorCalendarEventRSVPEmailCommand.php', 'PhabricatorCalendarEventRecurringTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php', @@ -3311,6 +3313,9 @@ phutil_register_library_map(array( 'PhabricatorPolicyCanViewCapability' => 'applications/policy/capability/PhabricatorPolicyCanViewCapability.php', 'PhabricatorPolicyCapability' => 'applications/policy/capability/PhabricatorPolicyCapability.php', 'PhabricatorPolicyCapabilityTestCase' => 'applications/policy/capability/__tests__/PhabricatorPolicyCapabilityTestCase.php', + 'PhabricatorPolicyCodex' => 'applications/policy/codex/PhabricatorPolicyCodex.php', + 'PhabricatorPolicyCodexInterface' => 'applications/policy/codex/PhabricatorPolicyCodexInterface.php', + 'PhabricatorPolicyCodexRuleDescription' => 'applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php', 'PhabricatorPolicyConfigOptions' => 'applications/policy/config/PhabricatorPolicyConfigOptions.php', 'PhabricatorPolicyConstants' => 'applications/policy/constants/PhabricatorPolicyConstants.php', 'PhabricatorPolicyController' => 'applications/policy/controller/PhabricatorPolicyController.php', @@ -6447,6 +6452,7 @@ phutil_register_library_map(array( 'PHUIPagerView' => 'AphrontView', 'PHUIPinboardItemView' => 'AphrontView', 'PHUIPinboardView' => 'AphrontView', + 'PHUIPolicySectionView' => 'AphrontTagView', 'PHUIPropertyGroupView' => 'AphrontTagView', 'PHUIPropertyListExample' => 'PhabricatorUIExample', 'PHUIPropertyListView' => 'AphrontView', @@ -6870,6 +6876,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarDAO', 'PhabricatorPolicyInterface', 'PhabricatorExtendedPolicyInterface', + 'PhabricatorPolicyCodexInterface', 'PhabricatorProjectInterface', 'PhabricatorMarkupInterface', 'PhabricatorApplicationTransactionInterface', @@ -6923,6 +6930,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventNameTransaction' => 'PhabricatorCalendarEventTransactionType', 'PhabricatorCalendarEventNotificationView' => 'Phobject', 'PhabricatorCalendarEventPHIDType' => 'PhabricatorPHIDType', + 'PhabricatorCalendarEventPolicyCodex' => 'PhabricatorPolicyCodex', 'PhabricatorCalendarEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorCalendarEventRSVPEmailCommand' => 'PhabricatorCalendarEventEmailCommand', 'PhabricatorCalendarEventRecurringTransaction' => 'PhabricatorCalendarEventTransactionType', @@ -8361,6 +8369,8 @@ phutil_register_library_map(array( 'PhabricatorPolicyCanViewCapability' => 'PhabricatorPolicyCapability', 'PhabricatorPolicyCapability' => 'Phobject', 'PhabricatorPolicyCapabilityTestCase' => 'PhabricatorTestCase', + 'PhabricatorPolicyCodex' => 'Phobject', + 'PhabricatorPolicyCodexRuleDescription' => 'Phobject', 'PhabricatorPolicyConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPolicyConstants' => 'Phobject', 'PhabricatorPolicyController' => 'PhabricatorController', diff --git a/src/applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php b/src/applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php new file mode 100644 index 0000000000..97e21d901e --- /dev/null +++ b/src/applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php @@ -0,0 +1,80 @@ +getObject(); + + if (!$object->isImportedEvent()) { + return null; + } + + return pht('Uses Import Policy'); + } + + public function getPolicyIcon() { + $object = $this->getObject(); + + if (!$object->isImportedEvent()) { + return null; + } + + return 'fa-download'; + } + + public function getPolicyTagClasses() { + $object = $this->getObject(); + + if (!$object->isImportedEvent()) { + return array(); + } + + return array( + 'policy-adjusted-special', + ); + } + + public function getPolicySpecialRuleDescriptions() { + $object = $this->getObject(); + + $rules = array(); + $rules[] = $this->newRule() + ->setDescription( + pht('The host of an event can always view and edit it.')); + + $rules[] = $this->newRule() + ->setCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + )) + ->setDescription( + pht('Users who are invited to an event can always view it.')); + + + $rules[] = $this->newRule() + ->setCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + )) + ->setIsActive($object->isImportedEvent()) + ->setDescription( + pht( + 'Imported events can only be viewed by users who can view '. + 'the import source.')); + + $rules[] = $this->newRule() + ->setCapabilities( + array( + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->setIsActive($object->isImportedEvent()) + ->setDescription( + pht( + 'Imported events can not be edited in Phabricator.')); + + return $rules; + } + + +} diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 58e3cf1dea..6d76ad6816 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -4,6 +4,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO implements PhabricatorPolicyInterface, PhabricatorExtendedPolicyInterface, + PhabricatorPolicyCodexInterface, PhabricatorProjectInterface, PhabricatorMarkupInterface, PhabricatorApplicationTransactionInterface, @@ -1217,18 +1218,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return false; } - public function describeAutomaticCapability($capability) { - if ($this->isImportedEvent()) { - return pht( - 'Events imported from external sources can not be edited in '. - 'Phabricator.'); - } - - return pht( - 'The host of an event can always view and edit it. Users who are '. - 'invited to an event can always view it.'); - } - /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ @@ -1251,6 +1240,12 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $extended; } +/* -( PhabricatorPolicyCodexInterface )------------------------------------ */ + + public function newPolicyCodex() { + return new PhabricatorCalendarEventPolicyCodex(); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 97d5a5e581..47443932e3 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -370,14 +370,6 @@ final class DifferentialRevision extends DifferentialDAO switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: - // NOTE: In Differential, an automatic capability on a revision (being - // an author) is sufficient to view it, even if you can not see the - // repository the revision belongs to. We can bail out early in this - // case. - if ($this->hasAutomaticCapability($capability, $viewer)) { - break; - } - $repository_phid = $this->getRepositoryPHID(); $repository = $this->getRepository(); diff --git a/src/applications/policy/codex/PhabricatorPolicyCodex.php b/src/applications/policy/codex/PhabricatorPolicyCodex.php new file mode 100644 index 0000000000..43a1cc5a1d --- /dev/null +++ b/src/applications/policy/codex/PhabricatorPolicyCodex.php @@ -0,0 +1,106 @@ +viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + + final public function setObject(PhabricatorPolicyCodexInterface $object) { + $this->object = $object; + return $this; + } + + final public function getObject() { + return $this->object; + } + + final public function setCapability($capability) { + $this->capability = $capability; + return $this; + } + + final public function getCapability() { + return $this->capability; + } + + final public function setPolicy(PhabricatorPolicy $policy) { + $this->policy = $policy; + return $this; + } + + final public function getPolicy() { + return $this->policy; + } + + final public static function newFromObject( + PhabricatorPolicyCodexInterface $object, + PhabricatorUser $viewer) { + + if (!($object instanceof PhabricatorPolicyInterface)) { + throw new Exception( + pht( + 'Object (of class "%s") implements interface "%s", but must also '. + 'implement interface "%s".', + get_class($object), + 'PhabricatorPolicyCodexInterface', + 'PhabricatorPolicyInterface')); + } + + $codex = $object->newPolicyCodex(); + if (!($codex instanceof PhabricatorPolicyCodex)) { + throw new Exception( + pht( + 'Object (of class "%s") implements interface "%s", but defines '. + 'method "%s" incorrectly: this method must return an object of '. + 'class "%s".', + get_class($object), + 'PhabricatorPolicyCodexInterface', + 'newPolicyCodex()', + __CLASS__)); + } + + $codex + ->setObject($object) + ->setViewer($viewer); + + return $codex; + } + +} diff --git a/src/applications/policy/codex/PhabricatorPolicyCodexInterface.php b/src/applications/policy/codex/PhabricatorPolicyCodexInterface.php new file mode 100644 index 0000000000..43f1e4c445 --- /dev/null +++ b/src/applications/policy/codex/PhabricatorPolicyCodexInterface.php @@ -0,0 +1,18 @@ +>PolicyCodex(); + } + +*/ diff --git a/src/applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php b/src/applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php new file mode 100644 index 0000000000..edc216fc66 --- /dev/null +++ b/src/applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php @@ -0,0 +1,37 @@ +description = $description; + return $this; + } + + public function getDescription() { + return $this->description; + } + + public function setCapabilities(array $capabilities) { + $this->capabilities = $capabilities; + return $this; + } + + public function getCapabilities() { + return $this->capabilities; + } + + public function setIsActive($is_active) { + $this->isActive = $is_active; + return $this; + } + + public function getIsActive() { + return $this->isActive; + } + +} diff --git a/src/applications/policy/controller/PhabricatorPolicyExplainController.php b/src/applications/policy/controller/PhabricatorPolicyExplainController.php index 54d1902c36..5030c4d636 100644 --- a/src/applications/policy/controller/PhabricatorPolicyExplainController.php +++ b/src/applications/policy/controller/PhabricatorPolicyExplainController.php @@ -34,124 +34,118 @@ final class PhabricatorPolicyExplainController ->setViewer($viewer) ->withPHIDs(array($phid)) ->executeOne(); + + $object_name = $handle->getName(); $object_uri = nonempty($handle->getURI(), '/'); - $explanation = PhabricatorPolicy::getPolicyExplanation( - $viewer, - $policy->getPHID()); - - $auto_info = (array)$object->describeAutomaticCapability($capability); - - $auto_info = array_merge( - array($explanation), - $auto_info); - $auto_info = array_filter($auto_info); - - $capability_name = $capability; - $capobj = PhabricatorPolicyCapability::getCapabilityByKey($capability); - if ($capobj) { - $capability_name = $capobj->getCapabilityName(); - } - $dialog = id(new AphrontDialogView()) ->setUser($viewer) - ->setClass('aphront-access-dialog'); - - $this->appendSpaceInformation($dialog, $object, $policy, $capability); - - $intro = pht( - 'Users with the "%s" capability for this object:', - $capability_name); - - $object_name = pht( - '%s %s', - $handle->getTypeName(), - $handle->getObjectName()); - - $dialog + ->setClass('aphront-access-dialog') ->setTitle(pht('Policy Details: %s', $object_name)) - ->appendParagraph($intro) ->addCancelButton($object_uri, pht('Done')); - if ($auto_info) { - $dialog->appendList($auto_info); - } + $space_section = $this->buildSpaceSection( + $object, + $policy, + $capability); + + $extended_section = $this->buildExtendedSection( + $object, + $capability); + + $exceptions_section = $this->buildExceptionsSection( + $object, + $capability); + + $object_section = $this->buildObjectSection( + $object, + $policy, + $capability, + $handle); + + $dialog->appendChild( + array( + $space_section, + $extended_section, + $exceptions_section, + $object_section, + )); - $this->appendStrengthInformation($dialog, $object, $policy, $capability); return $dialog; } - private function appendSpaceInformation( - AphrontDialogView $dialog, + private function buildSpaceSection( PhabricatorPolicyInterface $object, PhabricatorPolicy $policy, $capability) { $viewer = $this->getViewer(); if (!($object instanceof PhabricatorSpacesInterface)) { - return; + return null; } if (!PhabricatorSpacesNamespaceQuery::getSpacesExist($viewer)) { - return; + return null; } - // NOTE: We're intentionally letting users through here, even if they only - // have access to one space. The intent is to help users in "space jail" - // understand who objects they create are visible to: - $space_phid = PhabricatorSpacesNamespaceQuery::getObjectSpacePHID( $object); - $handles = $viewer->loadHandles(array($space_phid)); - $doc_href = PhabricatorEnv::getDoclink('Spaces User Guide'); - - $dialog->appendParagraph( - array( - pht( - 'This object is in %s, and can only be seen or edited by users with '. - 'access to view objects in the space.', - $handles[$space_phid]->renderLink()), - ' ', - phutil_tag( - 'strong', - array(), - phutil_tag( - 'a', - array( - 'href' => $doc_href, - 'target' => '_blank', - ), - pht('Learn More'))), - )); - $spaces = PhabricatorSpacesNamespaceQuery::getViewerSpaces($viewer); $space = idx($spaces, $space_phid); if (!$space) { - return; + return null; } $space_policies = PhabricatorPolicyQuery::loadPolicies($viewer, $space); $space_policy = idx($space_policies, PhabricatorPolicyCapability::CAN_VIEW); if (!$space_policy) { - return; + return null; } + $doc_href = PhabricatorEnv::getDoclink('Spaces User Guide'); + $capability_name = $this->getCapabilityName($capability); + + $space_section = id(new PHUIPolicySectionView()) + ->setViewer($viewer) + ->setIcon('fa-th-large bluegrey') + ->setHeader(pht('Space')) + ->setDocumentationLink(pht('Spaces Documentation'), $doc_href) + ->appendList( + array( + array( + phutil_tag('strong', array(), pht('Space:')), + ' ', + $viewer->renderHandle($space_phid)->setAsTag(true), + ), + array( + phutil_tag('strong', array(), pht('%s:', $capability_name)), + ' ', + $space_policy->getShortName(), + ), + )) + ->appendParagraph( + pht( + 'This object is in %s and can only be seen or edited by users '. + 'with access to view objects in the space.', + $viewer->renderHandle($space_phid))); + $space_explanation = PhabricatorPolicy::getPolicyExplanation( $viewer, $space_policy->getPHID()); $items = array(); $items[] = $space_explanation; - $dialog->appendParagraph(pht('Users who can see objects in this space:')); - $dialog->appendList($items); + $space_section + ->appendParagraph(pht('Users who can see objects in this space:')) + ->appendList($items); $view_capability = PhabricatorPolicyCapability::CAN_VIEW; if ($capability == $view_capability) { $stronger = $space_policy->isStrongerThan($policy); if ($stronger) { - $dialog->appendParagraph( + $space_section->appendHint( pht( 'The space this object is in has a more restrictive view '. 'policy ("%s") than the object does ("%s"), so the space\'s '. @@ -161,14 +155,15 @@ final class PhabricatorPolicyExplainController } } - $dialog->appendParagraph( + $space_section->appendHint( pht( 'After a user passes space policy checks, they must still pass '. 'object policy checks.')); + + return $space_section; } - private function appendStrengthInformation( - AphrontDialogView $dialog, + private function getStrengthInformation( PhabricatorPolicyInterface $object, PhabricatorPolicy $policy, $capability) { @@ -206,7 +201,157 @@ final class PhabricatorPolicyExplainController $default_policy->getShortName()); } - $dialog->appendParagraph($info); + return $info; + } + + private function getCapabilityName($capability) { + $capability_name = $capability; + $capobj = PhabricatorPolicyCapability::getCapabilityByKey($capability); + if ($capobj) { + $capability_name = $capobj->getCapabilityName(); + } + + return $capability_name; + } + + private function buildExtendedSection( + PhabricatorPolicyInterface $object, + $capability) { + $viewer = $this->getViewer(); + + if (!($object instanceof PhabricatorExtendedPolicyInterface)) { + return null; + } + + $extended_rules = $object->getExtendedPolicy($capability, $viewer); + if (!$extended_rules) { + return null; + } + + $items = array(); + foreach ($extended_rules as $extended_rule) { + $extended_target = $extended_rule[0]; + $extended_capabilities = (array)$extended_rule[1]; + if (is_object($extended_target)) { + $extended_target = $extended_target->getPHID(); + } + + foreach ($extended_capabilities as $extended_capability) { + $ex_name = $this->getCapabilityName($extended_capability); + $items[] = array( + phutil_tag('strong', array(), pht('%s:', $ex_name)), + ' ', + $viewer->renderHandle($extended_target)->setAsTag(true), + ); + } + } + + return id(new PHUIPolicySectionView()) + ->setViewer($viewer) + ->setIcon('fa-link') + ->setHeader(pht('Required Capabilities on Other Objects')) + ->appendParagraph( + pht( + 'To access this object, users must have first have access '. + 'capabilties on these other objects:')) + ->appendList($items); + } + + private function buildExceptionsSection( + PhabricatorPolicyInterface $object, + $capability) { + $viewer = $this->getViewer(); + + if ($object instanceof PhabricatorPolicyCodexInterface) { + $codex = PhabricatorPolicyCodex::newFromObject($object, $viewer); + $rules = $codex->getPolicySpecialRuleDescriptions(); + + $exceptions = array(); + foreach ($rules as $rule) { + $is_active = $rule->getIsActive(); + if ($is_active) { + $rule_capabilities = $rule->getCapabilities(); + if ($rule_capabilities) { + if (!in_array($capability, $rule_capabilities)) { + $is_active = false; + } + } + } + + $description = $rule->getDescription(); + + if (!$is_active) { + $description = phutil_tag( + 'span', + array( + 'class' => 'phui-policy-section-view-inactive-rule', + ), + $description); + } + + $exceptions[] = $description; + } + } else if (method_exists($object, 'describeAutomaticCapability')) { + $exceptions = (array)$object->describeAutomaticCapability($capability); + $exceptions = array_filter($exceptions); + } else { + $exceptions = array(); + } + + if (!$exceptions) { + return null; + } + + + return id(new PHUIPolicySectionView()) + ->setViewer($viewer) + ->setIcon('fa-unlock-alt red') + ->setHeader(pht('Special Rules')) + ->appendParagraph( + pht( + 'This object has special rules which override normal object '. + 'policy rules:')) + ->appendList($exceptions); + } + + private function buildObjectSection( + PhabricatorPolicyInterface $object, + PhabricatorPolicy $policy, + $capability, + PhabricatorObjectHandle $handle) { + + $viewer = $this->getViewer(); + $capability_name = $this->getCapabilityName($capability); + + $object_section = id(new PHUIPolicySectionView()) + ->setViewer($viewer) + ->setIcon($handle->getIcon().' bluegrey') + ->setHeader(pht('Object Policy')) + ->appendList( + array( + array( + phutil_tag('strong', array(), pht('%s:', $capability_name)), + ' ', + $policy->getShortName(), + ), + )) + ->appendParagraph( + pht( + 'In detail, this means that these users can take this action, '. + 'provided they pass all of the checks described above first:')) + ->appendList( + array( + PhabricatorPolicy::getPolicyExplanation( + $viewer, + $policy->getPHID()), + )); + + $strength = $this->getStrengthInformation($object, $policy, $capability); + if ($strength) { + $object_section->appendHint($strength); + } + + return $object_section; } } diff --git a/src/applications/policy/interface/PhabricatorPolicyInterface.php b/src/applications/policy/interface/PhabricatorPolicyInterface.php index 515f0d1626..df93512f2f 100644 --- a/src/applications/policy/interface/PhabricatorPolicyInterface.php +++ b/src/applications/policy/interface/PhabricatorPolicyInterface.php @@ -6,34 +6,6 @@ interface PhabricatorPolicyInterface extends PhabricatorPHIDInterface { public function getPolicy($capability); public function hasAutomaticCapability($capability, PhabricatorUser $viewer); - /** - * Describe exceptions to an object's policy setting. - * - * The intent of this method is to explain and inform users about special - * cases which override configured policy settings. If this object has any - * such exceptions, explain them by returning one or more human-readable - * strings which describe the exception in a broad, categorical way. For - * example: - * - * - "The owner of an X can always view and edit it." - * - "Members of a Y can always view it." - * - * You can return `null`, a single string, or a list of strings. - * - * The relevant capability to explain (like "view") is passed as a parameter. - * You should tailor any messages to be relevant to that capability, although - * they do not need to exclusively describe the capability, and in some cases - * being more general ("The author can view and edit...") will be more clear. - * - * Messages should describe general rules, not specific objects, because the - * main goal is to teach the user the rules. For example, write "the author", - * not the specific author's name. - * - * @param const @{class:PhabricatorPolicyCapability} constant. - * @return wild Description of policy exceptions. See above. - */ - public function describeAutomaticCapability($capability); - } // TEMPLATE IMPLEMENTATION ///////////////////////////////////////////////////// @@ -58,8 +30,4 @@ interface PhabricatorPolicyInterface extends PhabricatorPHIDInterface { return false; } - public function describeAutomaticCapability($capability) { - return null; - } - */ diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php index f2bfb8ce48..a7426ccad6 100644 --- a/src/applications/policy/storage/PhabricatorPolicy.php +++ b/src/applications/policy/storage/PhabricatorPolicy.php @@ -225,7 +225,9 @@ final class PhabricatorPolicy switch ($policy) { case PhabricatorPolicies::POLICY_PUBLIC: - return pht('This object is public.'); + return pht( + 'This object is public and can be viewed by anyone, even if they '. + 'do not have a Phabricator account.'); case PhabricatorPolicies::POLICY_USER: return pht('Logged in users can take this action.'); case PhabricatorPolicies::POLICY_ADMIN: diff --git a/src/applications/policy/view/PHUIPolicySectionView.php b/src/applications/policy/view/PHUIPolicySectionView.php new file mode 100644 index 0000000000..471e5035fb --- /dev/null +++ b/src/applications/policy/view/PHUIPolicySectionView.php @@ -0,0 +1,142 @@ +header = $header; + return $this; + } + + public function getHeader() { + return $this->header; + } + + public function setIcon($icon) { + $this->icon = $icon; + return $this; + } + + public function getIcon() { + return $this->icon; + } + + public function setDocumentationLink($name, $href) { + $link = phutil_tag( + 'a', + array( + 'href' => $href, + 'target' => '_blank', + ), + $name); + + $this->documentationLink = phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-link', + ), + array( + id(new PHUIIconView())->setIcon('fa-book'), + $link, + )); + + return $this; + } + + public function getDocumentationLink() { + return $this->documentationLink; + } + + public function appendList(array $items) { + foreach ($items as $key => $item) { + $items[$key] = phutil_tag( + 'li', + array( + 'class' => 'remarkup-list-item', + ), + $item); + } + + $list = phutil_tag( + 'ul', + array( + 'class' => 'remarkup-list', + ), + $items); + + return $this->appendChild($list); + } + + public function appendHint($content) { + $hint = phutil_tag( + 'p', + array( + 'class' => 'phui-policy-section-view-hint', + ), + array( + id(new PHUIIconView()) + ->setIcon('fa-sticky-note bluegrey'), + ' ', + pht('Note:'), + ' ', + $content, + )); + + return $this->appendChild($hint); + } + + public function appendParagraph($content) { + return $this->appendChild(phutil_tag('p', array(), $content)); + } + + protected function getTagAttributes() { + return array( + 'class' => 'phui-policy-section-view', + ); + } + + protected function getTagContent() { + require_celerity_resource('phui-header-view-css'); + + $icon_view = null; + $icon = $this->getIcon(); + if ($icon !== null) { + $icon_view = id(new PHUIIconView()) + ->setIcon($icon); + } + + $header_view = phutil_tag( + 'span', + array( + 'class' => 'phui-policy-section-view-header-text', + ), + $this->getHeader()); + + $header = phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-header', + ), + array( + $icon_view, + $header_view, + $this->getDocumentationLink(), + )); + + return array( + $header, + phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-body', + ), + $this->renderChildren()), + ); + } + + +} diff --git a/src/view/phui/PHUIHeaderView.php b/src/view/phui/PHUIHeaderView.php index e5e834bef5..0ae2c558d6 100644 --- a/src/view/phui/PHUIHeaderView.php +++ b/src/view/phui/PHUIHeaderView.php @@ -506,8 +506,40 @@ final class PHUIHeaderView extends AphrontTagView { } } + $policy_name = array($policy->getShortName()); + $policy_icon = $policy->getIcon().' bluegrey'; + + if ($object instanceof PhabricatorPolicyCodexInterface) { + $codex = PhabricatorPolicyCodex::newFromObject($object, $viewer); + + $codex_name = $codex->getPolicyShortName($policy, $view_capability); + if ($codex_name !== null) { + $policy_name = $codex_name; + } + + $codex_icon = $codex->getPolicyIcon($policy, $view_capability); + if ($codex_icon !== null) { + $policy_icon = $codex_icon; + } + + $codex_classes = $codex->getPolicyTagClasses($policy, $view_capability); + foreach ($codex_classes as $codex_class) { + $container_classes[] = $codex_class; + } + } + + if (!is_array($policy_name)) { + $policy_name = (array)$policy_name; + } + + $arrow = id(new PHUIIconView()) + ->setIcon('fa-angle-right') + ->addClass('policy-tier-separator'); + + $policy_name = phutil_implode_html($arrow, $policy_name); + $icon = id(new PHUIIconView()) - ->setIcon($policy->getIcon().' bluegrey'); + ->setIcon($policy_icon); $link = javelin_tag( 'a', @@ -516,7 +548,7 @@ final class PHUIHeaderView extends AphrontTagView { 'href' => '/policy/explain/'.$phid.'/'.$view_capability.'/', 'sigil' => 'workflow', ), - $policy->getShortName()); + $policy_name); return phutil_tag( 'span', diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css index dd746dd5ee..a010c82b5a 100644 --- a/webroot/rsrc/css/aphront/dialog-view.css +++ b/webroot/rsrc/css/aphront/dialog-view.css @@ -128,6 +128,10 @@ width: 50%; } +.aphront-access-dialog .aphront-dialog-body { + padding: 0 12px; +} + .aphront-policy-rejection { font-weight: bold; } diff --git a/webroot/rsrc/css/phui/phui-header-view.css b/webroot/rsrc/css/phui/phui-header-view.css index e773668f0a..06d16f60b3 100644 --- a/webroot/rsrc/css/phui/phui-header-view.css +++ b/webroot/rsrc/css/phui/phui-header-view.css @@ -240,6 +240,26 @@ body .phui-header-shell.phui-bleed-header color: {$sh-orangetext}; } +.policy-header-callout.policy-adjusted-special { + background: {$sh-indigobackground}; +} + +.policy-header-callout.policy-adjusted-special .policy-link, +.policy-header-callout.policy-adjusted-special .phui-icon-view { + color: {$sh-indigotext}; +} + +.policy-header-callout .policy-space-container { + font-weight: bold; + color: {$sh-redtext}; +} + +.policy-header-callout .policy-tier-separator { + padding: 0 0 0 4px; + color: {$lightgreytext}; +} + + .phui-header-action-links .phui-mobile-menu { display: none; } @@ -333,3 +353,45 @@ body .phui-header-shell.phui-bleed-header .phui-header-view .phui-tag-shade-indigo a { color: {$sh-indigotext}; } + +.phui-policy-section-view { + margin-bottom: 24px; +} + +.phui-policy-section-view-header { + background: {$bluebackground}; + border-bottom: 1px solid {$lightblueborder}; + padding: 4px 8px; + color: {$darkbluetext}; + margin-bottom: 8px; +} + +.phui-policy-section-view-header-text { + font-weight: bold; +} + +.phui-policy-section-view-header .phui-icon-view { + margin-right: 8px; +} + +.phui-policy-section-view-link { + float: right; +} + +.phui-policy-section-view-link .phui-icon-view { + color: {$bluetext}; +} + +.phui-policy-section-view-hint { + color: {$greytext}; + background: {$lightbluebackground}; + padding: 8px; +} + +.phui-policy-section-view-body { + padding: 0 12px; +} + +.phui-policy-section-view-inactive-rule { + color: {$greytext}; +} From 706c21375e8f8dc1b2092aeca54fb5a754441864 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 9 Nov 2016 15:13:37 -0800 Subject: [PATCH 20/31] Remove empty implementations of `describeAutomaticCapabilities()` Summary: This has been replaced by `PolicyCodex` after D16830. Also: - Rebuild Celerity map to fix grumpy unit test. - Fix one issue on the policy exception workflow to accommodate the new code. Test Plan: - `arc unit --everything` - Viewed policy explanations. - Viewed policy errors. Reviewers: chad Reviewed By: chad Subscribers: hach-que, PHID-OPKG-gm6ozazyms6q6i22gyam Differential Revision: https://secure.phabricator.com/D16831 --- resources/celerity/map.php | 10 ++-- .../almanac/storage/AlmanacDevice.php | 4 -- .../almanac/storage/AlmanacNamespace.php | 4 -- .../almanac/storage/AlmanacNetwork.php | 4 -- .../almanac/storage/AlmanacService.php | 4 -- .../storage/PhabricatorAuthProviderConfig.php | 4 -- .../storage/PhabricatorAuthTemporaryToken.php | 4 -- .../badges/storage/PhabricatorBadgesAward.php | 4 -- .../badges/storage/PhabricatorBadgesBadge.php | 4 -- .../base/PhabricatorApplication.php | 4 -- .../PhabricatorCalendarEventInvitee.php | 3 -- .../storage/PhabricatorCalendarExport.php | 5 -- .../PhabricatorCalendarExternalInvitee.php | 3 -- .../storage/PhabricatorCalendarImport.php | 4 -- .../storage/PhabricatorCalendarImportLog.php | 4 -- .../storage/PhabricatorChatLogChannel.php | 4 -- .../storage/PhabricatorChatLogEvent.php | 4 -- .../conduit/method/ConduitAPIMethod.php | 4 -- .../PhabricatorConduitMethodCallLog.php | 4 -- .../config/storage/PhabricatorConfigEntry.php | 4 -- .../storage/PhabricatorCountdown.php | 4 -- .../daemon/storage/PhabricatorDaemonLog.php | 4 -- .../storage/PhabricatorDashboard.php | 4 -- .../storage/PhabricatorDashboardPanel.php | 4 -- .../storage/DifferentialChangeset.php | 4 -- .../differential/storage/DifferentialHunk.php | 4 -- .../diviner/storage/DivinerLiveBook.php | 4 -- .../storage/DoorkeeperExternalObject.php | 4 -- .../drydock/storage/DrydockBlueprint.php | 4 -- .../feed/story/PhabricatorFeedStory.php | 5 -- .../files/storage/PhabricatorFileChunk.php | 5 -- .../storage/LegalpadDocumentSignature.php | 4 -- .../storage/PhabricatorFileImageMacro.php | 4 -- .../nuance/storage/NuanceImportCursorData.php | 4 -- .../nuance/storage/NuanceItemCommand.php | 4 -- .../nuance/storage/NuanceQueue.php | 4 -- .../nuance/storage/NuanceSource.php | 4 -- .../storage/PhabricatorOAuthServerClient.php | 4 -- .../storage/PhabricatorPackagesPackage.php | 4 -- .../storage/PhabricatorPackagesPublisher.php | 4 -- .../storage/PhabricatorPackagesVersion.php | 4 -- .../storage/PassphraseCredential.php | 4 -- .../phlux/storage/PhluxVariable.php | 4 -- .../pholio/storage/PholioImage.php | 4 -- .../phortune/storage/PhortuneProduct.php | 4 -- .../phragment/storage/PhragmentFragment.php | 4 -- .../__tests__/PhabricatorPolicyTestObject.php | 4 -- .../PhabricatorPolicyExplainController.php | 41 ++------------- .../policy/filter/PhabricatorPolicyFilter.php | 11 +++- .../policy/storage/PhabricatorPolicy.php | 52 +++++++++++++++++-- .../PhabricatorProjectColumnPosition.php | 4 -- .../releeph/storage/ReleephProject.php | 5 -- .../storage/PhabricatorRepository.php | 5 -- .../PhabricatorRepositoryCommitHint.php | 4 -- .../PhabricatorRepositoryGitLFSRef.php | 4 -- .../storage/PhabricatorRepositoryOldRef.php | 4 -- .../storage/PhabricatorRepositoryURI.php | 4 -- .../PhabricatorProfilePanelConfiguration.php | 5 -- .../search/storage/PhabricatorSavedQuery.php | 4 -- .../storage/PhabricatorUserPreferences.php | 5 -- .../storage/PhabricatorSpacesNamespace.php | 5 -- .../tokens/storage/PhabricatorToken.php | 4 -- .../editengine/PhabricatorEditEngine.php | 3 -- .../PhabricatorEditEngineConfiguration.php | 5 -- .../storage/PhabricatorWorkerTrigger.php | 4 -- 65 files changed, 67 insertions(+), 297 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 2462868c0f..e5d5786142 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', - 'core.pkg.css' => 'c99a9eb4', + 'core.pkg.css' => 'd2126ffb', 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', @@ -21,7 +21,7 @@ return array( 'maniphest.pkg.js' => '949a7498', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', 'rsrc/css/aphront/dark-console.css' => 'f54bf286', - 'rsrc/css/aphront/dialog-view.css' => '84f1e6a6', + 'rsrc/css/aphront/dialog-view.css' => '1e6b8603', 'rsrc/css/aphront/lightbox-attachment.css' => '7acac05d', 'rsrc/css/aphront/list-filter-view.css' => '5d6f0526', 'rsrc/css/aphront/multi-column.css' => '84cc6640', @@ -143,7 +143,7 @@ return array( 'rsrc/css/phui/phui-form-view.css' => 'b5bfd17a', 'rsrc/css/phui/phui-form.css' => 'aac1d51d', 'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f', - 'rsrc/css/phui/phui-header-view.css' => '06385974', + 'rsrc/css/phui/phui-header-view.css' => '6ec8f155', 'rsrc/css/phui/phui-hovercard.css' => 'de1a2119', 'rsrc/css/phui/phui-icon-set-selector.css' => '1ab67aad', 'rsrc/css/phui/phui-icon.css' => '417f80fb', @@ -553,7 +553,7 @@ return array( 'almanac-css' => 'dbb9b3af', 'aphront-bars' => '231ac33c', 'aphront-dark-console-css' => 'f54bf286', - 'aphront-dialog-view-css' => '84f1e6a6', + 'aphront-dialog-view-css' => '1e6b8603', 'aphront-list-filter-view-css' => '5d6f0526', 'aphront-multi-column-view-css' => '84cc6640', 'aphront-panel-view-css' => '8427b78d', @@ -867,7 +867,7 @@ return array( 'phui-form-css' => 'aac1d51d', 'phui-form-view-css' => 'b5bfd17a', 'phui-head-thing-view-css' => 'fd311e5f', - 'phui-header-view-css' => '06385974', + 'phui-header-view-css' => '6ec8f155', 'phui-hovercard' => '1bd28176', 'phui-hovercard-view-css' => 'de1a2119', 'phui-icon-set-selector-css' => '1ab67aad', diff --git a/src/applications/almanac/storage/AlmanacDevice.php b/src/applications/almanac/storage/AlmanacDevice.php index d2a3e4a763..2f89e6c2b6 100644 --- a/src/applications/almanac/storage/AlmanacDevice.php +++ b/src/applications/almanac/storage/AlmanacDevice.php @@ -167,10 +167,6 @@ final class AlmanacDevice return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ diff --git a/src/applications/almanac/storage/AlmanacNamespace.php b/src/applications/almanac/storage/AlmanacNamespace.php index 4bbb4d4090..7a8a4a20f0 100644 --- a/src/applications/almanac/storage/AlmanacNamespace.php +++ b/src/applications/almanac/storage/AlmanacNamespace.php @@ -174,10 +174,6 @@ final class AlmanacNamespace return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/almanac/storage/AlmanacNetwork.php b/src/applications/almanac/storage/AlmanacNetwork.php index 2f37ab4778..737f38faec 100644 --- a/src/applications/almanac/storage/AlmanacNetwork.php +++ b/src/applications/almanac/storage/AlmanacNetwork.php @@ -92,10 +92,6 @@ final class AlmanacNetwork return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/almanac/storage/AlmanacService.php b/src/applications/almanac/storage/AlmanacService.php index 37c3a44ba6..9ef42d6407 100644 --- a/src/applications/almanac/storage/AlmanacService.php +++ b/src/applications/almanac/storage/AlmanacService.php @@ -184,10 +184,6 @@ final class AlmanacService return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ diff --git a/src/applications/auth/storage/PhabricatorAuthProviderConfig.php b/src/applications/auth/storage/PhabricatorAuthProviderConfig.php index 14d68b352b..ba9b43a967 100644 --- a/src/applications/auth/storage/PhabricatorAuthProviderConfig.php +++ b/src/applications/auth/storage/PhabricatorAuthProviderConfig.php @@ -130,8 +130,4 @@ final class PhabricatorAuthProviderConfig return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php b/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php index be08f5db2d..76e9358831 100644 --- a/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php +++ b/src/applications/auth/storage/PhabricatorAuthTemporaryToken.php @@ -125,8 +125,4 @@ final class PhabricatorAuthTemporaryToken extends PhabricatorAuthDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/badges/storage/PhabricatorBadgesAward.php b/src/applications/badges/storage/PhabricatorBadgesAward.php index 851ac87002..3f1472258f 100644 --- a/src/applications/badges/storage/PhabricatorBadgesAward.php +++ b/src/applications/badges/storage/PhabricatorBadgesAward.php @@ -76,8 +76,4 @@ final class PhabricatorBadgesAward extends PhabricatorBadgesDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/badges/storage/PhabricatorBadgesBadge.php b/src/applications/badges/storage/PhabricatorBadgesBadge.php index be83c8404f..d14d49a57c 100644 --- a/src/applications/badges/storage/PhabricatorBadgesBadge.php +++ b/src/applications/badges/storage/PhabricatorBadgesBadge.php @@ -129,10 +129,6 @@ final class PhabricatorBadgesBadge extends PhabricatorBadgesDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index b45b1be102..1dd6bb887c 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -483,10 +483,6 @@ abstract class PhabricatorApplication return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( Policies )----------------------------------------------------------- */ diff --git a/src/applications/calendar/storage/PhabricatorCalendarEventInvitee.php b/src/applications/calendar/storage/PhabricatorCalendarEventInvitee.php index b238b614ab..23c9ad33df 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEventInvitee.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEventInvitee.php @@ -121,7 +121,4 @@ final class PhabricatorCalendarEventInvitee extends PhabricatorCalendarDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } } diff --git a/src/applications/calendar/storage/PhabricatorCalendarExport.php b/src/applications/calendar/storage/PhabricatorCalendarExport.php index 75e83dfaf0..5a8ad84da4 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarExport.php +++ b/src/applications/calendar/storage/PhabricatorCalendarExport.php @@ -159,11 +159,6 @@ final class PhabricatorCalendarExport extends PhabricatorCalendarDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/calendar/storage/PhabricatorCalendarExternalInvitee.php b/src/applications/calendar/storage/PhabricatorCalendarExternalInvitee.php index b1a85cf520..27a23d1e17 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarExternalInvitee.php +++ b/src/applications/calendar/storage/PhabricatorCalendarExternalInvitee.php @@ -68,7 +68,4 @@ final class PhabricatorCalendarExternalInvitee return false; } - public function describeAutomaticCapability($capability) { - return null; - } } diff --git a/src/applications/calendar/storage/PhabricatorCalendarImport.php b/src/applications/calendar/storage/PhabricatorCalendarImport.php index 7fdb3cd837..1c7c1a5431 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarImport.php +++ b/src/applications/calendar/storage/PhabricatorCalendarImport.php @@ -132,10 +132,6 @@ final class PhabricatorCalendarImport return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/calendar/storage/PhabricatorCalendarImportLog.php b/src/applications/calendar/storage/PhabricatorCalendarImportLog.php index e25a3090e0..14108f8ca7 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarImportLog.php +++ b/src/applications/calendar/storage/PhabricatorCalendarImportLog.php @@ -86,10 +86,6 @@ final class PhabricatorCalendarImportLog return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/chatlog/storage/PhabricatorChatLogChannel.php b/src/applications/chatlog/storage/PhabricatorChatLogChannel.php index d9b9ef67e9..2e62bcb6cf 100644 --- a/src/applications/chatlog/storage/PhabricatorChatLogChannel.php +++ b/src/applications/chatlog/storage/PhabricatorChatLogChannel.php @@ -48,8 +48,4 @@ final class PhabricatorChatLogChannel return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/chatlog/storage/PhabricatorChatLogEvent.php b/src/applications/chatlog/storage/PhabricatorChatLogEvent.php index 1513e12a82..a510c333dc 100644 --- a/src/applications/chatlog/storage/PhabricatorChatLogEvent.php +++ b/src/applications/chatlog/storage/PhabricatorChatLogEvent.php @@ -56,8 +56,4 @@ final class PhabricatorChatLogEvent return $this->getChannel()->hasAutomaticCapability($capability, $viewer); } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/conduit/method/ConduitAPIMethod.php b/src/applications/conduit/method/ConduitAPIMethod.php index 7992518919..bc3f9e44e0 100644 --- a/src/applications/conduit/method/ConduitAPIMethod.php +++ b/src/applications/conduit/method/ConduitAPIMethod.php @@ -372,10 +372,6 @@ abstract class ConduitAPIMethod return false; } - public function describeAutomaticCapability($capability) { - return null; - } - protected function hasApplicationCapability( $capability, PhabricatorUser $viewer) { diff --git a/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php b/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php index 2e3f12b0e1..7387b64e53 100644 --- a/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php +++ b/src/applications/conduit/storage/PhabricatorConduitMethodCallLog.php @@ -52,8 +52,4 @@ final class PhabricatorConduitMethodCallLog return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/config/storage/PhabricatorConfigEntry.php b/src/applications/config/storage/PhabricatorConfigEntry.php index 53935d4ab0..a8f00133a9 100644 --- a/src/applications/config/storage/PhabricatorConfigEntry.php +++ b/src/applications/config/storage/PhabricatorConfigEntry.php @@ -95,8 +95,4 @@ final class PhabricatorConfigEntry return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/countdown/storage/PhabricatorCountdown.php b/src/applications/countdown/storage/PhabricatorCountdown.php index ad9b1b33aa..34249b9ab4 100644 --- a/src/applications/countdown/storage/PhabricatorCountdown.php +++ b/src/applications/countdown/storage/PhabricatorCountdown.php @@ -139,10 +139,6 @@ final class PhabricatorCountdown extends PhabricatorCountdownDAO return false; } - public function describeAutomaticCapability($capability) { - return false; - } - /* -( PhabricatorSpacesInterface )------------------------------------------- */ public function getSpacePHID() { diff --git a/src/applications/daemon/storage/PhabricatorDaemonLog.php b/src/applications/daemon/storage/PhabricatorDaemonLog.php index 2d005e33e0..2e61da3872 100644 --- a/src/applications/daemon/storage/PhabricatorDaemonLog.php +++ b/src/applications/daemon/storage/PhabricatorDaemonLog.php @@ -77,8 +77,4 @@ final class PhabricatorDaemonLog extends PhabricatorDaemonDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/dashboard/storage/PhabricatorDashboard.php b/src/applications/dashboard/storage/PhabricatorDashboard.php index 297ac8aea0..2930ca7674 100644 --- a/src/applications/dashboard/storage/PhabricatorDashboard.php +++ b/src/applications/dashboard/storage/PhabricatorDashboard.php @@ -160,10 +160,6 @@ final class PhabricatorDashboard extends PhabricatorDashboardDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/dashboard/storage/PhabricatorDashboardPanel.php b/src/applications/dashboard/storage/PhabricatorDashboardPanel.php index 5dae421674..64e6ea5b87 100644 --- a/src/applications/dashboard/storage/PhabricatorDashboardPanel.php +++ b/src/applications/dashboard/storage/PhabricatorDashboardPanel.php @@ -159,10 +159,6 @@ final class PhabricatorDashboardPanel return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorCustomFieldInterface )------------------------------------ */ diff --git a/src/applications/differential/storage/DifferentialChangeset.php b/src/applications/differential/storage/DifferentialChangeset.php index 18cafad137..4d7b9f5698 100644 --- a/src/applications/differential/storage/DifferentialChangeset.php +++ b/src/applications/differential/storage/DifferentialChangeset.php @@ -226,8 +226,4 @@ final class DifferentialChangeset extends DifferentialDAO return $this->getDiff()->hasAutomaticCapability($capability, $viewer); } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/differential/storage/DifferentialHunk.php b/src/applications/differential/storage/DifferentialHunk.php index bd13cadfec..fcc7926590 100644 --- a/src/applications/differential/storage/DifferentialHunk.php +++ b/src/applications/differential/storage/DifferentialHunk.php @@ -228,8 +228,4 @@ abstract class DifferentialHunk extends DifferentialDAO return $this->getChangeset()->hasAutomaticCapability($capability, $viewer); } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/diviner/storage/DivinerLiveBook.php b/src/applications/diviner/storage/DivinerLiveBook.php index 079c5a9f4c..07089264ae 100644 --- a/src/applications/diviner/storage/DivinerLiveBook.php +++ b/src/applications/diviner/storage/DivinerLiveBook.php @@ -114,10 +114,6 @@ final class DivinerLiveBook extends DivinerDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php b/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php index 9d8d3bc967..a4ce41adac 100644 --- a/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php +++ b/src/applications/doorkeeper/storage/DoorkeeperExternalObject.php @@ -121,8 +121,4 @@ final class DoorkeeperExternalObject extends DoorkeeperDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/drydock/storage/DrydockBlueprint.php b/src/applications/drydock/storage/DrydockBlueprint.php index afd3b771ca..f7654a8b31 100644 --- a/src/applications/drydock/storage/DrydockBlueprint.php +++ b/src/applications/drydock/storage/DrydockBlueprint.php @@ -325,10 +325,6 @@ final class DrydockBlueprint extends DrydockDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorCustomFieldInterface )------------------------------------ */ diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index d586104e70..8a15301216 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -454,11 +454,6 @@ abstract class PhabricatorFeedStory } - public function describeAutomaticCapability($capability) { - return null; - } - - /* -( PhabricatorMarkupInterface Implementation )--------------------------- */ diff --git a/src/applications/files/storage/PhabricatorFileChunk.php b/src/applications/files/storage/PhabricatorFileChunk.php index eaa10acd63..648ec18c6b 100644 --- a/src/applications/files/storage/PhabricatorFileChunk.php +++ b/src/applications/files/storage/PhabricatorFileChunk.php @@ -77,11 +77,6 @@ final class PhabricatorFileChunk extends PhabricatorFileDAO } - public function describeAutomaticCapability($capability) { - return null; - } - - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/legalpad/storage/LegalpadDocumentSignature.php b/src/applications/legalpad/storage/LegalpadDocumentSignature.php index a811169cf7..1cd0875dda 100644 --- a/src/applications/legalpad/storage/LegalpadDocumentSignature.php +++ b/src/applications/legalpad/storage/LegalpadDocumentSignature.php @@ -93,8 +93,4 @@ final class LegalpadDocumentSignature return ($viewer->getPHID() == $this->getSignerPHID()); } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/macro/storage/PhabricatorFileImageMacro.php b/src/applications/macro/storage/PhabricatorFileImageMacro.php index 37f0d5c06b..bc23e639d7 100644 --- a/src/applications/macro/storage/PhabricatorFileImageMacro.php +++ b/src/applications/macro/storage/PhabricatorFileImageMacro.php @@ -139,8 +139,4 @@ final class PhabricatorFileImageMacro extends PhabricatorFileDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/nuance/storage/NuanceImportCursorData.php b/src/applications/nuance/storage/NuanceImportCursorData.php index 01f3d56c6d..bbd00f5cc5 100644 --- a/src/applications/nuance/storage/NuanceImportCursorData.php +++ b/src/applications/nuance/storage/NuanceImportCursorData.php @@ -63,8 +63,4 @@ final class NuanceImportCursorData return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/nuance/storage/NuanceItemCommand.php b/src/applications/nuance/storage/NuanceItemCommand.php index 94409dd89a..bda6860ff5 100644 --- a/src/applications/nuance/storage/NuanceItemCommand.php +++ b/src/applications/nuance/storage/NuanceItemCommand.php @@ -48,8 +48,4 @@ final class NuanceItemCommand return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/nuance/storage/NuanceQueue.php b/src/applications/nuance/storage/NuanceQueue.php index 15197d9bd8..9691a42e2f 100644 --- a/src/applications/nuance/storage/NuanceQueue.php +++ b/src/applications/nuance/storage/NuanceQueue.php @@ -67,10 +67,6 @@ final class NuanceQueue return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/nuance/storage/NuanceSource.php b/src/applications/nuance/storage/NuanceSource.php index 8ab934e247..c3f1d57c9b 100644 --- a/src/applications/nuance/storage/NuanceSource.php +++ b/src/applications/nuance/storage/NuanceSource.php @@ -138,10 +138,6 @@ final class NuanceSource extends NuanceDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorNgramsInterface )----------------------------------------- */ diff --git a/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php b/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php index 4a4f48bfed..4154383c30 100644 --- a/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php +++ b/src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php @@ -83,10 +83,6 @@ final class PhabricatorOAuthServerClient return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/packages/storage/PhabricatorPackagesPackage.php b/src/applications/packages/storage/PhabricatorPackagesPackage.php index fe4dd99162..53e040e5f4 100644 --- a/src/applications/packages/storage/PhabricatorPackagesPackage.php +++ b/src/applications/packages/storage/PhabricatorPackagesPackage.php @@ -158,10 +158,6 @@ final class PhabricatorPackagesPackage return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/packages/storage/PhabricatorPackagesPublisher.php b/src/applications/packages/storage/PhabricatorPackagesPublisher.php index 0cf7e627d1..57737cdab9 100644 --- a/src/applications/packages/storage/PhabricatorPackagesPublisher.php +++ b/src/applications/packages/storage/PhabricatorPackagesPublisher.php @@ -134,10 +134,6 @@ final class PhabricatorPackagesPublisher return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/packages/storage/PhabricatorPackagesVersion.php b/src/applications/packages/storage/PhabricatorPackagesVersion.php index 65ee005ff8..aba2473073 100644 --- a/src/applications/packages/storage/PhabricatorPackagesVersion.php +++ b/src/applications/packages/storage/PhabricatorPackagesVersion.php @@ -126,10 +126,6 @@ final class PhabricatorPackagesVersion return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ diff --git a/src/applications/passphrase/storage/PassphraseCredential.php b/src/applications/passphrase/storage/PassphraseCredential.php index 88bc4595bd..d57c7405d5 100644 --- a/src/applications/passphrase/storage/PassphraseCredential.php +++ b/src/applications/passphrase/storage/PassphraseCredential.php @@ -155,10 +155,6 @@ final class PassphraseCredential extends PassphraseDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorSubscribableInterface )----------------------------------- */ diff --git a/src/applications/phlux/storage/PhluxVariable.php b/src/applications/phlux/storage/PhluxVariable.php index a4ade1e9c7..875b3dee92 100644 --- a/src/applications/phlux/storage/PhluxVariable.php +++ b/src/applications/phlux/storage/PhluxVariable.php @@ -80,8 +80,4 @@ final class PhluxVariable extends PhluxDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/pholio/storage/PholioImage.php b/src/applications/pholio/storage/PholioImage.php index 29b649dd14..e81c71a05a 100644 --- a/src/applications/pholio/storage/PholioImage.php +++ b/src/applications/pholio/storage/PholioImage.php @@ -119,8 +119,4 @@ final class PholioImage extends PholioDAO return $this->getMock()->hasAutomaticCapability($capability, $viewer); } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/phortune/storage/PhortuneProduct.php b/src/applications/phortune/storage/PhortuneProduct.php index a3f89ffec3..18fc553e87 100644 --- a/src/applications/phortune/storage/PhortuneProduct.php +++ b/src/applications/phortune/storage/PhortuneProduct.php @@ -110,8 +110,4 @@ final class PhortuneProduct extends PhortuneDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/phragment/storage/PhragmentFragment.php b/src/applications/phragment/storage/PhragmentFragment.php index 1ab156e60f..af733ab730 100644 --- a/src/applications/phragment/storage/PhragmentFragment.php +++ b/src/applications/phragment/storage/PhragmentFragment.php @@ -345,8 +345,4 @@ final class PhragmentFragment extends PhragmentDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/policy/__tests__/PhabricatorPolicyTestObject.php b/src/applications/policy/__tests__/PhabricatorPolicyTestObject.php index 0e8d4c4957..efd500d438 100644 --- a/src/applications/policy/__tests__/PhabricatorPolicyTestObject.php +++ b/src/applications/policy/__tests__/PhabricatorPolicyTestObject.php @@ -52,10 +52,6 @@ final class PhabricatorPolicyTestObject return $this; } - public function describeAutomaticCapability($capability) { - return null; - } - public function setExtendedPolicies(array $extended_policies) { $this->extendedPolicies = $extended_policies; return $this; diff --git a/src/applications/policy/controller/PhabricatorPolicyExplainController.php b/src/applications/policy/controller/PhabricatorPolicyExplainController.php index 5030c4d636..ef61272d88 100644 --- a/src/applications/policy/controller/PhabricatorPolicyExplainController.php +++ b/src/applications/policy/controller/PhabricatorPolicyExplainController.php @@ -262,47 +262,16 @@ final class PhabricatorPolicyExplainController $capability) { $viewer = $this->getViewer(); - if ($object instanceof PhabricatorPolicyCodexInterface) { - $codex = PhabricatorPolicyCodex::newFromObject($object, $viewer); - $rules = $codex->getPolicySpecialRuleDescriptions(); - - $exceptions = array(); - foreach ($rules as $rule) { - $is_active = $rule->getIsActive(); - if ($is_active) { - $rule_capabilities = $rule->getCapabilities(); - if ($rule_capabilities) { - if (!in_array($capability, $rule_capabilities)) { - $is_active = false; - } - } - } - - $description = $rule->getDescription(); - - if (!$is_active) { - $description = phutil_tag( - 'span', - array( - 'class' => 'phui-policy-section-view-inactive-rule', - ), - $description); - } - - $exceptions[] = $description; - } - } else if (method_exists($object, 'describeAutomaticCapability')) { - $exceptions = (array)$object->describeAutomaticCapability($capability); - $exceptions = array_filter($exceptions); - } else { - $exceptions = array(); - } + $exceptions = PhabricatorPolicy::getSpecialRules( + $object, + $viewer, + $capability, + false); if (!$exceptions) { return null; } - return id(new PHUIPolicySectionView()) ->setViewer($viewer) ->setIcon('fa-unlock-alt red') diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php index c3ac7fc016..1466d4e569 100644 --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -581,9 +581,16 @@ final class PhabricatorPolicyFilter extends Phobject { } $more = PhabricatorPolicy::getPolicyExplanation($this->viewer, $policy); - $exceptions = $object->describeAutomaticCapability($capability); + $more = (array)$more; + $more = array_filter($more); - $details = array_filter(array_merge(array($more), (array)$exceptions)); + $exceptions = PhabricatorPolicy::getSpecialRules( + $object, + $this->viewer, + $capability, + true); + + $details = array_filter(array_merge($more, $exceptions)); $access_denied = $this->renderAccessDenied($object); diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php index a7426ccad6..68462cbc19 100644 --- a/src/applications/policy/storage/PhabricatorPolicy.php +++ b/src/applications/policy/storage/PhabricatorPolicy.php @@ -422,6 +422,54 @@ final class PhabricatorPolicy return ($this_strength > $other_strength); } + public static function getSpecialRules( + PhabricatorPolicyInterface $object, + PhabricatorUser $viewer, + $capability, + $active_only) { + + if ($object instanceof PhabricatorPolicyCodexInterface) { + $codex = PhabricatorPolicyCodex::newFromObject($object, $viewer); + $rules = $codex->getPolicySpecialRuleDescriptions(); + + $exceptions = array(); + foreach ($rules as $rule) { + $is_active = $rule->getIsActive(); + if ($is_active) { + $rule_capabilities = $rule->getCapabilities(); + if ($rule_capabilities) { + if (!in_array($capability, $rule_capabilities)) { + $is_active = false; + } + } + } + + if (!$is_active && $active_only) { + continue; + } + + $description = $rule->getDescription(); + + if (!$is_active) { + $description = phutil_tag( + 'span', + array( + 'class' => 'phui-policy-section-view-inactive-rule', + ), + $description); + } + + $exceptions[] = $description; + } + } else if (method_exists($object, 'describeAutomaticCapability')) { + $exceptions = (array)$object->describeAutomaticCapability($capability); + $exceptions = array_filter($exceptions); + } else { + $exceptions = array(); + } + + return $exceptions; + } /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -444,10 +492,6 @@ final class PhabricatorPolicy return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/project/storage/PhabricatorProjectColumnPosition.php b/src/applications/project/storage/PhabricatorProjectColumnPosition.php index 40929606ac..0bd9be6d4a 100644 --- a/src/applications/project/storage/PhabricatorProjectColumnPosition.php +++ b/src/applications/project/storage/PhabricatorProjectColumnPosition.php @@ -87,8 +87,4 @@ final class PhabricatorProjectColumnPosition extends PhabricatorProjectDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/releeph/storage/ReleephProject.php b/src/applications/releeph/storage/ReleephProject.php index cf5e179abf..bda0d0372d 100644 --- a/src/applications/releeph/storage/ReleephProject.php +++ b/src/applications/releeph/storage/ReleephProject.php @@ -153,9 +153,4 @@ final class ReleephProject extends ReleephDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - - } diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index c27f562864..d38eda1860 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -2330,11 +2330,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - - /* -( PhabricatorMarkupInterface )----------------------------------------- */ diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php index e8594a1496..0a8e259dfe 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php @@ -130,8 +130,4 @@ final class PhabricatorRepositoryCommitHint return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/repository/storage/PhabricatorRepositoryGitLFSRef.php b/src/applications/repository/storage/PhabricatorRepositoryGitLFSRef.php index 507af99cb4..6e55c03891 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryGitLFSRef.php +++ b/src/applications/repository/storage/PhabricatorRepositoryGitLFSRef.php @@ -45,10 +45,6 @@ final class PhabricatorRepositoryGitLFSRef return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/repository/storage/PhabricatorRepositoryOldRef.php b/src/applications/repository/storage/PhabricatorRepositoryOldRef.php index 65cba3c48b..12763a42fe 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryOldRef.php +++ b/src/applications/repository/storage/PhabricatorRepositoryOldRef.php @@ -45,8 +45,4 @@ final class PhabricatorRepositoryOldRef return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/repository/storage/PhabricatorRepositoryURI.php b/src/applications/repository/storage/PhabricatorRepositoryURI.php index 15e25205c8..2f2815da1c 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryURI.php +++ b/src/applications/repository/storage/PhabricatorRepositoryURI.php @@ -642,10 +642,6 @@ final class PhabricatorRepositoryURI return false; } - public function describeAutomaticCapability($capability) { - return null; - } - /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ diff --git a/src/applications/search/storage/PhabricatorProfilePanelConfiguration.php b/src/applications/search/storage/PhabricatorProfilePanelConfiguration.php index d6f70e802e..6946026d8b 100644 --- a/src/applications/search/storage/PhabricatorProfilePanelConfiguration.php +++ b/src/applications/search/storage/PhabricatorProfilePanelConfiguration.php @@ -162,11 +162,6 @@ final class PhabricatorProfilePanelConfiguration } - public function describeAutomaticCapability($capability) { - return null; - } - - /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ diff --git a/src/applications/search/storage/PhabricatorSavedQuery.php b/src/applications/search/storage/PhabricatorSavedQuery.php index 8d3435f8b4..5bc8a95915 100644 --- a/src/applications/search/storage/PhabricatorSavedQuery.php +++ b/src/applications/search/storage/PhabricatorSavedQuery.php @@ -81,8 +81,4 @@ final class PhabricatorSavedQuery extends PhabricatorSearchDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - } diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index 5ed360ca2c..847836b284 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -222,11 +222,6 @@ final class PhabricatorUserPreferences return false; } - public function describeAutomaticCapability($capability) { - return null; - } - - /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/spaces/storage/PhabricatorSpacesNamespace.php b/src/applications/spaces/storage/PhabricatorSpacesNamespace.php index f728d22dfa..5f01376840 100644 --- a/src/applications/spaces/storage/PhabricatorSpacesNamespace.php +++ b/src/applications/spaces/storage/PhabricatorSpacesNamespace.php @@ -84,11 +84,6 @@ final class PhabricatorSpacesNamespace return false; } - public function describeAutomaticCapability($capability) { - return null; - } - - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/tokens/storage/PhabricatorToken.php b/src/applications/tokens/storage/PhabricatorToken.php index 0beeba3b81..a1f156611e 100644 --- a/src/applications/tokens/storage/PhabricatorToken.php +++ b/src/applications/tokens/storage/PhabricatorToken.php @@ -28,10 +28,6 @@ final class PhabricatorToken extends PhabricatorTokenDAO return false; } - public function describeAutomaticCapability($capability) { - return null; - } - public function renderIcon() { // TODO: Maybe move to a View class? diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 2a84d68775..f45da8d148 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -2275,7 +2275,4 @@ abstract class PhabricatorEditEngine return false; } - public function describeAutomaticCapability($capability) { - return null; - } } diff --git a/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php b/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php index e169171f98..fc778c65b7 100644 --- a/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php +++ b/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php @@ -302,11 +302,6 @@ final class PhabricatorEditEngineConfiguration return false; } - public function describeAutomaticCapability($capability) { - return null; - } - - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTrigger.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTrigger.php index c4104a5c6e..7b9a24d7e0 100644 --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTrigger.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTrigger.php @@ -190,8 +190,4 @@ final class PhabricatorWorkerTrigger return true; } - public function describeAutomaticCapability($capability) { - return null; - } - } From e634812a6d3660439c72b81224b69fd29fe18bf6 Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Thu, 10 Nov 2016 00:40:09 +0000 Subject: [PATCH 21/31] Remove plain-text file view of Diffusion files. Summary: fixes T11792. There's no good reason any more to have this option, so just drop it. Test Plan: Load a file, toggle remaining "blame" button. Load search results page and an image too, which are serviced by the same controller. Reviewers: chad, #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin Maniphest Tasks: T11792 Differential Revision: https://secure.phabricator.com/D16833 --- src/__phutil_library_map__.php | 2 - .../controller/DiffusionBrowseController.php | 284 ++++++------------ .../PhabricatorDiffusionColorSetting.php | 16 - 3 files changed, 85 insertions(+), 217 deletions(-) delete mode 100644 src/applications/settings/setting/PhabricatorDiffusionColorSetting.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 071eef10e3..3a06c89c2b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2486,7 +2486,6 @@ phutil_register_library_map(array( 'PhabricatorDifferentialRevisionTestDataGenerator' => 'applications/differential/lipsum/PhabricatorDifferentialRevisionTestDataGenerator.php', 'PhabricatorDiffusionApplication' => 'applications/diffusion/application/PhabricatorDiffusionApplication.php', 'PhabricatorDiffusionBlameSetting' => 'applications/settings/setting/PhabricatorDiffusionBlameSetting.php', - 'PhabricatorDiffusionColorSetting' => 'applications/settings/setting/PhabricatorDiffusionColorSetting.php', 'PhabricatorDiffusionConfigOptions' => 'applications/diffusion/config/PhabricatorDiffusionConfigOptions.php', 'PhabricatorDisabledUserController' => 'applications/auth/controller/PhabricatorDisabledUserController.php', 'PhabricatorDisplayPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDisplayPreferencesSettingsPanel.php', @@ -7406,7 +7405,6 @@ phutil_register_library_map(array( 'PhabricatorDifferentialRevisionTestDataGenerator' => 'PhabricatorTestDataGenerator', 'PhabricatorDiffusionApplication' => 'PhabricatorApplication', 'PhabricatorDiffusionBlameSetting' => 'PhabricatorInternalSetting', - 'PhabricatorDiffusionColorSetting' => 'PhabricatorInternalSetting', 'PhabricatorDiffusionConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorDisabledUserController' => 'PhabricatorAuthController', 'PhabricatorDisplayPreferencesSettingsPanel' => 'PhabricatorEditEngineSettingsPanel', diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index 9d51146534..153906b25c 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -106,16 +106,11 @@ final class DiffusionBrowseController extends DiffusionController { $path = $drequest->getPath(); $blame_key = PhabricatorDiffusionBlameSetting::SETTINGKEY; - $color_key = PhabricatorDiffusionColorSetting::SETTINGKEY; $show_blame = $request->getBool( 'blame', $viewer->getUserSetting($blame_key)); - $show_color = $request->getBool( - 'color', - $viewer->getUserSetting($color_key)); - $view = $request->getStr('view'); if ($request->isFormPost() && $view != 'raw' && $viewer->isLoggedIn()) { $preferences = PhabricatorUserPreferences::loadUserPreferences($viewer); @@ -128,22 +123,18 @@ final class DiffusionBrowseController extends DiffusionController { $xactions = array(); $xactions[] = $preferences->newTransaction($blame_key, $show_blame); - $xactions[] = $preferences->newTransaction($color_key, $show_color); $editor->applyTransactions($preferences, $xactions); $uri = $request->getRequestURI() - ->alter('blame', null) - ->alter('color', null); + ->alter('blame', null); return id(new AphrontRedirectResponse())->setURI($uri); } - // We need the blame information if blame is on and we're building plain - // text, or blame is on and this is an Ajax request. If blame is on and - // this is a colorized request, we don't show blame at first (we ajax it - // in afterward) so we don't need to query for it. - $needs_blame = ($show_blame && !$show_color) || - ($show_blame && $request->isAjax()); + // We need the blame information if blame is on and this is an Ajax request. + // If blame is on and this is a colorized request, we don't show blame at + // first (we ajax it in afterward) so we don't need to query for it. + $needs_blame = ($show_blame && $request->isAjax()); $params = array( 'commit' => $drequest->getCommit(), @@ -196,10 +187,10 @@ final class DiffusionBrowseController extends DiffusionController { $data = $file->loadFileData(); - $ref = $this->getGitLFSRef($repository, $data); - if ($ref) { + $lfs_ref = $this->getGitLFSRef($repository, $data); + if ($lfs_ref) { if ($view == 'git-lfs') { - $file = $this->loadGitLFSFile($ref); + $file = $this->loadGitLFSFile($lfs_ref); // Rename the file locally so we generate a better vanity URI for // it. In storage, it just has a name like "lfs-13f9a94c0923...", @@ -211,7 +202,7 @@ final class DiffusionBrowseController extends DiffusionController { return $file->getRedirectResponse(); } else { - $corpus = $this->buildGitLFSCorpus($ref); + $corpus = $this->buildGitLFSCorpus($lfs_ref); } } else if (ArcanistDiffUtils::isHeuristicBinaryFile($data)) { $file_uri = $file->getBestURI(); @@ -228,7 +219,6 @@ final class DiffusionBrowseController extends DiffusionController { // Build the content of the file. $corpus = $this->buildCorpus( $show_blame, - $show_color, $data, $needs_blame, $drequest, @@ -248,8 +238,7 @@ final class DiffusionBrowseController extends DiffusionController { $curtain = $this->enrichCurtain( $view, $drequest, - $show_blame, - $show_color); + $show_blame); $properties = $this->buildPropertyView($drequest); $header = $this->buildHeaderView($drequest); @@ -647,7 +636,6 @@ final class DiffusionBrowseController extends DiffusionController { private function buildCorpus( $show_blame, - $show_color, $file_corpus, $needs_blame, DiffusionRequest $drequest, @@ -675,91 +663,82 @@ final class DiffusionBrowseController extends DiffusionController { $blame_commits = array(); } - if (!$show_color) { - $corpus = $this->renderPlaintextCorpus( - $file_corpus, - $blame_list, - $blame_commits, - $show_blame); + require_celerity_resource('syntax-highlighting-css'); + if ($can_highlight) { + $highlighted = PhabricatorSyntaxHighlighter::highlightWithFilename( + $path, + $file_corpus); } else { - require_celerity_resource('syntax-highlighting-css'); - - if ($can_highlight) { - $highlighted = PhabricatorSyntaxHighlighter::highlightWithFilename( - $path, - $file_corpus); - } else { - // Highlight as plain text to escape the content properly. - $highlighted = PhabricatorSyntaxHighlighter::highlightWithLanguage( - 'txt', - $file_corpus); - } - - $lines = phutil_split_lines($highlighted); - - $rows = $this->buildDisplayRows( - $lines, - $blame_list, - $blame_commits, - $show_blame, - $show_color); - - $corpus_table = javelin_tag( - 'table', - array( - 'class' => 'diffusion-source remarkup-code PhabricatorMonospaced', - 'sigil' => 'phabricator-source', - ), - $rows); - - if ($this->getRequest()->isAjax()) { - return $corpus_table; - } - - $id = celerity_generate_unique_node_id(); - - $repo = $drequest->getRepository(); - $symbol_repos = nonempty($repo->getSymbolSources(), array()); - $symbol_repos[] = $repo->getPHID(); - - $lang = last(explode('.', $drequest->getPath())); - $repo_languages = $repo->getSymbolLanguages(); - $repo_languages = nonempty($repo_languages, array()); - $repo_languages = array_fill_keys($repo_languages, true); - - $needs_symbols = true; - if ($repo_languages && $symbol_repos) { - $have_symbols = id(new DiffusionSymbolQuery()) - ->existsSymbolsInRepository($repo->getPHID()); - if (!$have_symbols) { - $needs_symbols = false; - } - } - - if ($needs_symbols && $repo_languages) { - $needs_symbols = isset($repo_languages[$lang]); - } - - if ($needs_symbols) { - Javelin::initBehavior( - 'repository-crossreference', - array( - 'container' => $id, - 'lang' => $lang, - 'repositories' => $symbol_repos, - )); - } - - $corpus = phutil_tag( - 'div', - array( - 'id' => $id, - ), - $corpus_table); - - Javelin::initBehavior('load-blame', array('id' => $id)); + // Highlight as plain text to escape the content properly. + $highlighted = PhabricatorSyntaxHighlighter::highlightWithLanguage( + 'txt', + $file_corpus); } + $lines = phutil_split_lines($highlighted); + + $rows = $this->buildDisplayRows( + $lines, + $blame_list, + $blame_commits, + $show_blame); + + $corpus_table = javelin_tag( + 'table', + array( + 'class' => 'diffusion-source remarkup-code PhabricatorMonospaced', + 'sigil' => 'phabricator-source', + ), + $rows); + + if ($this->getRequest()->isAjax()) { + return $corpus_table; + } + + $id = celerity_generate_unique_node_id(); + + $repo = $drequest->getRepository(); + $symbol_repos = nonempty($repo->getSymbolSources(), array()); + $symbol_repos[] = $repo->getPHID(); + + $lang = last(explode('.', $drequest->getPath())); + $repo_languages = $repo->getSymbolLanguages(); + $repo_languages = nonempty($repo_languages, array()); + $repo_languages = array_fill_keys($repo_languages, true); + + $needs_symbols = true; + if ($repo_languages && $symbol_repos) { + $have_symbols = id(new DiffusionSymbolQuery()) + ->existsSymbolsInRepository($repo->getPHID()); + if (!$have_symbols) { + $needs_symbols = false; + } + } + + if ($needs_symbols && $repo_languages) { + $needs_symbols = isset($repo_languages[$lang]); + } + + if ($needs_symbols) { + Javelin::initBehavior( + 'repository-crossreference', + array( + 'container' => $id, + 'lang' => $lang, + 'repositories' => $symbol_repos, + )); + } + + $corpus = phutil_tag( + 'div', + array( + 'id' => $id, + ), + $corpus_table); + + Javelin::initBehavior('load-blame', array('id' => $id)); + + $edit = $this->renderEditButton(); $file = $this->renderFileButton(); $header = id(new PHUIHeaderView()) @@ -808,8 +787,7 @@ final class DiffusionBrowseController extends DiffusionController { private function enrichCurtain( PHUICurtainView $curtain, DiffusionRequest $drequest, - $show_blame, - $show_color) { + $show_blame) { $viewer = $this->getViewer(); $base_uri = $this->getRequest()->getRequestURI(); @@ -842,24 +820,6 @@ final class DiffusionBrowseController extends DiffusionController { ->setUser($viewer) ->setRenderAsForm($viewer->isLoggedIn())); - if ($show_color) { - $highlight_text = pht('Disable Highlighting'); - $highlight_icon = 'fa-star-o grey'; - $highlight_value = 0; - } else { - $highlight_text = pht('Enable Highlighting'); - $highlight_icon = 'fa-star'; - $highlight_value = 1; - } - - $curtain->addAction( - id(new PhabricatorActionView()) - ->setName($highlight_text) - ->setHref($base_uri->alter('color', $highlight_value)) - ->setIcon($highlight_icon) - ->setUser($viewer) - ->setRenderAsForm($viewer->isLoggedIn())); - $href = null; if ($this->getRequest()->getStr('lint') !== null) { $lint_text = pht('Hide %d Lint Message(s)', count($this->lintMessages)); @@ -1002,8 +962,7 @@ final class DiffusionBrowseController extends DiffusionController { array $lines, array $blame_list, array $blame_commits, - $show_blame, - $show_color) { + $show_blame) { $request = $this->getRequest(); $viewer = $this->getViewer(); @@ -1891,79 +1850,6 @@ final class DiffusionBrowseController extends DiffusionController { return $links; } - private function renderPlaintextCorpus( - $file_corpus, - array $blame_list, - array $blame_commits, - $show_blame) { - - $viewer = $this->getViewer(); - - if (!$show_blame) { - $corpus = $file_corpus; - } else { - $author_phids = array(); - foreach ($blame_commits as $commit) { - $author_phid = $commit->getAuthorPHID(); - if ($author_phid === null) { - continue; - } - $author_phids[$author_phid] = $author_phid; - } - - if ($author_phids) { - $handles = $viewer->loadHandles($author_phids); - } else { - $handles = array(); - } - - $authors = array(); - $names = array(); - foreach ($blame_commits as $identifier => $commit) { - $author = $commit->renderAuthorShortName($handles); - $name = $commit->getLocalName(); - - $authors[$identifier] = $author; - $names[$identifier] = $name; - } - - $lines = phutil_split_lines($file_corpus); - - $rows = array(); - foreach ($lines as $line_number => $line) { - $commit_name = null; - $author = null; - - if (isset($blame_list[$line_number])) { - $identifier = $blame_list[$line_number]; - - if (isset($names[$identifier])) { - $commit_name = $names[$identifier]; - } - - if (isset($authors[$identifier])) { - $author = $authors[$identifier]; - } - } - - $rows[] = sprintf( - '%-10s %-20s %s', - $commit_name, - $author, - $line); - } - $corpus = implode('', $rows); - } - - return phutil_tag( - 'textarea', - array( - 'style' => 'border: none; width: 100%; height: 80em; '. - 'font-family: monospace', - ), - $corpus); - } - private function getGitLFSRef(PhabricatorRepository $repository, $data) { if (!$repository->canUseGitLFS()) { return null; diff --git a/src/applications/settings/setting/PhabricatorDiffusionColorSetting.php b/src/applications/settings/setting/PhabricatorDiffusionColorSetting.php deleted file mode 100644 index 6fdd6cade9..0000000000 --- a/src/applications/settings/setting/PhabricatorDiffusionColorSetting.php +++ /dev/null @@ -1,16 +0,0 @@ - Date: Wed, 9 Nov 2016 16:31:30 -0800 Subject: [PATCH 22/31] Normalize form inputs a little Summary: Lots of minor details, colors, spacing, padding. Test Plan: View carefully in create task, mobile create task, comment form, tokenizers. {F1913060} Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16832 --- resources/celerity/map.php | 30 ++++++++++----------- webroot/rsrc/css/aphront/tokenizer.css | 5 +++- webroot/rsrc/css/core/remarkup.css | 26 +++++++++++------- webroot/rsrc/css/phui/phui-comment-form.css | 29 -------------------- webroot/rsrc/css/phui/phui-form-view.css | 3 ++- webroot/rsrc/css/phui/phui-form.css | 21 ++++++++++----- 6 files changed, 51 insertions(+), 63 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index e5d5786142..16c77ba644 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', - 'core.pkg.css' => 'd2126ffb', + 'core.pkg.css' => 'e7c00111', 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', @@ -29,7 +29,7 @@ return array( 'rsrc/css/aphront/panel-view.css' => '8427b78d', 'rsrc/css/aphront/phabricator-nav-view.css' => 'b29426e9', 'rsrc/css/aphront/table-view.css' => '3225137a', - 'rsrc/css/aphront/tokenizer.css' => '056da01b', + 'rsrc/css/aphront/tokenizer.css' => '9a8cb501', 'rsrc/css/aphront/tooltip.css' => '1a07aea8', 'rsrc/css/aphront/typeahead-browse.css' => '8904346a', 'rsrc/css/aphront/typeahead.css' => 'd4f16145', @@ -109,7 +109,7 @@ return array( 'rsrc/css/application/tokens/tokens.css' => '3d0f239e', 'rsrc/css/application/uiexample/example.css' => '528b19de', 'rsrc/css/core/core.css' => 'd0801452', - 'rsrc/css/core/remarkup.css' => '04ab9f17', + 'rsrc/css/core/remarkup.css' => 'dda84e80', 'rsrc/css/core/syntax.css' => '769d3498', 'rsrc/css/core/z-index.css' => 'd1270942', 'rsrc/css/diviner/diviner-shared.css' => 'aa3656aa', @@ -132,7 +132,7 @@ return array( 'rsrc/css/phui/phui-button.css' => '4a5fbe3d', 'rsrc/css/phui/phui-chart.css' => '6bf6f78e', 'rsrc/css/phui/phui-cms.css' => 'be43c8a8', - 'rsrc/css/phui/phui-comment-form.css' => '103cdc49', + 'rsrc/css/phui/phui-comment-form.css' => '4ecc56ef', 'rsrc/css/phui/phui-crumbs-view.css' => '195ac419', 'rsrc/css/phui/phui-curtain-view.css' => '947bf1a4', 'rsrc/css/phui/phui-document-pro.css' => 'ca1fed81', @@ -140,8 +140,8 @@ return array( 'rsrc/css/phui/phui-document.css' => 'c32e8dec', 'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9', 'rsrc/css/phui/phui-fontkit.css' => '9cda225e', - 'rsrc/css/phui/phui-form-view.css' => 'b5bfd17a', - 'rsrc/css/phui/phui-form.css' => 'aac1d51d', + 'rsrc/css/phui/phui-form-view.css' => '91adabe4', + 'rsrc/css/phui/phui-form.css' => 'ca2490c6', 'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f', 'rsrc/css/phui/phui-header-view.css' => '6ec8f155', 'rsrc/css/phui/phui-hovercard.css' => 'de1a2119', @@ -558,7 +558,7 @@ return array( 'aphront-multi-column-view-css' => '84cc6640', 'aphront-panel-view-css' => '8427b78d', 'aphront-table-view-css' => '3225137a', - 'aphront-tokenizer-control-css' => '056da01b', + 'aphront-tokenizer-control-css' => '9a8cb501', 'aphront-tooltip-css' => '1a07aea8', 'aphront-typeahead-control-css' => 'd4f16145', 'application-search-view-css' => '8452c849', @@ -812,7 +812,7 @@ return array( 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', 'phabricator-prefab' => '8d40ae75', - 'phabricator-remarkup-css' => '04ab9f17', + 'phabricator-remarkup-css' => 'dda84e80', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', @@ -855,7 +855,7 @@ return array( 'phui-calendar-month-css' => '8e10e92c', 'phui-chart-css' => '6bf6f78e', 'phui-cms-css' => 'be43c8a8', - 'phui-comment-form-css' => '103cdc49', + 'phui-comment-form-css' => '4ecc56ef', 'phui-crumbs-view-css' => '195ac419', 'phui-curtain-view-css' => '947bf1a4', 'phui-document-summary-view-css' => '9ca48bdf', @@ -864,8 +864,8 @@ return array( 'phui-feed-story-css' => '44a9c8e9', 'phui-font-icon-base-css' => '870a7360', 'phui-fontkit-css' => '9cda225e', - 'phui-form-css' => 'aac1d51d', - 'phui-form-view-css' => 'b5bfd17a', + 'phui-form-css' => 'ca2490c6', + 'phui-form-view-css' => '91adabe4', 'phui-head-thing-view-css' => 'fd311e5f', 'phui-header-view-css' => '6ec8f155', 'phui-hovercard' => '1bd28176', @@ -953,10 +953,6 @@ return array( 'javelin-util', 'phabricator-tooltip', ), - '056da01b' => array( - 'aphront-typeahead-control-css', - 'phui-tag-view-css', - ), '065227cc' => array( 'javelin-behavior', 'javelin-dom', @@ -1750,6 +1746,10 @@ return array( 'phabricator-phtize', 'changeset-view-manager', ), + '9a8cb501' => array( + 'aphront-typeahead-control-css', + 'phui-tag-view-css', + ), '9bbf3762' => array( 'javelin-behavior', 'javelin-dom', diff --git a/webroot/rsrc/css/aphront/tokenizer.css b/webroot/rsrc/css/aphront/tokenizer.css index 0e10c94d26..7116ce3ace 100644 --- a/webroot/rsrc/css/aphront/tokenizer.css +++ b/webroot/rsrc/css/aphront/tokenizer.css @@ -38,6 +38,7 @@ body input.jx-tokenizer-input { font-size: {$normalfontsize}; color: #333; height: 26px; + background: transparent; } body input.jx-tokenizer-input:focus { @@ -63,7 +64,7 @@ a.jx-tokenizer-x:hover { } a.jx-tokenizer-token { - padding: 2px 6px; + padding: 1px 6px; border: 1px solid {$lightblueborder}; margin: 3px 2px 0 4px; background-color: {$sh-bluebackground}; @@ -173,6 +174,8 @@ a.jx-tokenizer-token-invalid:hover { border-bottom: none; padding: 0; width: 30px; + border-top-right-radius: 3px; + border-bottom-right-radius: 3px; } .button.tokenizer-browse-button .phui-icon-view { diff --git a/webroot/rsrc/css/core/remarkup.css b/webroot/rsrc/css/core/remarkup.css index 48d06d4d5f..7d084f9a23 100644 --- a/webroot/rsrc/css/core/remarkup.css +++ b/webroot/rsrc/css/core/remarkup.css @@ -436,11 +436,13 @@ body div.phabricator-remarkup > *:last-child { } .remarkup-assist-textarea { - border-left-color: {$blueborder}; - border-right-color: {$blueborder}; - border-bottom-color: {$blueborder}; + border-left-color: {$greyborder}; + border-right-color: {$greyborder}; + border-bottom-color: {$greyborder}; border-top-color: {$thinblueborder}; border-radius: 0; + border-bottom-left-radius: 3px; + border-bottom-right-radius: 3px; box-shadow: none; -webkit-box-shadow: none; @@ -470,12 +472,15 @@ var.remarkup-assist-textarea { } .remarkup-assist-bar { - height: 26px; + height: 32px; border-width: 1px 1px 0; border-style: solid; - border-top-color: {$blueborder}; - border-left-color: {$blueborder}; - border-right-color: {$blueborder}; + border-top-color: {$greyborder}; + border-left-color: {$greyborder}; + border-right-color: {$greyborder}; + border-top-left-radius: 3px; + border-top-right-radius: 3px; + padding: 0 4px; background: {$lightbluebackground}; overflow: hidden; @@ -483,6 +488,7 @@ var.remarkup-assist-textarea { .remarkup-assist-button { display: block; + margin-top: 2px; padding: 4px 5px; float: left; } @@ -503,11 +509,11 @@ var.remarkup-assist-textarea { display: block; float: left; - margin: 7px 4px; - height: 14px; + height: 18px; + margin: 7px 6px; width: 0px; - border-right: 1px solid #cccccc; + border-right: 1px solid {$lightgreyborder}; } .remarkup-interpreter-error { diff --git a/webroot/rsrc/css/phui/phui-comment-form.css b/webroot/rsrc/css/phui/phui-comment-form.css index 1fca450d97..505ea17359 100644 --- a/webroot/rsrc/css/phui/phui-comment-form.css +++ b/webroot/rsrc/css/phui/phui-comment-form.css @@ -58,23 +58,10 @@ body.device .phui-box.phui-object-box.phui-comment-form-view { border-radius: 0; } -.phui-comment-form-view .remarkup-assist-button { - margin-top: 2px; - padding: 4px 5px; - border-radius: 3px; -} - -.phui-comment-form-view .remarkup-assist-separator { - height: 18px; - margin: 7px 6px; -} - .phui-comment-form-view .aphront-form-input .remarkup-assist-textarea { border-color: {$lightblueborder}; border-top: 1px solid {$thinblueborder}; padding: 8px; - border-bottom-left-radius: 3px; - border-bottom-right-radius: 3px; height: 10em; background-color: rgba({$alphablue},.02); } @@ -162,22 +149,6 @@ body.device .phui-box.phui-object-box.phui-comment-form-view { display: none; } -/* h8rs gonna h8 */ -.phui-comment-form-view select { - -webkit-appearance: none; - -moz-appearance: none; - appearance: none; - - background: #fff url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAUCAMAAACzvE1FAAAADFBMVEUzMzMzMzMzMzMzMzMKAG/3AAAAA3RSTlMAf4C/aSLHAAAAPElEQVR42q3NMQ4AIAgEQTn//2cLdRKppSGzBYwzVXvznNWs8C58CiussPJj8h6NwgorrKRdTvuV9v16Afn0AYFOB7aYAAAAAElFTkSuQmCC") no-repeat right 8px center; - background-size: 8px 10px; - border-radius: 3px; - color: {$darkbluetext}; - height: 28px; - position: relative; - padding: 0 8px; - width: 180px; -} - .device-phone .aphront-form-control-submit button { width: 100%; } diff --git a/webroot/rsrc/css/phui/phui-form-view.css b/webroot/rsrc/css/phui/phui-form-view.css index 91e7b64dcf..11852a87f6 100644 --- a/webroot/rsrc/css/phui/phui-form-view.css +++ b/webroot/rsrc/css/phui/phui-form-view.css @@ -15,8 +15,9 @@ } .phui-form-view label.aphront-form-label { - padding-top: 5px; width: 19%; + height: 28px; + line-height: 28px; float: left; text-align: right; font-weight: bold; diff --git a/webroot/rsrc/css/phui/phui-form.css b/webroot/rsrc/css/phui/phui-form.css index 5c2ef38446..4150ac6940 100644 --- a/webroot/rsrc/css/phui/phui-form.css +++ b/webroot/rsrc/css/phui/phui-form.css @@ -46,7 +46,8 @@ input[type="color"], div.jx-tokenizer-container { padding: 4px 6px; background-color: #ffffff; - border: 1px solid {$blueborder}; + border: 1px solid {$greyborder}; + border-radius: 3px; -webkit-box-shadow: inset 0 1px 1px rgba({$alphablack}, 0.075); -moz-box-shadow: inset 0 1px 1px rgba({$alphablack}, 0.075); @@ -63,7 +64,6 @@ div.jx-tokenizer-container { /* iOS Safari */ -webkit-appearance: none; - border-radius: 0; } textarea:focus, @@ -105,11 +105,18 @@ input[type="checkbox"] { } select { - height: 24px; - line-height: 24px; - border: 1px solid {$lightgreyborder}; - background-color: #ffffff; - min-width: 140px; + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; + + background: #fff url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAUCAMAAACzvE1FAAAADFBMVEUzMzMzMzMzMzMzMzMKAG/3AAAAA3RSTlMAf4C/aSLHAAAAPElEQVR42q3NMQ4AIAgEQTn//2cLdRKppSGzBYwzVXvznNWs8C58CiussPJj8h6NwgorrKRdTvuV9v16Afn0AYFOB7aYAAAAAElFTkSuQmCC") no-repeat right 8px center; + background-size: 8px 10px; + border-radius: 3px; + color: {$darkbluetext}; + border: 1px solid {$greyborder}; + height: 28px; + padding: 0 8px; + min-width: 180px; } select[multiple], From 663629e8ad9d050110294fa456307a9d300e85b7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 9 Nov 2016 17:05:41 -0800 Subject: [PATCH 23/31] =?UTF-8?q?Use=20Doritos=E2=84=A2=20Brand=C2=AE=20pe?= =?UTF-8?q?rfect=20circles=20to=20indicate=20Busy/Away/Disabled?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Fixes T11829. - Currently in use: Doritos™ Brand® "Nacho Cheese"® perfect circles: • - Available alternative: Doritos™ Brand® "Cool Ranch"® perfect circles: ● Test Plan: {F1913116} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11829 Differential Revision: https://secure.phabricator.com/D16834 --- .../phid/PhabricatorObjectHandle.php | 12 ++++++++++- .../application/base/standard-page-view.css | 20 ++++++------------ .../rsrc/image/icon/fatcow/bullet_black.png | Bin 230 -> 0 bytes .../rsrc/image/icon/fatcow/bullet_orange.png | Bin 232 -> 0 bytes webroot/rsrc/image/icon/fatcow/bullet_red.png | Bin 230 -> 0 bytes 5 files changed, 17 insertions(+), 15 deletions(-) delete mode 100644 webroot/rsrc/image/icon/fatcow/bullet_black.png delete mode 100644 webroot/rsrc/image/icon/fatcow/bullet_orange.png delete mode 100644 webroot/rsrc/image/icon/fatcow/bullet_red.png diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index f3c04d2a25..943464ed67 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -314,8 +314,18 @@ final class PhabricatorObjectHandle $classes[] = 'handle-status-'.$this->status; } + $circle = null; if ($this->availability != self::AVAILABILITY_FULL) { $classes[] = 'handle-availability-'.$this->availability; + $circle = array( + phutil_tag( + 'span', + array( + 'class' => 'perfect-circle', + ), + "\xE2\x80\xA2"), + ' ', + ); } if ($this->getType() == PhabricatorPeopleUserPHIDType::TYPECONST) { @@ -339,7 +349,7 @@ final class PhabricatorObjectHandle return javelin_tag( $uri ? 'a' : 'span', $attributes, - array($icon, $name)); + array($circle, $icon, $name)); } public function renderTag() { diff --git a/webroot/rsrc/css/application/base/standard-page-view.css b/webroot/rsrc/css/application/base/standard-page-view.css index 36294ca288..ebcf157ad4 100644 --- a/webroot/rsrc/css/application/base/standard-page-view.css +++ b/webroot/rsrc/css/application/base/standard-page-view.css @@ -76,24 +76,16 @@ a.handle-status-closed:hover { color: #19558D; } -a.handle-availability-disabled, -a.handle-availability-none, -a.handle-availability-partial { - padding-left: 11px; - background-repeat: no-repeat; - background-position: -4px center; +.handle-availability-none .perfect-circle { + color: {$red}; } -a.handle-availability-none { - background-image: url(/rsrc/image/icon/fatcow/bullet_red.png); +.handle-availability-partial .perfect-circle { + color: {$orange}; } -a.handle-availability-partial { - background-image: url(/rsrc/image/icon/fatcow/bullet_orange.png); -} - -a.handle-availability-disabled { - background-image: url(/rsrc/image/icon/fatcow/bullet_black.png); +.handle-availability-disabled .perfect-circle { + color: {$greytext}; } .aphront-developer-error-callout { diff --git a/webroot/rsrc/image/icon/fatcow/bullet_black.png b/webroot/rsrc/image/icon/fatcow/bullet_black.png deleted file mode 100644 index a113d8ef3b385b8ef62dcf67bce1c9e148bc6180..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 230 zcmeAS@N?(olHy`uVBq!ia0vp^d_c_4!3HF+i2N%7Qj#UE5hcO-X(i=}MX3yqDfvmM z3ZA)%>8U}fi7AzZCsS>JiaI=9978H@CH?vT-=5jBp|jD)VVj#+ieFY1SKzF)^YfZ7 zyLl_vu^oBe_(MqG_=By2Tyq*K#089Pe z{`tptTbS%)dgTAZq>RCETgXMH6Dw~w+~Dq5@A$L7(L%gKU6?CW;0EK-EB=-}CH_ws b7}*#O>q?$o`$lRX&`k`Uu6{1-oD!M<-fU8z diff --git a/webroot/rsrc/image/icon/fatcow/bullet_orange.png b/webroot/rsrc/image/icon/fatcow/bullet_orange.png deleted file mode 100644 index dacfd4697fe27f5c3bef46bd689ae67fa1a9c7ee..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 232 zcmeAS@N?(olHy`uVBq!ia0vp^d_c_4!3HF+i2N%7Qj#UE5hcO-X(i=}MX3yqDfvmM z3ZA)%>8U}fi7AzZCsS>Jin=^q978H@CH?vT-=5jBp)kSJZhg3#*W{GPv21EGMbjK= zcslMge^gcIa|}Pwh5ID>Tf_-F?hQAxvX8U}fi7AzZCsS>JiaI=9978H@CH?vT-=5jBp|jD)VVj#+$~i}E*+nKd<##t- zcJo%SV>|M`@rRJW@dsN4x#l!fhzl4y|73F5BhhiQp+e5iK1?~&iOsi#$(_0V^U}r{ z{PU0PwlLYp^vM5*Ng0FTwvdZXCsy8WxWV1A-tlLDqlI{fx-eI$zzxQuSNtt|O8lQN bFtRZm5|uoAq<57b&`k`Uu6{1-oD!M<)ud80 From 63c2665df1671edc800e311832c90071e547e4ab Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 9 Nov 2016 17:15:56 -0800 Subject: [PATCH 24/31] =?UTF-8?q?Vegetables=3F=20Dortios=E2=84=A2=20Brand?= =?UTF-8?q?=C2=AE=C2=A0snack=20chips=20are=20healthy=20corn=20chips=20pack?= =?UTF-8?q?ed=20full=20of=20vitamins=20and=20nutrients.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- resources/celerity/map.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 16c77ba644..24b1db4f59 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', - 'core.pkg.css' => 'e7c00111', + 'core.pkg.css' => 'f7ab1e0b', 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', @@ -39,7 +39,7 @@ return array( 'rsrc/css/application/base/notification-menu.css' => '1e055865', 'rsrc/css/application/base/phabricator-application-launch-view.css' => '95351601', 'rsrc/css/application/base/phui-theme.css' => '798c69b8', - 'rsrc/css/application/base/standard-page-view.css' => '79176f5a', + 'rsrc/css/application/base/standard-page-view.css' => '894d8a25', 'rsrc/css/application/chatlog/chatlog.css' => 'd295b020', 'rsrc/css/application/conduit/conduit-api.css' => '7bc725c4', 'rsrc/css/application/config/config-options.css' => '0ede4c9b', @@ -302,9 +302,6 @@ return array( 'rsrc/image/grippy_texture.png' => 'aca81e2f', 'rsrc/image/icon/fatcow/arrow_branch.png' => '2537c01c', 'rsrc/image/icon/fatcow/arrow_merge.png' => '21b660e0', - 'rsrc/image/icon/fatcow/bullet_black.png' => 'ff190031', - 'rsrc/image/icon/fatcow/bullet_orange.png' => 'e273e5bb', - 'rsrc/image/icon/fatcow/bullet_red.png' => 'c0b75434', 'rsrc/image/icon/fatcow/calendar_edit.png' => '24632275', 'rsrc/image/icon/fatcow/document_black.png' => '45fe1c60', 'rsrc/image/icon/fatcow/flag_blue.png' => 'a01abb1d', @@ -817,7 +814,7 @@ return array( 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', 'phabricator-source-code-view-css' => 'cbeef983', - 'phabricator-standard-page-view' => '79176f5a', + 'phabricator-standard-page-view' => '894d8a25', 'phabricator-textareautils' => '320810c8', 'phabricator-title' => '485aaa6c', 'phabricator-tooltip' => '6323f942', From 6dc94fc10a3b4edecb8001af3f48dbde8c551bb9 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 9 Nov 2016 22:09:48 -0800 Subject: [PATCH 25/31] Clean up fullscreen remarkup UI Summary: Some aftermath fallout from rebuilding the comment box. Test Plan: Go fullscreen, pop back out. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16836 --- resources/celerity/map.php | 6 +++--- webroot/rsrc/css/core/remarkup.css | 12 +++++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 24b1db4f59..a3f2c56e98 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', - 'core.pkg.css' => 'f7ab1e0b', + 'core.pkg.css' => '9d155da1', 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', @@ -109,7 +109,7 @@ return array( 'rsrc/css/application/tokens/tokens.css' => '3d0f239e', 'rsrc/css/application/uiexample/example.css' => '528b19de', 'rsrc/css/core/core.css' => 'd0801452', - 'rsrc/css/core/remarkup.css' => 'dda84e80', + 'rsrc/css/core/remarkup.css' => 'e70ca862', 'rsrc/css/core/syntax.css' => '769d3498', 'rsrc/css/core/z-index.css' => 'd1270942', 'rsrc/css/diviner/diviner-shared.css' => 'aa3656aa', @@ -809,7 +809,7 @@ return array( 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', 'phabricator-prefab' => '8d40ae75', - 'phabricator-remarkup-css' => 'dda84e80', + 'phabricator-remarkup-css' => 'e70ca862', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', diff --git a/webroot/rsrc/css/core/remarkup.css b/webroot/rsrc/css/core/remarkup.css index 7d084f9a23..fe4a718331 100644 --- a/webroot/rsrc/css/core/remarkup.css +++ b/webroot/rsrc/css/core/remarkup.css @@ -557,7 +557,7 @@ var.remarkup-assist-textarea { .remarkup-control-fullscreen-mode textarea.remarkup-assist-textarea { position: absolute; - top: 27px; + top: 32px; left: 0; right: 0; bottom: 0; @@ -566,6 +566,16 @@ var.remarkup-assist-textarea { border-width: 1px 0 0 0; outline: none; resize: none; + background: #fff !important; +} + +.remarkup-control-fullscreen-mode textarea.remarkup-assist-textarea:focus { + border-color: none; + box-shadow: none; +} + +.remarkup-control-fullscreen-mode .remarkup-assist-button .fa-arrows-alt { + color: {$sky}; } .phabricator-image-macro-hero { From 9a1d59ad5bd51ef4bf56533db260cc9e11a54fe6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Nov 2016 07:24:15 -0800 Subject: [PATCH 26/31] Separate sever-side typeahead queries into "prefix" and "content" phases Summary: Ref T8510. When users type "platypus" into a typeahead, they want "Platypus Playground" to be a higher-ranked match than "AAA Platypus", even though the latter is alphabetically first. Specifically, the rule is: results which match the query as a prefix of the result text should rank above results which do not. I believe we now always get this right on the client side. However, WMF has at least one case (described in T8510) where we do not get it right on the server side, and thus the user sees the wrong result. The remaining issue is that if "platypus" matches more than 100 results, the result "Platypus Playground" may not appear in the result set at all, beacuse there are 100 copies of "AAA Platypus 1", "AAA Platypus 2", etc., first. So even though the client will apply the correct sort, it doesn't have the result the user wants and can't show it to them. To fix this, split the server-side query into two phases: - In the first phase, the "prefix" phase, we find results that **start with** "platypus". - In the second phase, the "content" phase, we find results that contain "platypus" anywhere. We skip the "prefix" phase if the user has not typed a query (for example, in the browse view). Test Plan: This is a lot of stuff, but the new ranking here puts projects which start with "w" at the top of the list. Lower down the list, you can see some projects which contain "w" but do not appear at the top (like "Serious Work"). {F1913931} Reviewers: chad Reviewed By: chad Maniphest Tasks: T8510 Differential Revision: https://secure.phabricator.com/D16838 --- .../people/query/PhabricatorPeopleQuery.php | 17 +++ .../typeahead/PhabricatorPeopleDatasource.php | 11 +- .../project/query/PhabricatorProjectQuery.php | 17 +++ .../PhabricatorProjectDatasource.php | 5 +- ...orTypeaheadModularDatasourceController.php | 5 + ...habricatorTypeaheadCompositeDatasource.php | 140 +++++++++++++++++- .../PhabricatorTypeaheadDatasource.php | 24 +++ .../storage/PhabricatorTypeaheadResult.php | 11 ++ 8 files changed, 219 insertions(+), 11 deletions(-) diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php index 88e2437dbd..32ae31859b 100644 --- a/src/applications/people/query/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -17,6 +17,7 @@ final class PhabricatorPeopleQuery private $isApproved; private $nameLike; private $nameTokens; + private $namePrefixes; private $needPrimaryEmail; private $needProfile; @@ -95,6 +96,11 @@ final class PhabricatorPeopleQuery return $this; } + public function withNamePrefixes(array $prefixes) { + $this->namePrefixes = $prefixes; + return $this; + } + public function needPrimaryEmail($need) { $this->needPrimaryEmail = $need; return $this; @@ -256,6 +262,17 @@ final class PhabricatorPeopleQuery $this->usernames); } + if ($this->namePrefixes) { + $parts = array(); + foreach ($this->namePrefixes as $name_prefix) { + $parts[] = qsprintf( + $conn, + 'user.username LIKE %>', + $name_prefix); + } + $where[] = '('.implode(' OR ', $parts).')'; + } + if ($this->emails !== null) { $where[] = qsprintf( $conn, diff --git a/src/applications/people/typeahead/PhabricatorPeopleDatasource.php b/src/applications/people/typeahead/PhabricatorPeopleDatasource.php index 494b68dbfb..df146808bb 100644 --- a/src/applications/people/typeahead/PhabricatorPeopleDatasource.php +++ b/src/applications/people/typeahead/PhabricatorPeopleDatasource.php @@ -17,13 +17,18 @@ final class PhabricatorPeopleDatasource public function loadResults() { $viewer = $this->getViewer(); - $tokens = $this->getTokens(); $query = id(new PhabricatorPeopleQuery()) ->setOrderVector(array('username')); - if ($tokens) { - $query->withNameTokens($tokens); + if ($this->getPhase() == self::PHASE_PREFIX) { + $prefix = $this->getPrefixQuery(); + $query->withNamePrefixes(array($prefix)); + } else { + $tokens = $this->getTokens(); + if ($tokens) { + $query->withNameTokens($tokens); + } } $users = $this->executeQuery($query); diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index ab41ecf0ac..87c6bb805e 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -12,6 +12,7 @@ final class PhabricatorProjectQuery private $slugMap; private $allSlugs; private $names; + private $namePrefixes; private $nameTokens; private $icons; private $colors; @@ -78,6 +79,11 @@ final class PhabricatorProjectQuery return $this; } + public function withNamePrefixes(array $prefixes) { + $this->namePrefixes = $prefixes; + return $this; + } + public function withNameTokens(array $tokens) { $this->nameTokens = array_values($tokens); return $this; @@ -464,6 +470,17 @@ final class PhabricatorProjectQuery $this->names); } + if ($this->namePrefixes) { + $parts = array(); + foreach ($this->namePrefixes as $name_prefix) { + $parts[] = qsprintf( + $conn, + 'name LIKE %>', + $name_prefix); + } + $where[] = '('.implode(' OR ', $parts).')'; + } + if ($this->icons !== null) { $where[] = qsprintf( $conn, diff --git a/src/applications/project/typeahead/PhabricatorProjectDatasource.php b/src/applications/project/typeahead/PhabricatorProjectDatasource.php index 03c2424f4a..03a6e39c33 100644 --- a/src/applications/project/typeahead/PhabricatorProjectDatasource.php +++ b/src/applications/project/typeahead/PhabricatorProjectDatasource.php @@ -28,7 +28,10 @@ final class PhabricatorProjectDatasource ->needImages(true) ->needSlugs(true); - if ($tokens) { + if ($this->getPhase() == self::PHASE_PREFIX) { + $prefix = $this->getPrefixQuery(); + $query->withNamePrefixes(array($prefix)); + } else if ($tokens) { $query->withNameTokens($tokens); } diff --git a/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php b/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php index a7c8381bfd..5c641b7c1f 100644 --- a/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php +++ b/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php @@ -325,6 +325,11 @@ final class PhabricatorTypeaheadModularDatasourceController pht('Icon'), pht('Closed'), pht('Sprite'), + pht('Color'), + pht('Type'), + pht('Unique'), + pht('Auto'), + pht('Phase'), )); $result_box = id(new PHUIObjectBoxView()) diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php index 306b33b497..33f06e4ae5 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php @@ -4,6 +4,8 @@ abstract class PhabricatorTypeaheadCompositeDatasource extends PhabricatorTypeaheadDatasource { private $usable; + private $prefixString; + private $prefixLength; abstract public function getComponentDatasources(); @@ -22,9 +24,51 @@ abstract class PhabricatorTypeaheadCompositeDatasource } public function loadResults() { + $phases = array(); + + // We only need to do a prefix phase query if there's an actual query + // string. If the user didn't type anything, nothing can possibly match it. + if (strlen($this->getRawQuery())) { + $phases[] = self::PHASE_PREFIX; + } + + $phases[] = self::PHASE_CONTENT; + $offset = $this->getOffset(); $limit = $this->getLimit(); + $results = array(); + foreach ($phases as $phase) { + if ($limit) { + $phase_limit = ($offset + $limit) - count($results); + } else { + $phase_limit = 0; + } + + $phase_results = $this->loadResultsForPhase( + $phase, + $phase_limit); + + foreach ($phase_results as $result) { + $results[] = $result; + } + + if ($limit) { + if (count($results) >= $offset + $limit) { + break; + } + } + } + + return $results; + } + + protected function loadResultsForPhase($phase, $limit) { + if ($phase == self::PHASE_PREFIX) { + $this->prefixString = $this->getPrefixQuery(); + $this->prefixLength = strlen($this->prefixString); + } + // If the input query is a function like `members(platy`, and we can // parse the function, we strip the function off and hand the stripped // query to child sources. This makes it easier to implement function @@ -62,28 +106,110 @@ abstract class PhabricatorTypeaheadCompositeDatasource } $source + ->setPhase($phase) ->setFunctionStack($source_stack) ->setRawQuery($source_query) ->setQuery($this->getQuery()) ->setViewer($this->getViewer()); - if ($limit) { - $source->setLimit($offset + $limit); - } - if ($is_browse) { $source->setIsBrowse(true); } - $source_results = $source->loadResults(); - $source_results = $source->didLoadResults($source_results); + if ($limit) { + // If we are loading results from a source with a limit, it may return + // some results which belong to the wrong phase. We need an entire page + // of valid results in the correct phase AFTER any results for the + // wrong phase are filtered for pagination to work correctly. + + // To make sure we can get there, we fetch more and more results until + // enough of them survive filtering to generate a full page. + + // We start by fetching 150% of the results than we think we need, and + // double the amount we overfetch by each time. + $factor = 1.5; + while (true) { + $query_source = clone $source; + $total = (int)ceil($limit * $factor) + 1; + $query_source->setLimit($total); + + $source_results = $query_source->loadResultsForPhase( + $phase, + $limit); + + // If there are fewer unfiltered results than we asked for, we know + // this is the entire result set and we don't need to keep going. + if (count($source_results) < $total) { + $source_results = $query_source->didLoadResults($source_results); + $source_results = $this->filterPhaseResults( + $phase, + $source_results); + break; + } + + // Otherwise, this result set have everything we need, or may not. + // Filter the results that are part of the wrong phase out first... + $source_results = $query_source->didLoadResults($source_results); + $source_results = $this->filterPhaseResults($phase, $source_results); + + // Now check if we have enough results left. If we do, we're all set. + if (count($source_results) >= $total) { + break; + } + + // We filtered out too many results to have a full page left, so we + // need to run the query again, asking for even more results. We'll + // keep doing this until we get a full page or get all of the + // results. + $factor = $factor * 2; + } + } else { + $source_results = $source->loadResults(); + $source_results = $source->didLoadResults($source_results); + $source_results = $this->filterPhaseResults($phase, $source_results); + } + $results[] = $source_results; } $results = array_mergev($results); $results = msort($results, 'getSortKey'); - $count = count($results); + $results = $this->sliceResults($results); + + return $results; + } + + private function filterPhaseResults($phase, $source_results) { + foreach ($source_results as $key => $source_result) { + $result_phase = $this->getResultPhase($source_result); + + if ($result_phase != $phase) { + unset($source_results[$key]); + continue; + } + + $source_result->setPhase($result_phase); + } + + return $source_results; + } + + private function getResultPhase(PhabricatorTypeaheadResult $result) { + if ($this->prefixLength) { + $result_name = phutil_utf8_strtolower($result->getName()); + if (!strncmp($result_name, $this->prefixString, $this->prefixLength)) { + return self::PHASE_PREFIX; + } + } + + return self::PHASE_CONTENT; + } + + protected function sliceResults(array $results) { + $offset = $this->getOffset(); + $limit = $this->getLimit(); + if ($offset || $limit) { if (!$limit) { $limit = count($results); diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php index 9fee4b2434..163b6c40b3 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php @@ -13,6 +13,10 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { private $parameters = array(); private $functionStack = array(); private $isBrowse; + private $phase = self::PHASE_CONTENT; + + const PHASE_PREFIX = 'prefix'; + const PHASE_CONTENT = 'content'; public function setLimit($limit) { $this->limit = $limit; @@ -46,6 +50,10 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { return $this; } + public function getPrefixQuery() { + return phutil_utf8_strtolower($this->getRawQuery()); + } + public function getRawQuery() { return $this->rawQuery; } @@ -81,6 +89,15 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { return $this->isBrowse; } + public function setPhase($phase) { + $this->phase = $phase; + return $this; + } + + public function getPhase() { + return $this->phase; + } + public function getDatasourceURI() { $uri = new PhutilURI('/typeahead/class/'.get_class($this).'/'); $uri->setQueryParams($this->parameters); @@ -106,6 +123,13 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { abstract public function getDatasourceApplicationClass(); abstract public function loadResults(); + protected function loadResultsForPhase($phase, $limit) { + // By default, sources just load all of their results in every phase and + // rely on filtering at a higher level to sequence phases correctly. + $this->setLimit($limit); + return $this->loadResults(); + } + protected function didLoadResults(array $results) { return $results; } diff --git a/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php b/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php index 4c5c079734..14cbe726dc 100644 --- a/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php +++ b/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php @@ -18,6 +18,7 @@ final class PhabricatorTypeaheadResult extends Phobject { private $unique; private $autocomplete; private $attributes = array(); + private $phase; public function setIcon($icon) { $this->icon = $icon; @@ -154,6 +155,7 @@ final class PhabricatorTypeaheadResult extends Phobject { $this->tokenType, $this->unique ? 1 : null, $this->autocomplete, + $this->phase, ); while (end($data) === null) { array_pop($data); @@ -211,4 +213,13 @@ final class PhabricatorTypeaheadResult extends Phobject { return $this; } + public function setPhase($phase) { + $this->phase = $phase; + return $this; + } + + public function getPhase() { + return $this->phase; + } + } From ff677c1964b15d61cf9a4ddd727c1da97b63fe00 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Nov 2016 06:20:13 -0800 Subject: [PATCH 27/31] Fix two error strings in the diffusion.uri.edit Conduit method Summary: Fixes T11839. Both are missing a parameter and one is a copy/paste slop. Test Plan: {F1913812} {F1913813} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11839 Differential Revision: https://secure.phabricator.com/D16837 --- src/applications/diffusion/editor/DiffusionURIEditor.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/applications/diffusion/editor/DiffusionURIEditor.php b/src/applications/diffusion/editor/DiffusionURIEditor.php index 0b7096db3c..b8eabb9ef1 100644 --- a/src/applications/diffusion/editor/DiffusionURIEditor.php +++ b/src/applications/diffusion/editor/DiffusionURIEditor.php @@ -304,8 +304,9 @@ final class DiffusionURIEditor $type, pht('Invalid'), pht( - 'Value "%s" is not a valid display setting for this URI. '. + 'Value "%s" is not a valid IO setting for this URI. '. 'Available types for this URI are: %s.', + $new, implode(', ', array_keys($available))), $xaction); continue; @@ -418,6 +419,7 @@ final class DiffusionURIEditor pht( 'Value "%s" is not a valid display setting for this URI. '. 'Available types for this URI are: %s.', + $new, implode(', ', array_keys($available)))); } } From 40d3bcb8910b37d91ceae09dfd2693190612e045 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 11 Nov 2016 12:35:22 -0800 Subject: [PATCH 28/31] Fix a complicated object caching issue with the policy filter Summary: Fixes T11853. To set this up: - Create "Project A". - Join "Project A". - Create a subproject, "Project A Subproject 1". - This causes Project A to become a parent project. - This moves you to be a member of "Project A Subproject 1" instead of "Project A" directly. - Create another subproject, "Project A Subproject 2". - Do not join this subproject. - Set the second subproject's policy to "Visible To: Members of Project A". - Try to edit the second subproject. Before this change, this fails: - When querying projects, we sometime try to skip loading the viewer's membership in ancestor projects as a small optimization. - Via `PhabricatorExtendedPolicyInterface`, we may then return the parent project to the policy filter for extended checks. - The PolicyFilter has an optimization: if we're checking an object, and we already have that object, we can just use the object we already have. This is common and useful. - However, in this case it causes us to reuse an incomplete object (an object without proper membership information). We fail a policy check which we should pass. Instead, don't skip loading the viewer's membership in ancestor projects. Test Plan: - Did all that stuff above. - Could edit the subproject. - Ran `arc unit --everything`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11853 Differential Revision: https://secure.phabricator.com/D16840 --- .../project/query/PhabricatorProjectQuery.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index 87c6bb805e..44055b7260 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -260,14 +260,14 @@ final class PhabricatorProjectQuery $all_graph = $this->getAllReachableAncestors($projects); - if ($this->needAncestorMembers || $this->needWatchers) { - $src_projects = $all_graph; - } else { - $src_projects = $projects; - } + // NOTE: Although we may not need much information about ancestors, we + // always need to test if the viewer is a member, because we will return + // ancestor projects to the policy filter via ExtendedPolicy calls. If + // we skip populating membership data on a parent, the policy framework + // will think the user is not a member of the parent project. $all_sources = array(); - foreach ($src_projects as $project) { + foreach ($all_graph as $project) { // For milestones, we need parent members. if ($project->isMilestone()) { $parent_phid = $project->getParentProjectPHID(); @@ -306,7 +306,7 @@ final class PhabricatorProjectQuery } $membership_projects = array(); - foreach ($src_projects as $project) { + foreach ($all_graph as $project) { $project_phid = $project->getPHID(); if ($project->isMilestone()) { From 32d2955f22cf2ba2e9c67b4b1962869d1843273e Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 11 Nov 2016 13:36:03 -0800 Subject: [PATCH 29/31] Give more space for new select dropdown image Summary: On really wide selects, text will go over the background image. Give it more padding. Test Plan: Review selects on email preferences page. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16842 --- resources/celerity/map.php | 6 +++--- webroot/rsrc/css/phui/phui-form.css | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index a3f2c56e98..bf485af92a 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', - 'core.pkg.css' => '9d155da1', + 'core.pkg.css' => '231b1ee5', 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', @@ -141,7 +141,7 @@ return array( 'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9', 'rsrc/css/phui/phui-fontkit.css' => '9cda225e', 'rsrc/css/phui/phui-form-view.css' => '91adabe4', - 'rsrc/css/phui/phui-form.css' => 'ca2490c6', + 'rsrc/css/phui/phui-form.css' => 'b8fb087a', 'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f', 'rsrc/css/phui/phui-header-view.css' => '6ec8f155', 'rsrc/css/phui/phui-hovercard.css' => 'de1a2119', @@ -861,7 +861,7 @@ return array( 'phui-feed-story-css' => '44a9c8e9', 'phui-font-icon-base-css' => '870a7360', 'phui-fontkit-css' => '9cda225e', - 'phui-form-css' => 'ca2490c6', + 'phui-form-css' => 'b8fb087a', 'phui-form-view-css' => '91adabe4', 'phui-head-thing-view-css' => 'fd311e5f', 'phui-header-view-css' => '6ec8f155', diff --git a/webroot/rsrc/css/phui/phui-form.css b/webroot/rsrc/css/phui/phui-form.css index 4150ac6940..b4fc0aaeaa 100644 --- a/webroot/rsrc/css/phui/phui-form.css +++ b/webroot/rsrc/css/phui/phui-form.css @@ -115,7 +115,7 @@ select { color: {$darkbluetext}; border: 1px solid {$greyborder}; height: 28px; - padding: 0 8px; + padding: 0 24px 0 8px; min-width: 180px; } From 4c540fb01b9a9b4292bb8d9c4d07d33792b83c37 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 11 Nov 2016 13:26:38 -0800 Subject: [PATCH 30/31] Fix some policy CSS Summary: Ref T11853. My CSS change for the more enormous policy dialog was a little too broad, and affected the "You shall not pass!" dialog too. Narrow the scope of the CSS rules. Also add a missing "." that I caught. Test Plan: - Looked at policy exception dialogs. - Looked at policy explanation dialogs. - Looked at the end of that sentence. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11853 Differential Revision: https://secure.phabricator.com/D16841 --- resources/celerity/map.php | 6 +++--- .../controller/PhabricatorPolicyExplainController.php | 2 +- .../PhabricatorProjectSubprojectWarningController.php | 2 +- webroot/rsrc/css/aphront/dialog-view.css | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index bf485af92a..010f9c997a 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', - 'core.pkg.css' => '231b1ee5', + 'core.pkg.css' => 'a729d20e', 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', @@ -21,7 +21,7 @@ return array( 'maniphest.pkg.js' => '949a7498', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', 'rsrc/css/aphront/dark-console.css' => 'f54bf286', - 'rsrc/css/aphront/dialog-view.css' => '1e6b8603', + 'rsrc/css/aphront/dialog-view.css' => 'ea3745f5', 'rsrc/css/aphront/lightbox-attachment.css' => '7acac05d', 'rsrc/css/aphront/list-filter-view.css' => '5d6f0526', 'rsrc/css/aphront/multi-column.css' => '84cc6640', @@ -550,7 +550,7 @@ return array( 'almanac-css' => 'dbb9b3af', 'aphront-bars' => '231ac33c', 'aphront-dark-console-css' => 'f54bf286', - 'aphront-dialog-view-css' => '1e6b8603', + 'aphront-dialog-view-css' => 'ea3745f5', 'aphront-list-filter-view-css' => '5d6f0526', 'aphront-multi-column-view-css' => '84cc6640', 'aphront-panel-view-css' => '8427b78d', diff --git a/src/applications/policy/controller/PhabricatorPolicyExplainController.php b/src/applications/policy/controller/PhabricatorPolicyExplainController.php index ef61272d88..45ac0ee5e1 100644 --- a/src/applications/policy/controller/PhabricatorPolicyExplainController.php +++ b/src/applications/policy/controller/PhabricatorPolicyExplainController.php @@ -40,7 +40,7 @@ final class PhabricatorPolicyExplainController $dialog = id(new AphrontDialogView()) ->setUser($viewer) - ->setClass('aphront-access-dialog') + ->setClass('aphront-access-dialog aphront-policy-explain-dialog') ->setTitle(pht('Policy Details: %s', $object_name)) ->addCancelButton($object_uri, pht('Done')); diff --git a/src/applications/project/controller/PhabricatorProjectSubprojectWarningController.php b/src/applications/project/controller/PhabricatorProjectSubprojectWarningController.php index d9dd401101..aa030c980b 100644 --- a/src/applications/project/controller/PhabricatorProjectSubprojectWarningController.php +++ b/src/applications/project/controller/PhabricatorProjectSubprojectWarningController.php @@ -35,7 +35,7 @@ final class PhabricatorProjectSubprojectWarningController $conversion_help = pht( "Creating a project's first subproject **moves all ". - "members** to become members of the subproject instead". + "members** to become members of the subproject instead.". "\n\n". "See [[ %s | Projects User Guide ]] in the documentation for details. ". "This process can not be undone.", diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css index a010c82b5a..7e3f911af9 100644 --- a/webroot/rsrc/css/aphront/dialog-view.css +++ b/webroot/rsrc/css/aphront/dialog-view.css @@ -128,7 +128,7 @@ width: 50%; } -.aphront-access-dialog .aphront-dialog-body { +.aphront-policy-explain-dialog .aphront-dialog-body { padding: 0 12px; } From 96219ab568c57a573f52a6977f2f4835d126205d Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 11 Nov 2016 22:22:59 +0000 Subject: [PATCH 31/31] Fiddle with Conpherence textarea Summary: Slightly larger, consistent colors. Test Plan: Mobile, Tablet, Desktop Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16843 --- resources/celerity/map.php | 6 ++-- .../application/conpherence/message-pane.css | 31 +++++-------------- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 010f9c997a..646f4b6d93 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'conpherence.pkg.css' => 'cea72e09', + 'conpherence.pkg.css' => '0b64e988', 'conpherence.pkg.js' => '6249a1cf', 'core.pkg.css' => 'a729d20e', 'core.pkg.js' => '1a77dddf', @@ -50,7 +50,7 @@ return array( 'rsrc/css/application/conpherence/durable-column.css' => 'd82e130c', 'rsrc/css/application/conpherence/header-pane.css' => '1c81cda6', 'rsrc/css/application/conpherence/menu.css' => '4f51db5a', - 'rsrc/css/application/conpherence/message-pane.css' => '394ae8fa', + 'rsrc/css/application/conpherence/message-pane.css' => 'b085d40d', 'rsrc/css/application/conpherence/notification.css' => '965db05b', 'rsrc/css/application/conpherence/participant-pane.css' => 'ac1baaa8', 'rsrc/css/application/conpherence/transaction.css' => '85129c68', @@ -568,7 +568,7 @@ return array( 'conpherence-durable-column-view' => 'd82e130c', 'conpherence-header-pane-css' => '1c81cda6', 'conpherence-menu-css' => '4f51db5a', - 'conpherence-message-pane-css' => '394ae8fa', + 'conpherence-message-pane-css' => 'b085d40d', 'conpherence-notification-css' => '965db05b', 'conpherence-participant-pane-css' => 'ac1baaa8', 'conpherence-thread-manager' => '358c717b', diff --git a/webroot/rsrc/css/application/conpherence/message-pane.css b/webroot/rsrc/css/application/conpherence/message-pane.css index 6336ac0cc5..9c8856fe0d 100644 --- a/webroot/rsrc/css/application/conpherence/message-pane.css +++ b/webroot/rsrc/css/application/conpherence/message-pane.css @@ -58,7 +58,7 @@ left: 240px; right: 240px; top: 103px; - bottom: 122px; + bottom: 142px; overflow-x: hidden; overflow-y: auto; -webkit-overflow-scrolling: touch; @@ -97,7 +97,7 @@ .conpherence-message-pane .phui-form-view { border-width: 0; - height: 110px; + height: 130px; padding: 0 20px 12px; position: fixed; bottom: 0; @@ -110,7 +110,7 @@ color: {$lightgreytext}; font-style: italic; position: absolute; - bottom: 6px; + bottom: 2px; left: 24px; } @@ -166,17 +166,16 @@ } .conpherence-message-pane .remarkup-assist-bar { - border: 2px solid {$lightgreyborder}; border-bottom: none; border-top-left-radius: 3px; border-top-right-radius: 3px; - background-color: {$lightgreybackground}; } .device .conpherence-message-pane .remarkup-assist-bar { position: absolute; - top: 12px; - left: 12px; + top: 9px; + left: 9px; + width: 24px; background: {$bluebackground}; border-radius: 3px; border: none; @@ -349,40 +348,26 @@ } .conpherence-message-pane .remarkup-assist-textarea { - height: 68px; + height: 88px; padding: 8px; - border: 2px solid {$lightgreyborder}; - border-top: 1px solid {$thinblueborder}; box-sizing: border-box; -moz-box-sizing: border-box; -webkit-box-sizing: border-box; - resize: none; - outline: none; - box-shadow: none; border-bottom-left-radius: 3px; border-bottom-right-radius: 3px; } -.conpherence-message-pane .remarkup-assist-textarea:focus { - border: 2px solid {$lightgreyborder}; - border-top: 1px solid {$thinblueborder}; -} - .device .conpherence-message-pane .remarkup-assist-textarea { margin: 0; padding: 7px 8px 6px 30px; width: 100%; height: 34px; resize: none; - border-top: 2px solid {$lightgreyborder}; + border-color: {$greyborder}; border-top-left-radius: 3px; border-top-right-radius: 3px; } -.conpherence-message-pane .remarkup-assist-textarea:focus { - outline: none; -} - .device .conpherence-message-pane .aphront-form-control-submit { padding: 0; position: absolute;