From 2c55a4ad723797e349090a869c5699e00f22b4d4 Mon Sep 17 00:00:00 2001 From: Mike Riley Date: Sun, 31 Jul 2016 13:35:31 +0000 Subject: [PATCH 01/23] Provide a basic search engine for builds Summary: This supports a few basic use cases that aren't served by the buildable search engine: - I'm trying to discover when the last time that this particular build plan failed was. - I want to know if any builds have deadlocked. - At a glance, I'm more interested in what build plans are running, not which buildables are being built. This is more often than not the case. Test Plan: {F1744003} Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D16347 --- src/__phutil_library_map__.php | 6 + .../PhabricatorHarbormasterApplication.php | 1 + .../HarbormasterBuildListController.php | 15 +++ .../HarbormasterBuildableListController.php | 9 ++ .../query/HarbormasterBuildSearchEngine.php | 126 ++++++++++++++++++ .../storage/build/HarbormasterBuild.php | 38 +++--- .../HarbormasterBuildStatusDatasource.php | 45 +++++++ 7 files changed, 218 insertions(+), 22 deletions(-) create mode 100644 src/applications/harbormaster/controller/HarbormasterBuildListController.php create mode 100644 src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php create mode 100644 src/applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 1ac5528904..c59605602e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1107,6 +1107,7 @@ phutil_register_library_map(array( 'HarbormasterBuildFailureException' => 'applications/harbormaster/exception/HarbormasterBuildFailureException.php', 'HarbormasterBuildGraph' => 'applications/harbormaster/engine/HarbormasterBuildGraph.php', 'HarbormasterBuildLintMessage' => 'applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php', + 'HarbormasterBuildListController' => 'applications/harbormaster/controller/HarbormasterBuildListController.php', 'HarbormasterBuildLog' => 'applications/harbormaster/storage/build/HarbormasterBuildLog.php', 'HarbormasterBuildLogChunk' => 'applications/harbormaster/storage/build/HarbormasterBuildLogChunk.php', 'HarbormasterBuildLogChunkIterator' => 'applications/harbormaster/storage/build/HarbormasterBuildLogChunkIterator.php', @@ -1129,6 +1130,8 @@ phutil_register_library_map(array( 'HarbormasterBuildPlanTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanTransactionQuery.php', 'HarbormasterBuildQuery' => 'applications/harbormaster/query/HarbormasterBuildQuery.php', 'HarbormasterBuildRequest' => 'applications/harbormaster/engine/HarbormasterBuildRequest.php', + 'HarbormasterBuildSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildSearchEngine.php', + 'HarbormasterBuildStatusDatasource' => 'applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php', 'HarbormasterBuildStep' => 'applications/harbormaster/storage/configuration/HarbormasterBuildStep.php', 'HarbormasterBuildStepCoreCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php', 'HarbormasterBuildStepCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCustomField.php', @@ -5647,6 +5650,7 @@ phutil_register_library_map(array( 'HarbormasterBuildFailureException' => 'Exception', 'HarbormasterBuildGraph' => 'AbstractDirectedGraph', 'HarbormasterBuildLintMessage' => 'HarbormasterDAO', + 'HarbormasterBuildListController' => 'HarbormasterController', 'HarbormasterBuildLog' => array( 'HarbormasterDAO', 'PhabricatorPolicyInterface', @@ -5682,6 +5686,8 @@ phutil_register_library_map(array( 'HarbormasterBuildPlanTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'HarbormasterBuildQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildRequest' => 'Phobject', + 'HarbormasterBuildSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'HarbormasterBuildStatusDatasource' => 'PhabricatorTypeaheadDatasource', 'HarbormasterBuildStep' => array( 'HarbormasterDAO', 'PhabricatorApplicationTransactionInterface', diff --git a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php index 00fb570e03..ed7104965a 100644 --- a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php +++ b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php @@ -70,6 +70,7 @@ final class PhabricatorHarbormasterApplication extends PhabricatorApplication { => 'HarbormasterBuildableActionController', ), 'build/' => array( + $this->getQueryRoutePattern() => 'HarbormasterBuildListController', '(?P\d+)/' => 'HarbormasterBuildViewController', '(?Ppause|resume|restart|abort)/'. '(?P\d+)/(?:(?P[^/]+)/)?' diff --git a/src/applications/harbormaster/controller/HarbormasterBuildListController.php b/src/applications/harbormaster/controller/HarbormasterBuildListController.php new file mode 100644 index 0000000000..0da9f57d6f --- /dev/null +++ b/src/applications/harbormaster/controller/HarbormasterBuildListController.php @@ -0,0 +1,15 @@ +setController($this) + ->buildResponse(); + } + +} diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableListController.php b/src/applications/harbormaster/controller/HarbormasterBuildableListController.php index 99595335be..1c08b1950f 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildableListController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableListController.php @@ -9,6 +9,15 @@ final class HarbormasterBuildableListController extends HarbormasterController { public function handleRequest(AphrontRequest $request) { $items = array(); + $items[] = id(new PHUIListItemView()) + ->setType(PHUIListItemView::TYPE_LABEL) + ->setName(pht('Builds')); + + $items[] = id(new PHUIListItemView()) + ->setType(PHUIListItemView::TYPE_LINK) + ->setName(pht('Browse Builds')) + ->setHref($this->getApplicationURI('build/')); + $items[] = id(new PHUIListItemView()) ->setType(PHUIListItemView::TYPE_LABEL) ->setName(pht('Build Plans')); diff --git a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php new file mode 100644 index 0000000000..7a15305b20 --- /dev/null +++ b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php @@ -0,0 +1,126 @@ +setLabel(pht('Build Plans')) + ->setKey('plans') + ->setAliases(array('plan')) + ->setDescription( + pht('Search for builds running a given build plan.')) + ->setDatasource(new HarbormasterBuildPlanDatasource()), + id(new PhabricatorSearchDatasourceField()) + ->setLabel(pht('Statuses')) + ->setKey('statuses') + ->setAliases(array('status')) + ->setDescription( + pht('Search for builds with given statuses.')) + ->setDatasource(new HarbormasterBuildStatusDatasource()), + ); + } + + protected function buildQueryFromParameters(array $map) { + $query = $this->newQuery(); + + if ($map['plans']) { + $query->withBuildPlanPHIDs($map['plans']); + } + + if ($map['statuses']) { + $query->withBuildStatuses($map['statuses']); + } + + return $query; + } + + protected function getURI($path) { + return '/harbormaster/build/'.$path; + } + + protected function getBuiltinQueryNames() { + return array( + 'all' => pht('All Builds'), + ); + } + + public function buildSavedQueryFromBuiltin($query_key) { + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + switch ($query_key) { + case 'all': + return $query; + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + + protected function renderResultList( + array $builds, + PhabricatorSavedQuery $query, + array $handles) { + assert_instances_of($builds, 'HarbormasterBuild'); + + $viewer = $this->requireViewer(); + + $buildables = mpull($builds, 'getBuildable'); + $object_phids = mpull($buildables, 'getBuildablePHID'); + $initiator_phids = mpull($builds, 'getInitiatorPHID'); + $phids = array_mergev(array($initiator_phids, $object_phids)); + $phids = array_unique(array_filter($phids)); + + $handles = $viewer->loadHandles($phids); + + $list = new PHUIObjectItemListView(); + foreach ($builds as $build) { + $id = $build->getID(); + $initiator = $handles[$build->getInitiatorPHID()]; + $buildable_object = $handles[$build->getBuildable()->getBuildablePHID()]; + + $item = id(new PHUIObjectItemView()) + ->setViewer($viewer) + ->setObject($build) + ->setObjectName(pht('Build %d', $build->getID())) + ->setHeader($build->getName()) + ->setHref($build->getURI()) + ->setEpoch($build->getDateCreated()) + ->addAttribute($buildable_object->getName()); + + if ($initiator) { + $item->addHandleIcon($initiator, $initiator->getName()); + } + + $status = $build->getBuildStatus(); + + $status_icon = HarbormasterBuild::getBuildStatusIcon($status); + $status_color = HarbormasterBuild::getBuildStatusColor($status); + $status_label = HarbormasterBuild::getBuildStatusName($status); + + $item->setStatusIcon("{$status_icon} {$status_color}", $status_label); + + $list->addItem($item); + } + + $result = new PhabricatorApplicationSearchResultView(); + $result->setObjectList($list); + $result->setNoDataString(pht('No builds found.')); + + return $result; + } + +} diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 4d9278c7bf..385b9133a8 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -71,28 +71,22 @@ final class HarbormasterBuild extends HarbormasterDAO * @return string Human-readable name. */ public static function getBuildStatusName($status) { - switch ($status) { - case self::STATUS_INACTIVE: - return pht('Inactive'); - case self::STATUS_PENDING: - return pht('Pending'); - case self::STATUS_BUILDING: - return pht('Building'); - case self::STATUS_PASSED: - return pht('Passed'); - case self::STATUS_FAILED: - return pht('Failed'); - case self::STATUS_ABORTED: - return pht('Aborted'); - case self::STATUS_ERROR: - return pht('Unexpected Error'); - case self::STATUS_PAUSED: - return pht('Paused'); - case self::STATUS_DEADLOCKED: - return pht('Deadlocked'); - default: - return pht('Unknown'); - } + $map = self::getBuildStatusMap(); + return idx($map, $status, pht('Unknown ("%s")', $status)); + } + + public static function getBuildStatusMap() { + return array( + self::STATUS_INACTIVE => pht('Inactive'), + self::STATUS_PENDING => pht('Pending'), + self::STATUS_BUILDING => pht('Building'), + self::STATUS_PASSED => pht('Passed'), + self::STATUS_FAILED => pht('Failed'), + self::STATUS_ABORTED => pht('Aborted'), + self::STATUS_ERROR => pht('Unexpected Error'), + self::STATUS_PAUSED => pht('Paused'), + self::STATUS_DEADLOCKED => pht('Deadlocked'), + ); } public static function getBuildStatusIcon($status) { diff --git a/src/applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php b/src/applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php new file mode 100644 index 0000000000..3e757b4348 --- /dev/null +++ b/src/applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php @@ -0,0 +1,45 @@ +buildResults(); + return $this->filterResultsAgainstTokens($results); + } + + public function renderTokens(array $values) { + return $this->renderTokensFromResults($this->buildResults(), $values); + } + + private function buildResults() { + $results = array(); + + $status_map = HarbormasterBuild::getBuildStatusMap(); + foreach ($status_map as $value => $name) { + $result = id(new PhabricatorTypeaheadResult()) + ->setIcon(HarbormasterBuild::getBuildStatusIcon($value)) + ->setColor(HarbormasterBuild::getBuildStatusColor($value)) + ->setPHID($value) + ->setName($name) + ->addAttribute(pht('Status')); + + $results[$value] = $result; + } + + return $results; + } + +} From 42b81a80900d98d460f4b96d3fd7873e2682ff97 Mon Sep 17 00:00:00 2001 From: Mike Riley Date: Sun, 31 Jul 2016 14:56:31 +0000 Subject: [PATCH 02/23] Move build statuses to a constants class Summary: No functional changes here, just lifting this out to make room for activities, heeding lint warnings along the way. Test Plan: before: ```lang=bash $ grep -Rn "HarbormasterBuild::" * src/applications/harbormaster/storage/HarbormasterBuildable.php:169: $build = HarbormasterBuild::initializeNewBuild($viewer) src/applications/harbormaster/storage/HarbormasterBuildable.php:173: ->setBuildStatus(HarbormasterBuild::STATUS_PENDING); src/applications/harbormaster/controller/HarbormasterStepEditController.php:242: $variables = HarbormasterBuild::getAvailableBuildVariables(); src/applications/harbormaster/controller/HarbormasterBuildableViewController.php:200: 'fa-dot-circle-o '.HarbormasterBuild::getBuildStatusColor($status), src/applications/harbormaster/controller/HarbormasterBuildableViewController.php:201: HarbormasterBuild::getBuildStatusName($status)); src/applications/harbormaster/controller/HarbormasterBuildableViewController.php:203: $item->addAttribute(HarbormasterBuild::getBuildStatusName($status)); src/applications/harbormaster/controller/HarbormasterBuildViewController.php:584: HarbormasterBuild::getBuildStatusName($status); src/applications/harbormaster/controller/HarbormasterBuildViewController.php:585: $icon = HarbormasterBuild::getBuildStatusIcon($status); src/applications/harbormaster/controller/HarbormasterBuildViewController.php:586: $color = HarbormasterBuild::getBuildStatusColor($status); src/applications/harbormaster/event/HarbormasterUIEventListener.php:135: $status_name = HarbormasterBuild::getBuildStatusName($status); src/applications/harbormaster/event/HarbormasterUIEventListener.php:136: $icon = HarbormasterBuild::getBuildStatusIcon($status); src/applications/harbormaster/event/HarbormasterUIEventListener.php:137: $color = HarbormasterBuild::getBuildStatusColor($status); src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php:78: 'buildStatusName' => HarbormasterBuild::getBuildStatusName($status), src/applications/harbormaster/engine/HarbormasterBuildEngine.php:66: $build->setBuildStatus(HarbormasterBuild::STATUS_ERROR); src/applications/harbormaster/engine/HarbormasterBuildEngine.php:109: $build->setBuildStatus(HarbormasterBuild::STATUS_ABORTED); src/applications/harbormaster/engine/HarbormasterBuildEngine.php:113: if (($build->getBuildStatus() == HarbormasterBuild::STATUS_PENDING) || src/applications/harbormaster/engine/HarbormasterBuildEngine.php:116: $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING); src/applications/harbormaster/engine/HarbormasterBuildEngine.php:121: $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING); src/applications/harbormaster/engine/HarbormasterBuildEngine.php:126: $build->setBuildStatus(HarbormasterBuild::STATUS_PAUSED); src/applications/harbormaster/engine/HarbormasterBuildEngine.php:132: if ($build->getBuildStatus() == HarbormasterBuild::STATUS_BUILDING) { src/applications/harbormaster/engine/HarbormasterBuildEngine.php:246: $build->setBuildStatus(HarbormasterBuild::STATUS_FAILED); src/applications/harbormaster/engine/HarbormasterBuildEngine.php:254: $build->setBuildStatus(HarbormasterBuild::STATUS_PASSED); src/applications/harbormaster/engine/HarbormasterBuildEngine.php:290: $build->setBuildStatus(HarbormasterBuild::STATUS_DEADLOCKED); src/applications/harbormaster/engine/HarbormasterBuildEngine.php:446: if ($build->getBuildStatus() != HarbormasterBuild::STATUS_PASSED) { src/applications/harbormaster/engine/HarbormasterBuildEngine.php:449: if ($build->getBuildStatus() == HarbormasterBuild::STATUS_FAILED || src/applications/harbormaster/engine/HarbormasterBuildEngine.php:450: $build->getBuildStatus() == HarbormasterBuild::STATUS_ERROR || src/applications/harbormaster/engine/HarbormasterBuildEngine.php:451: $build->getBuildStatus() == HarbormasterBuild::STATUS_DEADLOCKED) { ``` after: ```lang=bash $ grep -Rn "HarbormasterBuild::" * src/applications/harbormaster/storage/HarbormasterBuildable.php:169: $build = HarbormasterBuild::initializeNewBuild($viewer) src/applications/harbormaster/controller/HarbormasterStepEditController.php:242: $variables = HarbormasterBuild::getAvailableBuildVariables(); ``` ran a manual build as a sanity check Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D16348 --- src/__phutil_library_map__.php | 2 + ...arbormasterQueryBuildsConduitAPIMethod.php | 3 +- .../constants/HarbormasterBuildStatus.php | 121 ++++++++++++++++ .../HarbormasterBuildViewController.php | 6 +- .../HarbormasterBuildableViewController.php | 9 +- .../engine/HarbormasterBuildEngine.php | 32 ++--- .../event/HarbormasterUIEventListener.php | 6 +- .../query/HarbormasterBuildSearchEngine.php | 6 +- .../storage/HarbormasterBuildable.php | 2 +- .../storage/build/HarbormasterBuild.php | 135 ++---------------- .../HarbormasterBuildStatusDatasource.php | 6 +- 11 files changed, 168 insertions(+), 160 deletions(-) create mode 100644 src/applications/harbormaster/constants/HarbormasterBuildStatus.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index c59605602e..7fb33cc430 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1131,6 +1131,7 @@ phutil_register_library_map(array( 'HarbormasterBuildQuery' => 'applications/harbormaster/query/HarbormasterBuildQuery.php', 'HarbormasterBuildRequest' => 'applications/harbormaster/engine/HarbormasterBuildRequest.php', 'HarbormasterBuildSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildSearchEngine.php', + 'HarbormasterBuildStatus' => 'applications/harbormaster/constants/HarbormasterBuildStatus.php', 'HarbormasterBuildStatusDatasource' => 'applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php', 'HarbormasterBuildStep' => 'applications/harbormaster/storage/configuration/HarbormasterBuildStep.php', 'HarbormasterBuildStepCoreCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php', @@ -5687,6 +5688,7 @@ phutil_register_library_map(array( 'HarbormasterBuildQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildRequest' => 'Phobject', 'HarbormasterBuildSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'HarbormasterBuildStatus' => 'Phobject', 'HarbormasterBuildStatusDatasource' => 'PhabricatorTypeaheadDatasource', 'HarbormasterBuildStep' => array( 'HarbormasterDAO', diff --git a/src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php index 95628500c1..b559a650fc 100644 --- a/src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php +++ b/src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php @@ -66,6 +66,7 @@ final class HarbormasterQueryBuildsConduitAPIMethod $id = $build->getID(); $uri = '/harbormaster/build/'.$id.'/'; $status = $build->getBuildStatus(); + $status_name = HarbormasterBuildStatus::getBuildStatusName($status); $data[] = array( 'id' => $id, @@ -75,7 +76,7 @@ final class HarbormasterQueryBuildsConduitAPIMethod 'buildablePHID' => $build->getBuildablePHID(), 'buildPlanPHID' => $build->getBuildPlanPHID(), 'buildStatus' => $status, - 'buildStatusName' => HarbormasterBuild::getBuildStatusName($status), + 'buildStatusName' => $status_name, ); } diff --git a/src/applications/harbormaster/constants/HarbormasterBuildStatus.php b/src/applications/harbormaster/constants/HarbormasterBuildStatus.php new file mode 100644 index 0000000000..341bf4b7ab --- /dev/null +++ b/src/applications/harbormaster/constants/HarbormasterBuildStatus.php @@ -0,0 +1,121 @@ + pht('Inactive'), + self::STATUS_PENDING => pht('Pending'), + self::STATUS_BUILDING => pht('Building'), + self::STATUS_PASSED => pht('Passed'), + self::STATUS_FAILED => pht('Failed'), + self::STATUS_ABORTED => pht('Aborted'), + self::STATUS_ERROR => pht('Unexpected Error'), + self::STATUS_PAUSED => pht('Paused'), + self::STATUS_DEADLOCKED => pht('Deadlocked'), + ); + } + + public static function getBuildStatusIcon($status) { + switch ($status) { + case self::STATUS_INACTIVE: + case self::STATUS_PENDING: + return PHUIStatusItemView::ICON_OPEN; + case self::STATUS_BUILDING: + return PHUIStatusItemView::ICON_RIGHT; + case self::STATUS_PASSED: + return PHUIStatusItemView::ICON_ACCEPT; + case self::STATUS_FAILED: + return PHUIStatusItemView::ICON_REJECT; + case self::STATUS_ABORTED: + return PHUIStatusItemView::ICON_MINUS; + case self::STATUS_ERROR: + return PHUIStatusItemView::ICON_MINUS; + case self::STATUS_PAUSED: + return PHUIStatusItemView::ICON_MINUS; + case self::STATUS_DEADLOCKED: + return PHUIStatusItemView::ICON_WARNING; + default: + return PHUIStatusItemView::ICON_QUESTION; + } + } + + public static function getBuildStatusColor($status) { + switch ($status) { + case self::STATUS_INACTIVE: + return 'dark'; + case self::STATUS_PENDING: + case self::STATUS_BUILDING: + return 'blue'; + case self::STATUS_PASSED: + return 'green'; + case self::STATUS_FAILED: + case self::STATUS_ABORTED: + case self::STATUS_ERROR: + case self::STATUS_DEADLOCKED: + return 'red'; + case self::STATUS_PAUSED: + return 'dark'; + default: + return 'bluegrey'; + } + } + +} diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php index f78db68149..7932451d2e 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -581,9 +581,9 @@ final class HarbormasterBuildViewController } else { $status = $build->getBuildStatus(); $status_name = - HarbormasterBuild::getBuildStatusName($status); - $icon = HarbormasterBuild::getBuildStatusIcon($status); - $color = HarbormasterBuild::getBuildStatusColor($status); + HarbormasterBuildStatus::getBuildStatusName($status); + $icon = HarbormasterBuildStatus::getBuildStatusIcon($status); + $color = HarbormasterBuildStatus::getBuildStatusColor($status); } $item->setTarget($status_name); diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php index 390ff2ec3d..6a47dae429 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php @@ -196,11 +196,10 @@ final class HarbormasterBuildableViewController ->setHref($view_uri); $status = $build->getBuildStatus(); - $item->setStatusIcon( - 'fa-dot-circle-o '.HarbormasterBuild::getBuildStatusColor($status), - HarbormasterBuild::getBuildStatusName($status)); - - $item->addAttribute(HarbormasterBuild::getBuildStatusName($status)); + $status_color = HarbormasterBuildStatus::getBuildStatusColor($status); + $status_name = HarbormasterBuildStatus::getBuildStatusName($status); + $item->setStatusIcon('fa-dot-circle-o '.$status_color, $status_name); + $item->addAttribute($status_name); if ($build->isRestarting()) { $item->addIcon('fa-repeat', pht('Restarting')); diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php index 273f899f91..4cece6a69d 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -63,7 +63,7 @@ final class HarbormasterBuildEngine extends Phobject { // If any exception is raised, the build is marked as a failure and the // exception is re-thrown (this ensures we don't leave builds in an // inconsistent state). - $build->setBuildStatus(HarbormasterBuild::STATUS_ERROR); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_ERROR); $build->save(); $lock->unlock(); @@ -106,30 +106,30 @@ final class HarbormasterBuildEngine extends Phobject { private function updateBuild(HarbormasterBuild $build) { if ($build->isAborting()) { $this->releaseAllArtifacts($build); - $build->setBuildStatus(HarbormasterBuild::STATUS_ABORTED); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_ABORTED); $build->save(); } - if (($build->getBuildStatus() == HarbormasterBuild::STATUS_PENDING) || + if (($build->getBuildStatus() == HarbormasterBuildStatus::STATUS_PENDING) || ($build->isRestarting())) { $this->restartBuild($build); - $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING); $build->save(); } if ($build->isResuming()) { - $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING); $build->save(); } if ($build->isPausing() && !$build->isComplete()) { - $build->setBuildStatus(HarbormasterBuild::STATUS_PAUSED); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_PAUSED); $build->save(); } $build->deleteUnprocessedCommands(); - if ($build->getBuildStatus() == HarbormasterBuild::STATUS_BUILDING) { + if ($build->getBuildStatus() == HarbormasterBuildStatus::STATUS_BUILDING) { $this->updateBuildSteps($build); } } @@ -243,7 +243,7 @@ final class HarbormasterBuildEngine extends Phobject { // If any step failed, fail the whole build, then bail. if (count($failed)) { - $build->setBuildStatus(HarbormasterBuild::STATUS_FAILED); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_FAILED); $build->save(); return; } @@ -251,7 +251,7 @@ final class HarbormasterBuildEngine extends Phobject { // If every step is complete, we're done with this build. Mark it passed // and bail. if (count($complete) == count($steps)) { - $build->setBuildStatus(HarbormasterBuild::STATUS_PASSED); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_PASSED); $build->save(); return; } @@ -287,7 +287,7 @@ final class HarbormasterBuildEngine extends Phobject { if (!$runnable && !$waiting && !$underway) { // This means the build is deadlocked, and the user has configured // circular dependencies. - $build->setBuildStatus(HarbormasterBuild::STATUS_DEADLOCKED); + $build->setBuildStatus(HarbormasterBuildStatus::STATUS_DEADLOCKED); $build->save(); return; } @@ -443,14 +443,14 @@ final class HarbormasterBuildEngine extends Phobject { $all_pass = true; $any_fail = false; foreach ($buildable->getBuilds() as $build) { - if ($build->getBuildStatus() != HarbormasterBuild::STATUS_PASSED) { + if ($build->getBuildStatus() != HarbormasterBuildStatus::STATUS_PASSED) { $all_pass = false; } - if ($build->getBuildStatus() == HarbormasterBuild::STATUS_FAILED || - $build->getBuildStatus() == HarbormasterBuild::STATUS_ERROR || - $build->getBuildStatus() == HarbormasterBuild::STATUS_DEADLOCKED) { - $any_fail = true; - } + $any_fail = in_array($build->getBuildStatus(), array( + HarbormasterBuildStatus::STATUS_FAILED, + HarbormasterBuildStatus::STATUS_ERROR, + HarbormasterBuildStatus::STATUS_DEADLOCKED, + )); } if ($any_fail) { diff --git a/src/applications/harbormaster/event/HarbormasterUIEventListener.php b/src/applications/harbormaster/event/HarbormasterUIEventListener.php index 82ffcad560..219f556b3a 100644 --- a/src/applications/harbormaster/event/HarbormasterUIEventListener.php +++ b/src/applications/harbormaster/event/HarbormasterUIEventListener.php @@ -132,9 +132,9 @@ final class HarbormasterUIEventListener } $status = $build->getBuildStatus(); - $status_name = HarbormasterBuild::getBuildStatusName($status); - $icon = HarbormasterBuild::getBuildStatusIcon($status); - $color = HarbormasterBuild::getBuildStatusColor($status); + $status_name = HarbormasterBuildStatus::getBuildStatusName($status); + $icon = HarbormasterBuildStatus::getBuildStatusIcon($status); + $color = HarbormasterBuildStatus::getBuildStatusColor($status); $item->setIcon($icon, $color, $status_name); diff --git a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php index 7a15305b20..2999bcc6f4 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php +++ b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php @@ -107,9 +107,9 @@ final class HarbormasterBuildSearchEngine $status = $build->getBuildStatus(); - $status_icon = HarbormasterBuild::getBuildStatusIcon($status); - $status_color = HarbormasterBuild::getBuildStatusColor($status); - $status_label = HarbormasterBuild::getBuildStatusName($status); + $status_icon = HarbormasterBuildStatus::getBuildStatusIcon($status); + $status_color = HarbormasterBuildStatus::getBuildStatusColor($status); + $status_label = HarbormasterBuildStatus::getBuildStatusName($status); $item->setStatusIcon("{$status_icon} {$status_color}", $status_label); diff --git a/src/applications/harbormaster/storage/HarbormasterBuildable.php b/src/applications/harbormaster/storage/HarbormasterBuildable.php index 7a7b32618c..5e55d38824 100644 --- a/src/applications/harbormaster/storage/HarbormasterBuildable.php +++ b/src/applications/harbormaster/storage/HarbormasterBuildable.php @@ -170,7 +170,7 @@ final class HarbormasterBuildable extends HarbormasterDAO ->setBuildablePHID($this->getPHID()) ->setBuildPlanPHID($plan->getPHID()) ->setBuildParameters($parameters) - ->setBuildStatus(HarbormasterBuild::STATUS_PENDING); + ->setBuildStatus(HarbormasterBuildStatus::STATUS_PENDING); if ($initiator_phid) { $build->setInitiatorPHID($initiator_phid); } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 385b9133a8..44846b9bc8 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -18,125 +18,9 @@ final class HarbormasterBuild extends HarbormasterDAO private $buildTargets = self::ATTACHABLE; private $unprocessedCommands = self::ATTACHABLE; - /** - * Not currently being built. - */ - const STATUS_INACTIVE = 'inactive'; - - /** - * Pending pick up by the Harbormaster daemon. - */ - const STATUS_PENDING = 'pending'; - - /** - * Current building the buildable. - */ - const STATUS_BUILDING = 'building'; - - /** - * The build has passed. - */ - const STATUS_PASSED = 'passed'; - - /** - * The build has failed. - */ - const STATUS_FAILED = 'failed'; - - /** - * The build has aborted. - */ - const STATUS_ABORTED = 'aborted'; - - /** - * The build encountered an unexpected error. - */ - const STATUS_ERROR = 'error'; - - /** - * The build has been paused. - */ - const STATUS_PAUSED = 'paused'; - - /** - * The build has been deadlocked. - */ - const STATUS_DEADLOCKED = 'deadlocked'; - - - /** - * Get a human readable name for a build status constant. - * - * @param const Build status constant. - * @return string Human-readable name. - */ - public static function getBuildStatusName($status) { - $map = self::getBuildStatusMap(); - return idx($map, $status, pht('Unknown ("%s")', $status)); - } - - public static function getBuildStatusMap() { - return array( - self::STATUS_INACTIVE => pht('Inactive'), - self::STATUS_PENDING => pht('Pending'), - self::STATUS_BUILDING => pht('Building'), - self::STATUS_PASSED => pht('Passed'), - self::STATUS_FAILED => pht('Failed'), - self::STATUS_ABORTED => pht('Aborted'), - self::STATUS_ERROR => pht('Unexpected Error'), - self::STATUS_PAUSED => pht('Paused'), - self::STATUS_DEADLOCKED => pht('Deadlocked'), - ); - } - - public static function getBuildStatusIcon($status) { - switch ($status) { - case self::STATUS_INACTIVE: - case self::STATUS_PENDING: - return PHUIStatusItemView::ICON_OPEN; - case self::STATUS_BUILDING: - return PHUIStatusItemView::ICON_RIGHT; - case self::STATUS_PASSED: - return PHUIStatusItemView::ICON_ACCEPT; - case self::STATUS_FAILED: - return PHUIStatusItemView::ICON_REJECT; - case self::STATUS_ABORTED: - return PHUIStatusItemView::ICON_MINUS; - case self::STATUS_ERROR: - return PHUIStatusItemView::ICON_MINUS; - case self::STATUS_PAUSED: - return PHUIStatusItemView::ICON_MINUS; - case self::STATUS_DEADLOCKED: - return PHUIStatusItemView::ICON_WARNING; - default: - return PHUIStatusItemView::ICON_QUESTION; - } - } - - public static function getBuildStatusColor($status) { - switch ($status) { - case self::STATUS_INACTIVE: - return 'dark'; - case self::STATUS_PENDING: - case self::STATUS_BUILDING: - return 'blue'; - case self::STATUS_PASSED: - return 'green'; - case self::STATUS_FAILED: - case self::STATUS_ABORTED: - case self::STATUS_ERROR: - case self::STATUS_DEADLOCKED: - return 'red'; - case self::STATUS_PAUSED: - return 'dark'; - default: - return 'bluegrey'; - } - } - public static function initializeNewBuild(PhabricatorUser $actor) { return id(new HarbormasterBuild()) - ->setBuildStatus(self::STATUS_INACTIVE) + ->setBuildStatus(HarbormasterBuildStatus::STATUS_INACTIVE) ->setBuildGeneration(0); } @@ -220,8 +104,9 @@ final class HarbormasterBuild extends HarbormasterDAO } public function isBuilding() { - return $this->getBuildStatus() === self::STATUS_PENDING || - $this->getBuildStatus() === self::STATUS_BUILDING; + return + $this->getBuildStatus() === HarbormasterBuildStatus::STATUS_PENDING || + $this->getBuildStatus() === HarbormasterBuildStatus::STATUS_BUILDING; } public function isAutobuild() { @@ -285,11 +170,11 @@ final class HarbormasterBuild extends HarbormasterDAO public function isComplete() { switch ($this->getBuildStatus()) { - case self::STATUS_PASSED: - case self::STATUS_FAILED: - case self::STATUS_ABORTED: - case self::STATUS_ERROR: - case self::STATUS_PAUSED: + case HarbormasterBuildStatus::STATUS_PASSED: + case HarbormasterBuildStatus::STATUS_FAILED: + case HarbormasterBuildStatus::STATUS_ABORTED: + case HarbormasterBuildStatus::STATUS_ERROR: + case HarbormasterBuildStatus::STATUS_PAUSED: return true; } @@ -297,7 +182,7 @@ final class HarbormasterBuild extends HarbormasterDAO } public function isPaused() { - return ($this->getBuildStatus() == self::STATUS_PAUSED); + return ($this->getBuildStatus() == HarbormasterBuildStatus::STATUS_PAUSED); } public function getURI() { diff --git a/src/applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php b/src/applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php index 3e757b4348..6cc11bd0c2 100644 --- a/src/applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php +++ b/src/applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php @@ -27,11 +27,11 @@ final class HarbormasterBuildStatusDatasource private function buildResults() { $results = array(); - $status_map = HarbormasterBuild::getBuildStatusMap(); + $status_map = HarbormasterBuildStatus::getBuildStatusMap(); foreach ($status_map as $value => $name) { $result = id(new PhabricatorTypeaheadResult()) - ->setIcon(HarbormasterBuild::getBuildStatusIcon($value)) - ->setColor(HarbormasterBuild::getBuildStatusColor($value)) + ->setIcon(HarbormasterBuildStatus::getBuildStatusIcon($value)) + ->setColor(HarbormasterBuildStatus::getBuildStatusColor($value)) ->setPHID($value) ->setName($name) ->addAttribute(pht('Status')); From 33fca12816b6a011d5ff44eee96c2fdbeb46c6e0 Mon Sep 17 00:00:00 2001 From: Mike Riley Date: Sun, 31 Jul 2016 15:35:18 +0000 Subject: [PATCH 03/23] Pick some preset build statuses Summary: We're picking three useful groups of build statuses to provide as default queries: - Stuff not yet building - Stuff building - Stuff which has finished building These are reasonable buckets for builds since (unlike most objects in phabricatorland) users are generally waiting impatiently for the machine to do something for them, rather than being responsible for doing something with the machine. Test Plan: clicked around the search engine and enjoyed my defaults Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D16349 --- .../constants/HarbormasterBuildStatus.php | 24 +++++++++++++++++++ .../query/HarbormasterBuildSearchEngine.php | 18 ++++++++++++++ .../storage/build/HarbormasterBuild.php | 13 +++------- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/applications/harbormaster/constants/HarbormasterBuildStatus.php b/src/applications/harbormaster/constants/HarbormasterBuildStatus.php index 341bf4b7ab..a2cdba5b63 100644 --- a/src/applications/harbormaster/constants/HarbormasterBuildStatus.php +++ b/src/applications/harbormaster/constants/HarbormasterBuildStatus.php @@ -118,4 +118,28 @@ final class HarbormasterBuildStatus extends Phobject { } } + public static function getWaitingStatusConstants() { + return array( + self::STATUS_INACTIVE, + self::STATUS_PENDING, + ); + } + + public static function getActiveStatusConstants() { + return array( + self::STATUS_BUILDING, + self::STATUS_PAUSED, + ); + } + + public static function getCompletedStatusConstants() { + return array( + self::STATUS_PASSED, + self::STATUS_FAILED, + self::STATUS_ABORTED, + self::STATUS_ERROR, + self::STATUS_DEADLOCKED, + ); + } + } diff --git a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php index 2999bcc6f4..0b07ca7150 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php +++ b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php @@ -55,6 +55,9 @@ final class HarbormasterBuildSearchEngine protected function getBuiltinQueryNames() { return array( 'all' => pht('All Builds'), + 'waiting' => pht('Waiting'), + 'active' => pht('Active'), + 'completed' => pht('Completed'), ); } @@ -65,6 +68,21 @@ final class HarbormasterBuildSearchEngine switch ($query_key) { case 'all': return $query; + case 'waiting': + return $query + ->setParameter( + 'statuses', + HarbormasterBuildStatus::getWaitingStatusConstants()); + case 'active': + return $query + ->setParameter( + 'statuses', + HarbormasterBuildStatus::getActiveStatusConstants()); + case 'completed': + return $query + ->setParameter( + 'statuses', + HarbormasterBuildStatus::getCompletedStatusConstants()); } return parent::buildSavedQueryFromBuiltin($query_key); diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 44846b9bc8..759f9b9fe0 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -169,16 +169,9 @@ final class HarbormasterBuild extends HarbormasterDAO } public function isComplete() { - switch ($this->getBuildStatus()) { - case HarbormasterBuildStatus::STATUS_PASSED: - case HarbormasterBuildStatus::STATUS_FAILED: - case HarbormasterBuildStatus::STATUS_ABORTED: - case HarbormasterBuildStatus::STATUS_ERROR: - case HarbormasterBuildStatus::STATUS_PAUSED: - return true; - } - - return false; + return in_array( + $this->getBuildStatus(), + HarbormasterBuildStatus::getCompletedStatusConstants()); } public function isPaused() { From 6e57582aff82d295ca558eec23c401b33d9a6dcd Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 31 Jul 2016 08:19:17 -0700 Subject: [PATCH 04/23] Allow `*.search` Conduit API methods to have data bulk-loaded by extensions Summary: Ref T11404. Currently, SearchEngineAttachments can bulk-load data but SearchEngineExtensions can not. This leads to poor performance of custom fields. See T11404 for discussion. This changes the API to support a bulk load + format pattern like the one Attachments use. The next change will use it to bulk-load custom field data. Test Plan: - Ran `differential.query`, `differential.revision.search` as a sanity check. - No behavioral changes are expected - See next revision. Reviewers: yelirekim, chad Reviewed By: chad Maniphest Tasks: T11404 Differential Revision: https://secure.phabricator.com/D16350 --- .../query/ConduitResultSearchEngineExtension.php | 2 +- .../PhabricatorPolicySearchEngineExtension.php | 2 +- .../PhabricatorApplicationSearchEngine.php | 16 ++++++++++++---- .../PhabricatorLiskSearchEngineExtension.php | 2 +- .../PhabricatorSearchEngineExtension.php | 6 +++++- .../PhabricatorSpacesSearchEngineExtension.php | 2 +- ...abricatorCustomFieldSearchEngineExtension.php | 2 +- 7 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/applications/conduit/query/ConduitResultSearchEngineExtension.php b/src/applications/conduit/query/ConduitResultSearchEngineExtension.php index 9bf4b1a346..f3ca974fde 100644 --- a/src/applications/conduit/query/ConduitResultSearchEngineExtension.php +++ b/src/applications/conduit/query/ConduitResultSearchEngineExtension.php @@ -25,7 +25,7 @@ final class ConduitResultSearchEngineExtension return $object->getFieldSpecificationsForConduit(); } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { return $object->getFieldValuesForConduit(); } diff --git a/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php b/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php index a2bb2271f4..2b603bc95f 100644 --- a/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php +++ b/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php @@ -30,7 +30,7 @@ final class PhabricatorPolicySearchEngineExtension ); } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { $capabilities = $object->getCapabilities(); $map = array(); diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index 3d6b4d9cff..d2ce3ad946 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -1138,6 +1138,11 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { if ($objects) { $field_extensions = $this->getConduitFieldExtensions(); + $extension_data = array(); + foreach ($field_extensions as $key => $extension) { + $extension_data[$key] = $extension->loadExtensionConduitData($objects); + } + $attachment_data = array(); foreach ($attachments as $key => $attachment) { $attachment_data[$key] = $attachment->loadAttachmentData( @@ -1148,7 +1153,8 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { foreach ($objects as $object) { $field_map = $this->getObjectWireFieldsForConduit( $object, - $field_extensions); + $field_extensions, + $extension_data); $attachment_map = array(); foreach ($attachments as $key => $attachment) { @@ -1312,11 +1318,13 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { protected function getObjectWireFieldsForConduit( $object, - array $field_extensions) { + array $field_extensions, + array $extension_data) { $fields = array(); - foreach ($field_extensions as $extension) { - $fields += $extension->getFieldValuesForConduit($object); + foreach ($field_extensions as $key => $extension) { + $data = idx($extension_data, $key, array()); + $fields += $extension->getFieldValuesForConduit($object, $data); } return $fields; diff --git a/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php b/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php index c502ad4cc6..c4d338bc15 100644 --- a/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php @@ -44,7 +44,7 @@ final class PhabricatorLiskSearchEngineExtension ); } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { return array( 'dateCreated' => (int)$object->getDateCreated(), 'dateModified' => (int)$object->getDateModified(), diff --git a/src/applications/search/engineextension/PhabricatorSearchEngineExtension.php b/src/applications/search/engineextension/PhabricatorSearchEngineExtension.php index 39af0a3fdd..f93c09cf8f 100644 --- a/src/applications/search/engineextension/PhabricatorSearchEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorSearchEngineExtension.php @@ -56,7 +56,11 @@ abstract class PhabricatorSearchEngineExtension extends Phobject { return array(); } - public function getFieldValuesForConduit($object) { + public function loadExtensionConduitData(array $objects) { + return null; + } + + public function getFieldValuesForConduit($object, $data) { return array(); } diff --git a/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php b/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php index 7bf92370e4..829b7b2b93 100644 --- a/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php +++ b/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php @@ -63,7 +63,7 @@ final class PhabricatorSpacesSearchEngineExtension ); } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { return array( 'spacePHID' => $object->getSpacePHID(), ); diff --git a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php index 246ac713fc..b159b8f89d 100644 --- a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php +++ b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php @@ -80,7 +80,7 @@ final class PhabricatorCustomFieldSearchEngineExtension return $map; } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { // TODO: This is currently very inefficient. We should bulk-load these // field values instead. From b8f75f95116e1ec00821cd5467fea188277b2a25 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 31 Jul 2016 08:19:27 -0700 Subject: [PATCH 05/23] Improve Conduit performance for custom fields Summary: Ref T11404. Depends on D16350. Currently, custom fields can issue "N+1" queries in some cases, so querying 100 revisions issues 100 extra queries. This affects all `*.search` endpoints for objects with custom fields, and some older endpoints (notably `differential.query`). This change bulk loads "normal" custom fields, which gets rid of some of these queries. Instead of loading fields for each object, we build a big list of all fields and load them all at once. The next change will tackle the remaining inefficient edge queries. Test Plan: - Configured a custom field with normal database storage in Differential. - Ran `differential.query`, looking at custom fields in results for correctness. - Ran `differential.revision.search`, looking at custom fields in results for correctness. - In both cases, observed queries drop from `3N` to `2N` (all the "normal" custom field stuff got bulk loaded). Reviewers: yelirekim, chad Reviewed By: chad Maniphest Tasks: T11404 Differential Revision: https://secure.phabricator.com/D16351 --- src/__phutil_library_map__.php | 2 + .../conduit/DifferentialConduitAPIMethod.php | 25 +++++- ...icatorCustomFieldSearchEngineExtension.php | 45 +++++++--- .../field/PhabricatorCustomFieldList.php | 59 ++++--------- .../PhabricatorCustomFieldStorageQuery.php | 84 +++++++++++++++++++ .../storage/PhabricatorCustomFieldStorage.php | 64 ++++++++++++++ 6 files changed, 224 insertions(+), 55 deletions(-) create mode 100644 src/infrastructure/customfield/query/PhabricatorCustomFieldStorageQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7fb33cc430..cb78038bbd 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2258,6 +2258,7 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldNumericIndexStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldNumericIndexStorage.php', 'PhabricatorCustomFieldSearchEngineExtension' => 'infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php', 'PhabricatorCustomFieldStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldStorage.php', + 'PhabricatorCustomFieldStorageQuery' => 'infrastructure/customfield/query/PhabricatorCustomFieldStorageQuery.php', 'PhabricatorCustomFieldStringIndexStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldStringIndexStorage.php', 'PhabricatorCustomHeaderConfigType' => 'applications/config/custom/PhabricatorCustomHeaderConfigType.php', 'PhabricatorDaemon' => 'infrastructure/daemon/PhabricatorDaemon.php', @@ -6994,6 +6995,7 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldNumericIndexStorage' => 'PhabricatorCustomFieldIndexStorage', 'PhabricatorCustomFieldSearchEngineExtension' => 'PhabricatorSearchEngineExtension', 'PhabricatorCustomFieldStorage' => 'PhabricatorLiskDAO', + 'PhabricatorCustomFieldStorageQuery' => 'Phobject', 'PhabricatorCustomFieldStringIndexStorage' => 'PhabricatorCustomFieldIndexStorage', 'PhabricatorCustomHeaderConfigType' => 'PhabricatorConfigOptionType', 'PhabricatorDaemon' => 'PhutilDaemon', diff --git a/src/applications/differential/conduit/DifferentialConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialConduitAPIMethod.php index e5b71c87bb..a0c3ae860c 100644 --- a/src/applications/differential/conduit/DifferentialConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialConduitAPIMethod.php @@ -150,21 +150,38 @@ abstract class DifferentialConduitAPIMethod extends ConduitAPIMethod { array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); - $results = array(); + $field_lists = array(); foreach ($revisions as $revision) { - // TODO: This is inefficient and issues a query for each object. + $revision_phid = $revision->getPHID(); + $field_list = PhabricatorCustomField::getObjectFields( $revision, PhabricatorCustomField::ROLE_CONDUIT); $field_list ->setViewer($viewer) - ->readFieldsFromStorage($revision); + ->readFieldsFromObject($revision); + $field_lists[$revision_phid] = $field_list; + } + + $all_fields = array(); + foreach ($field_lists as $field_list) { + foreach ($field_list->getFields() as $field) { + $all_fields[] = $field; + } + } + + id(new PhabricatorCustomFieldStorageQuery()) + ->addFields($all_fields) + ->execute(); + + $results = array(); + foreach ($field_lists as $revision_phid => $field_list) { foreach ($field_list->getFields() as $field) { $field_key = $field->getFieldKeyForConduit(); $value = $field->getConduitDictionaryValue(); - $results[$revision->getPHID()][$field_key] = $value; + $results[$revision_phid][$field_key] = $value; } } diff --git a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php index b159b8f89d..309e3d2949 100644 --- a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php +++ b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php @@ -80,17 +80,42 @@ final class PhabricatorCustomFieldSearchEngineExtension return $map; } + public function loadExtensionConduitData(array $objects) { + $viewer = $this->getViewer(); + + $field_map = array(); + foreach ($objects as $object) { + $object_phid = $object->getPHID(); + + $fields = PhabricatorCustomField::getObjectFields( + $object, + PhabricatorCustomField::ROLE_CONDUIT); + + $fields + ->setViewer($viewer) + ->readFieldsFromObject($object); + + $field_map[$object_phid] = $fields; + } + + $all_fields = array(); + foreach ($field_map as $field_list) { + foreach ($field_list->getFields() as $field) { + $all_fields[] = $field; + } + } + + id(new PhabricatorCustomFieldStorageQuery()) + ->addFields($all_fields) + ->execute(); + + return array( + 'fields' => $field_map, + ); + } + public function getFieldValuesForConduit($object, $data) { - // TODO: This is currently very inefficient. We should bulk-load these - // field values instead. - - $fields = PhabricatorCustomField::getObjectFields( - $object, - PhabricatorCustomField::ROLE_CONDUIT); - - $fields - ->setViewer($this->getViewer()) - ->readFieldsFromStorage($object); + $fields = $data['fields'][$object->getPHID()]; $map = array(); foreach ($fields->getFields() as $field) { diff --git a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php index 9fefa52ad4..cb1e56711d 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php @@ -29,6 +29,19 @@ final class PhabricatorCustomFieldList extends Phobject { return $this; } + public function readFieldsFromObject( + PhabricatorCustomFieldInterface $object) { + + $fields = $this->getFields(); + + foreach ($fields as $field) { + $field + ->setObject($object) + ->readValueFromObject($object); + } + + return $this; + } /** * Read stored values for all fields which support storage. @@ -39,48 +52,12 @@ final class PhabricatorCustomFieldList extends Phobject { public function readFieldsFromStorage( PhabricatorCustomFieldInterface $object) { - foreach ($this->fields as $field) { - $field->setObject($object); - $field->readValueFromObject($object); - } + $this->readFieldsFromObject($object); - $keys = array(); - foreach ($this->fields as $field) { - if ($field->shouldEnableForRole(PhabricatorCustomField::ROLE_STORAGE)) { - $keys[$field->getFieldIndex()] = $field; - } - } - - if (!$keys) { - return $this; - } - - // NOTE: We assume all fields share the same storage. This isn't guaranteed - // to be true, but always is for now. - - $table = head($keys)->newStorageObject(); - - $objects = array(); - if ($object->getPHID()) { - $objects = $table->loadAllWhere( - 'objectPHID = %s AND fieldIndex IN (%Ls)', - $object->getPHID(), - array_keys($keys)); - $objects = mpull($objects, null, 'getFieldIndex'); - } - - foreach ($keys as $key => $field) { - $storage = idx($objects, $key); - if ($storage) { - $field->setValueFromStorage($storage->getFieldValue()); - $field->didSetValueFromStorage(); - } else if ($object->getPHID()) { - // NOTE: We set this only if the object exists. Otherwise, we allow the - // field to retain any default value it may have. - $field->setValueFromStorage(null); - $field->didSetValueFromStorage(); - } - } + $fields = $this->getFields(); + id(new PhabricatorCustomFieldStorageQuery()) + ->addFields($fields) + ->execute(); return $this; } diff --git a/src/infrastructure/customfield/query/PhabricatorCustomFieldStorageQuery.php b/src/infrastructure/customfield/query/PhabricatorCustomFieldStorageQuery.php new file mode 100644 index 0000000000..9188117dcc --- /dev/null +++ b/src/infrastructure/customfield/query/PhabricatorCustomFieldStorageQuery.php @@ -0,0 +1,84 @@ +addField($field); + } + + return $this; + } + + public function addField(PhabricatorCustomField $field) { + $role_storage = PhabricatorCustomField::ROLE_STORAGE; + + if (!$field->shouldEnableForRole($role_storage)) { + return $this; + } + + $storage = $field->newStorageObject(); + $source_key = $storage->getStorageSourceKey(); + + $this->fieldMap[$source_key][] = $field; + + if (empty($this->storageSources[$source_key])) { + $this->storageSources[$source_key] = $storage; + } + + return $this; + } + + public function execute() { + foreach ($this->storageSources as $source_key => $storage) { + $fields = idx($this->fieldMap, $source_key, array()); + $this->loadFieldsFromStorage($storage, $fields); + } + } + + private function loadFieldsFromStorage($storage, array $fields) { + // Only try to load fields which have a persisted object. + $loadable = array(); + foreach ($fields as $key => $field) { + $object = $field->getObject(); + $phid = $object->getPHID(); + if (!$phid) { + continue; + } + + $loadable[$key] = $field; + } + + if ($loadable) { + $data = $storage->loadStorageSourceData($loadable); + } else { + $data = array(); + } + + foreach ($fields as $key => $field) { + if (array_key_exists($key, $data)) { + $value = $data[$key]; + $field->setValueFromStorage($value); + $field->didSetValueFromStorage(); + } else if (isset($loadable[$key])) { + // NOTE: We set this only if the object exists. Otherwise, we allow + // the field to retain any default value it may have. + $field->setValueFromStorage(null); + $field->didSetValueFromStorage(); + } + } + } + +} diff --git a/src/infrastructure/customfield/storage/PhabricatorCustomFieldStorage.php b/src/infrastructure/customfield/storage/PhabricatorCustomFieldStorage.php index 9817b2e591..0f60d77602 100644 --- a/src/infrastructure/customfield/storage/PhabricatorCustomFieldStorage.php +++ b/src/infrastructure/customfield/storage/PhabricatorCustomFieldStorage.php @@ -23,4 +23,68 @@ abstract class PhabricatorCustomFieldStorage ) + parent::getConfiguration(); } + + /** + * Get a key which uniquely identifies this storage source. + * + * When loading custom fields, fields using sources with the same source key + * are loaded in bulk. + * + * @return string Source identifier. + */ + final public function getStorageSourceKey() { + return $this->getApplicationName().'/'.$this->getTableName(); + } + + + /** + * Load stored data for custom fields. + * + * Given a map of fields, return a map with any stored data for those fields. + * The keys in the result should correspond to the keys in the input. The + * fields in the list may belong to different objects. + * + * @param map Map of fields. + * @return map Map of available field data. + */ + final public function loadStorageSourceData(array $fields) { + $map = array(); + $indexes = array(); + $object_phids = array(); + + foreach ($fields as $key => $field) { + $index = $field->getFieldIndex(); + $object_phid = $field->getObject()->getPHID(); + + $map[$index][$object_phid] = $key; + $indexes[$index] = $index; + $object_phids[$object_phid] = $object_phid; + } + + if (!$indexes) { + return array(); + } + + $conn = $this->establishConnection('r'); + $rows = queryfx_all( + $conn, + 'SELECT objectPHID, fieldIndex, fieldValue FROM %T + WHERE objectPHID IN (%Ls) AND fieldIndex IN (%Ls)', + $this->getTableName(), + $object_phids, + $indexes); + + $result = array(); + foreach ($rows as $row) { + $index = $row['fieldIndex']; + $object_phid = $row['objectPHID']; + $value = $row['fieldValue']; + + $key = $map[$index][$object_phid]; + $result[$key] = $value; + } + + return $result; + } + } From 8fd20e82fcc6400a61aa251643ea8033e26305b3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 31 Jul 2016 08:50:20 -0700 Subject: [PATCH 06/23] Improve Conduit performance of special edge-based custom Revision fields Summary: Ref T11404. Depends on D16351. Currently, both `differential.query` and `differential.revision.search` issue `2N` queries to fetch: - dependencies for each revision; and - projects for each revision. Fix this: - Take these custom fields out of Conduit so they don't load this data by default. - For `differential.query`, put this data back in by hard coding it. - For `differential.revision.search`, just leave it out. You can already optionally get projects efficiently, and this endpoint is a work in progress. I would tentatively be inclined to expose graph data as a "graph" extension once we need it. This makes both methods execute in `O(1)` time (which is still 20-30 queries, but at least it's not 320 queries anymore). Test Plan: - Ran `differential.query`, observed no change in results but 199 fewer internal queries. - Ran `differential.revision.search`, observed data gone from results and 200 fewer internal queries. Reviewers: yelirekim, chad Reviewed By: chad Maniphest Tasks: T11404 Differential Revision: https://secure.phabricator.com/D16352 --- .../conduit/DifferentialConduitAPIMethod.php | 27 +++++++++++++++++++ .../DifferentialParentRevisionsField.php | 9 +++---- .../customfield/DifferentialProjectsField.php | 5 +++- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/applications/differential/conduit/DifferentialConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialConduitAPIMethod.php index a0c3ae860c..288aa60a02 100644 --- a/src/applications/differential/conduit/DifferentialConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialConduitAPIMethod.php @@ -178,6 +178,7 @@ abstract class DifferentialConduitAPIMethod extends ConduitAPIMethod { $results = array(); foreach ($field_lists as $revision_phid => $field_list) { + $results[$revision_phid] = array(); foreach ($field_list->getFields() as $field) { $field_key = $field->getFieldKeyForConduit(); $value = $field->getConduitDictionaryValue(); @@ -185,6 +186,32 @@ abstract class DifferentialConduitAPIMethod extends ConduitAPIMethod { } } + // For compatibility, fill in these "custom fields" by querying for them + // efficiently. See T11404 for discussion. + + $legacy_edge_map = array( + 'phabricator:projects' => + PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, + 'phabricator:depends-on' => + DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST, + ); + + $query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs(array_keys($results)) + ->withEdgeTypes($legacy_edge_map); + + $query->execute(); + + foreach ($results as $revision_phid => $dict) { + foreach ($legacy_edge_map as $edge_key => $edge_type) { + $phid_list = $query->getDestinationPHIDs( + array($revision_phid), + array($edge_type)); + + $results[$revision_phid][$edge_key] = $phid_list; + } + } + return $results; } diff --git a/src/applications/differential/customfield/DifferentialParentRevisionsField.php b/src/applications/differential/customfield/DifferentialParentRevisionsField.php index 30ecea4bf2..5dbc99dd6f 100644 --- a/src/applications/differential/customfield/DifferentialParentRevisionsField.php +++ b/src/applications/differential/customfield/DifferentialParentRevisionsField.php @@ -7,10 +7,6 @@ final class DifferentialParentRevisionsField return 'differential:depends-on'; } - public function getFieldKeyForConduit() { - return 'phabricator:depends-on'; - } - public function getFieldName() { return pht('Parent Revisions'); } @@ -33,7 +29,10 @@ final class DifferentialParentRevisionsField } public function shouldAppearInConduitDictionary() { - return true; + // To improve performance, we exclude this field from Conduit results. + // See T11404 for discussion. In modern "differential.revision.search", + // this information is available efficiently as an attachment. + return false; } public function getConduitDictionaryValue() { diff --git a/src/applications/differential/customfield/DifferentialProjectsField.php b/src/applications/differential/customfield/DifferentialProjectsField.php index 53bd7293fd..425e30e132 100644 --- a/src/applications/differential/customfield/DifferentialProjectsField.php +++ b/src/applications/differential/customfield/DifferentialProjectsField.php @@ -91,7 +91,10 @@ final class DifferentialProjectsField } public function shouldAppearInConduitDictionary() { - return true; + // To improve performance, we exclude this field from Conduit results. + // See T11404 for discussion. In modern "differential.revision.search", + // this information is available efficiently as an attachment. + return false; } public function getApplicationTransactionMetadata() { From 64886b11d8d18fe4404378b3884865604c258715 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 31 Jul 2016 09:25:24 -0700 Subject: [PATCH 07/23] Remove expensive, pointless typeachecking in custom fields Summary: Ref T11404. On my system, this improves performance by 10-15% for `differential.revision.search`. `PhutilTypeSpec` provides high quality typechecking and is great for user-facing things that need good error messages. However, it's also a bit slow, and pointless here (the API is internal and it only has one possible option). I think I added this after writing `checkMap` just because I wanted to use it more often. My desire is sated after finding many reasonable ways to use it to give users high-quality error messages about things like configuration files. Test Plan: Profiled `differential.revision.search` before and after change, saw wall time drop from ~220ms to ~195ms. Reviewers: yelirekim, chad Reviewed By: chad Maniphest Tasks: T11404 Differential Revision: https://secure.phabricator.com/D16354 --- .../customfield/field/PhabricatorCustomField.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index cb561fca58..94c464efdd 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -112,12 +112,6 @@ abstract class PhabricatorCustomField extends Phobject { $object, array $options = array()) { - PhutilTypeSpec::checkMap( - $options, - array( - 'withDisabled' => 'optional bool', - )); - $field_objects = id(new PhutilSymbolLoader()) ->setAncestorClass($base_class) ->loadObjects(); From 1b192f746a6751e219c30fc45cbf3d05708eec2c Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 31 Jul 2016 09:41:17 -0700 Subject: [PATCH 08/23] Improve performance when constructing custom fields for objects Summary: Ref T11404. This improves things by about 10%: - Use `PhutilClassMapQuery`, which has slightly better caching. - Do a little less work to generate pretty error messages. - Make the "disabled" code a little faster (and sort of clearer, too?) by doing less fancy stuff. These are pretty minor adjustments and not the sort of optimizations I'd make normally, but this code gets called ~100x (once per revision) and generates ~10 fields normally, so even small savings can amount to something. (I also want to try to make `arc` faster in the next update, and improving Conduit performance helps with that.) Test Plan: Ran `differential.revision.search`, saw cost drop from ~195ms to ~170ms locally. Reviewers: yelirekim, chad Reviewed By: chad Maniphest Tasks: T11404 Differential Revision: https://secure.phabricator.com/D16355 --- .../field/PhabricatorCustomField.php | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index 94c464efdd..7791700a14 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -112,14 +112,13 @@ abstract class PhabricatorCustomField extends Phobject { $object, array $options = array()) { - $field_objects = id(new PhutilSymbolLoader()) + $field_objects = id(new PhutilClassMapQuery()) ->setAncestorClass($base_class) - ->loadObjects(); + ->execute(); $fields = array(); - $from_map = array(); foreach ($field_objects as $field_object) { - $current_class = get_class($field_object); + $field_object = clone $field_object; foreach ($field_object->createFields($object) as $field) { $key = $field->getFieldKey(); if (isset($fields[$key])) { @@ -127,11 +126,10 @@ abstract class PhabricatorCustomField extends Phobject { pht( "Both '%s' and '%s' define a custom field with ". "field key '%s'. Field keys must be unique.", - $from_map[$key], - $current_class, + get_class($fields[$key]), + get_class($field), $key)); } - $from_map[$key] = $current_class; $fields[$key] = $field; } } @@ -146,11 +144,13 @@ abstract class PhabricatorCustomField extends Phobject { if (empty($options['withDisabled'])) { foreach ($fields as $key => $field) { - $config = idx($spec, $key, array()) + array( - 'disabled' => $field->shouldDisableByDefault(), - ); + if (isset($spec[$key]['disabled'])) { + $is_disabled = $spec[$key]['disabled']; + } else { + $is_disabled = $field->shouldDisableByDefault(); + } - if (!empty($config['disabled'])) { + if ($is_disabled) { if ($field->canDisableField()) { unset($fields[$key]); } From cd8a9fd61ea68a01965cb3d695fa261fcc7ecb1b Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 31 Jul 2016 13:07:06 -0700 Subject: [PATCH 09/23] Resolve an issue with `differential.query` if no results are matched Fixes T11406. Auditors: chad --- .../differential/conduit/DifferentialConduitAPIMethod.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/applications/differential/conduit/DifferentialConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialConduitAPIMethod.php index 288aa60a02..fdcf4395dd 100644 --- a/src/applications/differential/conduit/DifferentialConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialConduitAPIMethod.php @@ -150,6 +150,10 @@ abstract class DifferentialConduitAPIMethod extends ConduitAPIMethod { array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); + if (!$revisions) { + return array(); + } + $field_lists = array(); foreach ($revisions as $revision) { $revision_phid = $revision->getPHID(); From 4865dbdff1998537fc6e8d167e42907d34701997 Mon Sep 17 00:00:00 2001 From: Mike Riley Date: Sun, 31 Jul 2016 20:54:44 +0000 Subject: [PATCH 10/23] Search builds based on who kicked them off Summary: It's only natural for users to be interested their own builds. We are also building in support for other sources of builds, the only formally supported way to run a build right now is via Herald. In our third party codebase, we designate an application as the "thing" that started builds which are scheduled and managed automatically by phabricator. I believe this is a common practice elsewhere in the codebase when you're at a loss for a real human identity and you need to apply some transactions. Test Plan: Ran some builds manually and saw them show up under the list of things I've run. Looking up builds based on those that had been started by a herald rule. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D16353 --- src/__phutil_library_map__.php | 4 ++ .../query/HarbormasterBuildQuery.php | 13 +++++ .../query/HarbormasterBuildSearchEngine.php | 16 ++++++ .../storage/build/HarbormasterBuild.php | 3 ++ .../HarbormasterBuildInitiatorDatasource.php | 22 +++++++++ .../herald/query/HeraldRuleQuery.php | 13 +++++ .../herald/storage/HeraldRule.php | 5 +- .../herald/typeahead/HeraldRuleDatasource.php | 49 +++++++++++++++++++ 8 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 src/applications/harbormaster/typeahead/HarbormasterBuildInitiatorDatasource.php create mode 100644 src/applications/herald/typeahead/HeraldRuleDatasource.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cb78038bbd..d4599a505c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1106,6 +1106,7 @@ phutil_register_library_map(array( 'HarbormasterBuildEngine' => 'applications/harbormaster/engine/HarbormasterBuildEngine.php', 'HarbormasterBuildFailureException' => 'applications/harbormaster/exception/HarbormasterBuildFailureException.php', 'HarbormasterBuildGraph' => 'applications/harbormaster/engine/HarbormasterBuildGraph.php', + 'HarbormasterBuildInitiatorDatasource' => 'applications/harbormaster/typeahead/HarbormasterBuildInitiatorDatasource.php', 'HarbormasterBuildLintMessage' => 'applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php', 'HarbormasterBuildListController' => 'applications/harbormaster/controller/HarbormasterBuildListController.php', 'HarbormasterBuildLog' => 'applications/harbormaster/storage/build/HarbormasterBuildLog.php', @@ -1281,6 +1282,7 @@ phutil_register_library_map(array( 'HeraldRepetitionPolicyConfig' => 'applications/herald/config/HeraldRepetitionPolicyConfig.php', 'HeraldRule' => 'applications/herald/storage/HeraldRule.php', 'HeraldRuleController' => 'applications/herald/controller/HeraldRuleController.php', + 'HeraldRuleDatasource' => 'applications/herald/typeahead/HeraldRuleDatasource.php', 'HeraldRuleEditor' => 'applications/herald/editor/HeraldRuleEditor.php', 'HeraldRuleListController' => 'applications/herald/controller/HeraldRuleListController.php', 'HeraldRulePHIDType' => 'applications/herald/phid/HeraldRulePHIDType.php', @@ -5651,6 +5653,7 @@ phutil_register_library_map(array( 'HarbormasterBuildEngine' => 'Phobject', 'HarbormasterBuildFailureException' => 'Exception', 'HarbormasterBuildGraph' => 'AbstractDirectedGraph', + 'HarbormasterBuildInitiatorDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'HarbormasterBuildLintMessage' => 'HarbormasterDAO', 'HarbormasterBuildListController' => 'HarbormasterController', 'HarbormasterBuildLog' => array( @@ -5865,6 +5868,7 @@ phutil_register_library_map(array( 'PhabricatorSubscribableInterface', ), 'HeraldRuleController' => 'HeraldController', + 'HeraldRuleDatasource' => 'PhabricatorTypeaheadDatasource', 'HeraldRuleEditor' => 'PhabricatorApplicationTransactionEditor', 'HeraldRuleListController' => 'HeraldController', 'HeraldRulePHIDType' => 'PhabricatorPHIDType', diff --git a/src/applications/harbormaster/query/HarbormasterBuildQuery.php b/src/applications/harbormaster/query/HarbormasterBuildQuery.php index 1ee1ecb3a8..cf6db6115b 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildQuery.php +++ b/src/applications/harbormaster/query/HarbormasterBuildQuery.php @@ -8,6 +8,7 @@ final class HarbormasterBuildQuery private $buildStatuses; private $buildablePHIDs; private $buildPlanPHIDs; + private $initiatorPHIDs; private $needBuildTargets; public function withIDs(array $ids) { @@ -35,6 +36,11 @@ final class HarbormasterBuildQuery return $this; } + public function withInitiatorPHIDs(array $initiator_phids) { + $this->initiatorPHIDs = $initiator_phids; + return $this; + } + public function needBuildTargets($need_targets) { $this->needBuildTargets = $need_targets; return $this; @@ -167,6 +173,13 @@ final class HarbormasterBuildQuery $this->buildPlanPHIDs); } + if ($this->initiatorPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'initiatorPHID IN (%Ls)', + $this->initiatorPHIDs); + } + return $where; } diff --git a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php index 0b07ca7150..e1d4215019 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php +++ b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php @@ -31,6 +31,14 @@ final class HarbormasterBuildSearchEngine ->setDescription( pht('Search for builds with given statuses.')) ->setDatasource(new HarbormasterBuildStatusDatasource()), + id(new PhabricatorSearchDatasourceField()) + ->setLabel(pht('Initiators')) + ->setKey('initiators') + ->setAliases(array('initiator')) + ->setDescription( + pht( + 'Search for builds started by someone or something in particular.')) + ->setDatasource(new HarbormasterBuildInitiatorDatasource()), ); } @@ -45,6 +53,10 @@ final class HarbormasterBuildSearchEngine $query->withBuildStatuses($map['statuses']); } + if ($map['initiators']) { + $query->withInitiatorPHIDs($map['initiators']); + } + return $query; } @@ -54,6 +66,7 @@ final class HarbormasterBuildSearchEngine protected function getBuiltinQueryNames() { return array( + 'initiated' => pht('My Builds'), 'all' => pht('All Builds'), 'waiting' => pht('Waiting'), 'active' => pht('Active'), @@ -66,6 +79,9 @@ final class HarbormasterBuildSearchEngine $query->setQueryKey($query_key); switch ($query_key) { + case 'initiated': + $viewer = $this->requireViewer(); + return $query->setParameter('initiators', array($viewer->getPHID())); case 'all': return $query; case 'waiting': diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 759f9b9fe0..5037e7371e 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -59,6 +59,9 @@ final class HarbormasterBuild extends HarbormasterDAO 'columns' => array('buildablePHID', 'planAutoKey'), 'unique' => true, ), + 'key_initiator' => array( + 'columns' => array('initiatorPHID'), + ), ), ) + parent::getConfiguration(); } diff --git a/src/applications/harbormaster/typeahead/HarbormasterBuildInitiatorDatasource.php b/src/applications/harbormaster/typeahead/HarbormasterBuildInitiatorDatasource.php new file mode 100644 index 0000000000..c66073e25e --- /dev/null +++ b/src/applications/harbormaster/typeahead/HarbormasterBuildInitiatorDatasource.php @@ -0,0 +1,22 @@ +datasourceQuery = $query; + return $this; + } + public function withTriggerObjectPHIDs(array $phids) { $this->triggerObjectPHIDs = $phids; return $this; @@ -219,6 +225,13 @@ final class HeraldRuleQuery extends PhabricatorCursorPagedPolicyAwareQuery { (int)$this->disabled); } + if ($this->datasourceQuery) { + $where[] = qsprintf( + $conn_r, + 'rule.name LIKE %>', + $this->datasourceQuery); + } + if ($this->triggerObjectPHIDs) { $where[] = qsprintf( $conn_r, diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php index 9902e952ef..19f9b95507 100644 --- a/src/applications/herald/storage/HeraldRule.php +++ b/src/applications/herald/storage/HeraldRule.php @@ -34,7 +34,7 @@ final class HeraldRule extends HeraldDAO return array( self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255', + 'name' => 'sort255', 'contentType' => 'text255', 'mustMatchAll' => 'bool', 'configVersion' => 'uint32', @@ -47,6 +47,9 @@ final class HeraldRule extends HeraldDAO 'repetitionPolicy' => 'uint32?', ), self::CONFIG_KEY_SCHEMA => array( + 'key_name' => array( + 'columns' => array('name(128)'), + ), 'key_author' => array( 'columns' => array('authorPHID'), ), diff --git a/src/applications/herald/typeahead/HeraldRuleDatasource.php b/src/applications/herald/typeahead/HeraldRuleDatasource.php new file mode 100644 index 0000000000..c2db05c9ea --- /dev/null +++ b/src/applications/herald/typeahead/HeraldRuleDatasource.php @@ -0,0 +1,49 @@ +getViewer(); + $raw_query = $this->getRawQuery(); + + $rules = id(new HeraldRuleQuery()) + ->setViewer($viewer) + ->withDatasourceQuery($raw_query) + ->execute(); + + $handles = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs(mpull($rules, 'getPHID')) + ->execute(); + + $results = array(); + foreach ($rules as $rule) { + $handle = $handles[$rule->getPHID()]; + + $result = id(new PhabricatorTypeaheadResult()) + ->setName($handle->getFullName()) + ->setPHID($handle->getPHID()); + + if ($rule->getIsDisabled()) { + $result->setClosed(pht('Archived')); + } + + $results[] = $result; + } + + return $results; + } +} From 98492765d31593cfba587416306d51103961945b Mon Sep 17 00:00:00 2001 From: Mike Riley Date: Sun, 31 Jul 2016 21:44:22 +0000 Subject: [PATCH 11/23] Subsume 'harbormaster.querybuilds' with a modern search API method Summary: We deprecate the existing API method used to access build information from the API, but preserve its response structure after calling through to the new method. I've cordoned off the fields I needed to define in order to meet the output structure by putting those fields in a search attachment. Test Plan: Used the API console and looked at the list view controller for builds. Old output structure: ```lang=json { "data": [ { "id": "16823", "phid": "PHID-HMBD-xghrwfz6luoye5rgc2hq", "uri": "https://secure.phabricator.com/harbormaster/build/16823/", "name": "Run Core Tests", "buildablePHID": "PHID-HMBB-s6ykzm2jzxz4ymduztq3", "buildPlanPHID": "PHID-HMCP-pcfxcgyoif67l3buc4zt", "buildStatus": "passed", "buildStatusName": "Passed" } ], "cursor": { "limit": 100, "after": "16823", "before": null } } ``` New output structure: ```lang=json { "data": [ { "id": 1, "type": "HMBD", "phid": "PHID-HMBD-qpgcmv67tzaauzayzit5", "uri": "http://ec2-54-165-244-168.compute-1.amazonaws.com/harbormaster/build/1/", "name": "arc lint + arc unit", "buildStatusName": "Passed", "buildablePHID": "PHID-HMBB-qdefith5uakkepqpjr2g", "buildPlanPHID": "PHID-HMCP-zswbhazb7ipmaf4plygg", "buildStatus": "passed", "initiatorPHID": "PHID-USER-rihx4366f3aczsvc2wtb", "dateCreated": 1450295643, "dateModified": 1450295644, "policy": { "view": "users", "edit": "users" } } ], "maps": {}, "query": { "queryKey": null }, "cursor": { "limit": 100, "after": null, "before": null, "order": null } } ``` Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D16356 --- src/__phutil_library_map__.php | 5 ++ ...arbormasterBuildSearchConduitAPIMethod.php | 18 ++++ ...arbormasterQueryBuildsConduitAPIMethod.php | 90 ++++++++----------- ...asterQueryBuildsSearchEngineAttachment.php | 28 ++++++ .../query/HarbormasterBuildSearchEngine.php | 12 +++ .../storage/build/HarbormasterBuild.php | 48 +++++++++- 6 files changed, 146 insertions(+), 55 deletions(-) create mode 100644 src/applications/harbormaster/conduit/HarbormasterBuildSearchConduitAPIMethod.php create mode 100644 src/applications/harbormaster/engineextension/HarbormasterQueryBuildsSearchEngineAttachment.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d4599a505c..e29c91ae0e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1131,6 +1131,7 @@ phutil_register_library_map(array( 'HarbormasterBuildPlanTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanTransactionQuery.php', 'HarbormasterBuildQuery' => 'applications/harbormaster/query/HarbormasterBuildQuery.php', 'HarbormasterBuildRequest' => 'applications/harbormaster/engine/HarbormasterBuildRequest.php', + 'HarbormasterBuildSearchConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildSearchConduitAPIMethod.php', 'HarbormasterBuildSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildSearchEngine.php', 'HarbormasterBuildStatus' => 'applications/harbormaster/constants/HarbormasterBuildStatus.php', 'HarbormasterBuildStatusDatasource' => 'applications/harbormaster/typeahead/HarbormasterBuildStatusDatasource.php', @@ -1204,6 +1205,7 @@ phutil_register_library_map(array( 'HarbormasterQueryAutotargetsConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterQueryAutotargetsConduitAPIMethod.php', 'HarbormasterQueryBuildablesConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterQueryBuildablesConduitAPIMethod.php', 'HarbormasterQueryBuildsConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php', + 'HarbormasterQueryBuildsSearchEngineAttachment' => 'applications/harbormaster/engineextension/HarbormasterQueryBuildsSearchEngineAttachment.php', 'HarbormasterRemarkupRule' => 'applications/harbormaster/remarkup/HarbormasterRemarkupRule.php', 'HarbormasterRunBuildPlansHeraldAction' => 'applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php', 'HarbormasterSchemaSpec' => 'applications/harbormaster/storage/HarbormasterSchemaSpec.php', @@ -5637,6 +5639,7 @@ phutil_register_library_map(array( 'HarbormasterDAO', 'PhabricatorApplicationTransactionInterface', 'PhabricatorPolicyInterface', + 'PhabricatorConduitResultInterface', ), 'HarbormasterBuildAbortedException' => 'Exception', 'HarbormasterBuildActionController' => 'HarbormasterController', @@ -5691,6 +5694,7 @@ phutil_register_library_map(array( 'HarbormasterBuildPlanTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'HarbormasterBuildQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildRequest' => 'Phobject', + 'HarbormasterBuildSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', 'HarbormasterBuildSearchEngine' => 'PhabricatorApplicationSearchEngine', 'HarbormasterBuildStatus' => 'Phobject', 'HarbormasterBuildStatusDatasource' => 'PhabricatorTypeaheadDatasource', @@ -5777,6 +5781,7 @@ phutil_register_library_map(array( 'HarbormasterQueryAutotargetsConduitAPIMethod' => 'HarbormasterConduitAPIMethod', 'HarbormasterQueryBuildablesConduitAPIMethod' => 'HarbormasterConduitAPIMethod', 'HarbormasterQueryBuildsConduitAPIMethod' => 'HarbormasterConduitAPIMethod', + 'HarbormasterQueryBuildsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment', 'HarbormasterRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'HarbormasterRunBuildPlansHeraldAction' => 'HeraldAction', 'HarbormasterSchemaSpec' => 'PhabricatorConfigSchemaSpec', diff --git a/src/applications/harbormaster/conduit/HarbormasterBuildSearchConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterBuildSearchConduitAPIMethod.php new file mode 100644 index 0000000000..daa3d6287e --- /dev/null +++ b/src/applications/harbormaster/conduit/HarbormasterBuildSearchConduitAPIMethod.php @@ -0,0 +1,18 @@ + 'optional list', @@ -27,65 +35,39 @@ final class HarbormasterQueryBuildsConduitAPIMethod protected function execute(ConduitAPIRequest $request) { $viewer = $request->getUser(); + $call = new ConduitCall( + 'harbormaster.build.search', + array_filter(array( + 'constraints' => array_filter(array( + 'ids' => $request->getValue('ids'), + 'phids' => $request->getValue('phids'), + 'statuses' => $request->getValue('buildStatuses'), + 'buildables' => $request->getValue('buildablePHIDs'), + 'plans' => $request->getValue('buildPlanPHIDs'), + )), + 'attachments' => array( + 'querybuilds' => true, + ), + 'limit' => $request->getValue('limit'), + 'before' => $request->getValue('before'), + 'after' => $request->getValue('after'), + ))); - $query = id(new HarbormasterBuildQuery()) - ->setViewer($viewer); + $subsumption = $call->setUser($viewer) + ->execute(); - $ids = $request->getValue('ids'); - if ($ids !== null) { - $query->withIDs($ids); + $data = []; + foreach ($subsumption['data'] as $build_data) { + $querybuilds = idxv($build_data, array('attachments', 'querybuilds'), []); + $fields = idx($build_data, 'fields', []); + unset($build_data['fields']); + unset($build_data['attachments']); + $data[] = array_mergev(array($build_data, $querybuilds, $fields)); } - $phids = $request->getValue('phids'); - if ($phids !== null) { - $query->withPHIDs($phids); - } + $subsumption['data'] = $data; - $statuses = $request->getValue('buildStatuses'); - if ($statuses !== null) { - $query->withBuildStatuses($statuses); - } - - $buildable_phids = $request->getValue('buildablePHIDs'); - if ($buildable_phids !== null) { - $query->withBuildablePHIDs($buildable_phids); - } - - $build_plan_phids = $request->getValue('buildPlanPHIDs'); - if ($build_plan_phids !== null) { - $query->withBuildPlanPHIDs($build_plan_phids); - } - - $pager = $this->newPager($request); - - $builds = $query->executeWithCursorPager($pager); - - $data = array(); - foreach ($builds as $build) { - - $id = $build->getID(); - $uri = '/harbormaster/build/'.$id.'/'; - $status = $build->getBuildStatus(); - $status_name = HarbormasterBuildStatus::getBuildStatusName($status); - - $data[] = array( - 'id' => $id, - 'phid' => $build->getPHID(), - 'uri' => PhabricatorEnv::getProductionURI($uri), - 'name' => $build->getBuildPlan()->getName(), - 'buildablePHID' => $build->getBuildablePHID(), - 'buildPlanPHID' => $build->getBuildPlanPHID(), - 'buildStatus' => $status, - 'buildStatusName' => $status_name, - ); - } - - $results = array( - 'data' => $data, - ); - - $results = $this->addPagerResults($results, $pager); - return $results; + return $subsumption; } } diff --git a/src/applications/harbormaster/engineextension/HarbormasterQueryBuildsSearchEngineAttachment.php b/src/applications/harbormaster/engineextension/HarbormasterQueryBuildsSearchEngineAttachment.php new file mode 100644 index 0000000000..ce46ee9cba --- /dev/null +++ b/src/applications/harbormaster/engineextension/HarbormasterQueryBuildsSearchEngineAttachment.php @@ -0,0 +1,28 @@ +getBuildStatus()); + return array( + 'uri' => PhabricatorEnv::getProductionURI($object->getURI()), + 'name' => $object->getName(), + 'buildStatusName' => $status_name, + ); + } + +} diff --git a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php index e1d4215019..948fea389c 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php +++ b/src/applications/harbormaster/query/HarbormasterBuildSearchEngine.php @@ -24,6 +24,12 @@ final class HarbormasterBuildSearchEngine ->setDescription( pht('Search for builds running a given build plan.')) ->setDatasource(new HarbormasterBuildPlanDatasource()), + id(new PhabricatorPHIDsSearchField()) + ->setLabel(pht('Buildables')) + ->setKey('buildables') + ->setAliases(array('buildable')) + ->setDescription( + pht('Search for builds running against particular buildables.')), id(new PhabricatorSearchDatasourceField()) ->setLabel(pht('Statuses')) ->setKey('statuses') @@ -42,6 +48,12 @@ final class HarbormasterBuildSearchEngine ); } + protected function getHiddenFields() { + return array( + 'buildables', + ); + } + protected function buildQueryFromParameters(array $map) { $query = $this->newQuery(); diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 5037e7371e..9255b6ec4a 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -3,7 +3,8 @@ final class HarbormasterBuild extends HarbormasterDAO implements PhabricatorApplicationTransactionInterface, - PhabricatorPolicyInterface { + PhabricatorPolicyInterface, + PhabricatorConduitResultInterface { protected $buildablePHID; protected $buildPlanPHID; @@ -397,4 +398,49 @@ final class HarbormasterBuild extends HarbormasterDAO return pht('A build inherits policies from its buildable.'); } + +/* -( PhabricatorConduitResultInterface )---------------------------------- */ + + + public function getFieldSpecificationsForConduit() { + return array( + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('buildablePHID') + ->setType('phid') + ->setDescription(pht('PHID of the object this build is building.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('buildPlanPHID') + ->setType('phid') + ->setDescription(pht('PHID of the build plan being run.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('buildStatus') + ->setType('map') + ->setDescription(pht('The current status of this build.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('initiatorPHID') + ->setType('phid') + ->setDescription(pht('The person (or thing) that started this build.')), + ); + } + + public function getFieldValuesForConduit() { + $status = $this->getBuildStatus(); + return array( + 'buildablePHID' => $this->getBuildablePHID(), + 'buildPlanPHID' => $this->getBuildPlanPHID(), + 'buildStatus' => array( + 'value' => $status, + 'name' => HarbormasterBuildStatus::getBuildStatusName($status), + ), + 'initiatorPHID' => nonempty($this->getInitiatorPHID(), null), + ); + } + + public function getConduitSearchAttachments() { + return array( + id(new HarbormasterQueryBuildsSearchEngineAttachment()) + ->setAttachmentKey('querybuilds'), + ); + } + } From 5380d877921eb9e0e37f43dc606c81dda43568fa Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 1 Aug 2016 04:27:41 -0700 Subject: [PATCH 12/23] Use long array syntax for compatibility instead of short array syntax Summary: Fixes T11409. This syntax isn't compatible with older PHP. Test Plan: Ran `arc lint` on the file. Reviewers: yelirekim, chad Reviewed By: chad Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T11409 Differential Revision: https://secure.phabricator.com/D16358 --- .../conduit/HarbormasterQueryBuildsConduitAPIMethod.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php index a45d6d80de..a76f53a362 100644 --- a/src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php +++ b/src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php @@ -56,10 +56,13 @@ final class HarbormasterQueryBuildsConduitAPIMethod $subsumption = $call->setUser($viewer) ->execute(); - $data = []; + $data = array(); foreach ($subsumption['data'] as $build_data) { - $querybuilds = idxv($build_data, array('attachments', 'querybuilds'), []); - $fields = idx($build_data, 'fields', []); + $querybuilds = idxv( + $build_data, + array('attachments', 'querybuilds'), + array()); + $fields = idx($build_data, 'fields', array()); unset($build_data['fields']); unset($build_data['attachments']); $data[] = array_mergev(array($build_data, $querybuilds, $fields)); From 11e84c166a38b632ce9d1ced4eaf46217603af6f Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 1 Aug 2016 12:06:35 -0700 Subject: [PATCH 13/23] Redesign Application Search Summary: This moves aphront-side-nav to use same table css display as profile nav. Slightly less code to support. Cleans up AppSearch UI, think I've gotten all the edge cases here, but bang on it, can hold until after release cut. Test Plan: Config, Maniphest, Differential, Diffusion, Home. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16346 --- resources/celerity/map.php | 34 +++--- resources/celerity/packages.php | 2 +- .../PhabricatorCalendarEventSearchEngine.php | 8 +- .../CelerityDefaultPostprocessor.php | 1 + ...PhabricatorApplicationSearchController.php | 34 ++++-- src/view/control/AphrontCursorPagerView.php | 6 +- src/view/layout/AphrontSideNavFilterView.php | 48 +++----- src/view/phui/PHUIObjectBoxView.php | 2 + src/view/phui/PHUIPagerView.php | 2 +- .../phui/calendar/PHUICalendarDayView.php | 4 + .../phui/calendar/PHUICalendarMonthView.php | 3 +- .../rsrc/css/aphront/phabricator-nav-view.css | 34 +++--- .../search/application-search-view.css | 50 ++++++++ .../css/layout/phabricator-side-menu-view.css | 59 ---------- .../rsrc/css/phui/calendar/phui-calendar.css | 7 ++ webroot/rsrc/css/phui/phui-basic-nav-view.css | 107 ++++++++++++++++++ webroot/rsrc/css/phui/phui-crumbs-view.css | 3 +- webroot/rsrc/css/phui/phui-document-pro.css | 7 +- webroot/rsrc/css/phui/phui-header-view.css | 1 + webroot/rsrc/css/phui/phui-profile-menu.css | 25 +--- 20 files changed, 271 insertions(+), 166 deletions(-) create mode 100644 webroot/rsrc/css/application/search/application-search-view.css delete mode 100644 webroot/rsrc/css/layout/phabricator-side-menu-view.css create mode 100644 webroot/rsrc/css/phui/phui-basic-nav-view.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 33f5f5af12..f5334ee6a5 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => '8b87d014', + 'core.pkg.css' => 'a90e12a7', 'core.pkg.js' => '13c7e56a', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '3fb7f532', @@ -24,7 +24,7 @@ return array( 'rsrc/css/aphront/multi-column.css' => 'fd18389d', 'rsrc/css/aphront/notification.css' => '3f6c89c9', 'rsrc/css/aphront/panel-view.css' => '8427b78d', - 'rsrc/css/aphront/phabricator-nav-view.css' => 'ac79a758', + 'rsrc/css/aphront/phabricator-nav-view.css' => '370da55a', 'rsrc/css/aphront/table-view.css' => '832656fd', 'rsrc/css/aphront/tokenizer.css' => '056da01b', 'rsrc/css/aphront/tooltip.css' => '1a07aea8', @@ -99,6 +99,7 @@ return array( 'rsrc/css/application/releeph/releeph-preview-branch.css' => 'b7a6f4a5', 'rsrc/css/application/releeph/releeph-request-differential-create-dialog.css' => '8d8b92cd', 'rsrc/css/application/releeph/releeph-request-typeahead.css' => '667a48ae', + 'rsrc/css/application/search/application-search-view.css' => 'b3e0e5ef', 'rsrc/css/application/search/search-results.css' => '7dea472c', 'rsrc/css/application/slowvote/slowvote.css' => 'a94b7230', 'rsrc/css/application/tokens/tokens.css' => '3d0f239e', @@ -113,22 +114,22 @@ return array( 'rsrc/css/font/font-lato.css' => 'c7ccd872', 'rsrc/css/font/phui-font-icon-base.css' => '6449bce8', 'rsrc/css/layout/phabricator-filetree-view.css' => 'fccf9f82', - 'rsrc/css/layout/phabricator-side-menu-view.css' => 'dd849797', 'rsrc/css/layout/phabricator-source-code-view.css' => 'cbeef983', 'rsrc/css/phui/calendar/phui-calendar-day.css' => '572b1893', 'rsrc/css/phui/calendar/phui-calendar-list.css' => 'fcc9fb41', 'rsrc/css/phui/calendar/phui-calendar-month.css' => '8e10e92c', - 'rsrc/css/phui/calendar/phui-calendar.css' => 'daadaf39', + 'rsrc/css/phui/calendar/phui-calendar.css' => '477acfaa', 'rsrc/css/phui/phui-action-list.css' => 'c5eba19d', 'rsrc/css/phui/phui-action-panel.css' => '91c7b835', 'rsrc/css/phui/phui-badge.css' => '3baef8db', + 'rsrc/css/phui/phui-basic-nav-view.css' => 'bfc71dd0', 'rsrc/css/phui/phui-big-info-view.css' => 'bd903741', 'rsrc/css/phui/phui-box.css' => '5c8387cf', 'rsrc/css/phui/phui-button.css' => '4a5fbe3d', 'rsrc/css/phui/phui-chart.css' => '6bf6f78e', - 'rsrc/css/phui/phui-crumbs-view.css' => 'b4fa5755', + 'rsrc/css/phui/phui-crumbs-view.css' => '9dac418c', 'rsrc/css/phui/phui-curtain-view.css' => '7148ae25', - 'rsrc/css/phui/phui-document-pro.css' => 'a3730b94', + 'rsrc/css/phui/phui-document-pro.css' => 'dc3d46ed', 'rsrc/css/phui/phui-document-summary.css' => '9ca48bdf', 'rsrc/css/phui/phui-document.css' => '715aedfb', 'rsrc/css/phui/phui-feed-story.css' => 'aa49845d', @@ -136,7 +137,7 @@ return array( 'rsrc/css/phui/phui-form-view.css' => 'fab0a10f', 'rsrc/css/phui/phui-form.css' => 'aac1d51d', 'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f', - 'rsrc/css/phui/phui-header-view.css' => '4c7dd8f5', + 'rsrc/css/phui/phui-header-view.css' => '06385974', 'rsrc/css/phui/phui-hovercard.css' => 'de1a2119', 'rsrc/css/phui/phui-icon-set-selector.css' => '1ab67aad', 'rsrc/css/phui/phui-icon.css' => 'd0534b71', @@ -148,7 +149,7 @@ return array( 'rsrc/css/phui/phui-object-item-list-view.css' => '8d99e42b', 'rsrc/css/phui/phui-pager.css' => 'bea33d23', 'rsrc/css/phui/phui-pinboard-view.css' => '2495140e', - 'rsrc/css/phui/phui-profile-menu.css' => 'c8557f33', + 'rsrc/css/phui/phui-profile-menu.css' => '8a3fc181', 'rsrc/css/phui/phui-property-list-view.css' => '6d8e58ac', 'rsrc/css/phui/phui-remarkup-preview.css' => '1a8f2591', 'rsrc/css/phui/phui-segment-bar-view.css' => '46342871', @@ -541,6 +542,7 @@ return array( 'aphront-tokenizer-control-css' => '056da01b', 'aphront-tooltip-css' => '1a07aea8', 'aphront-typeahead-control-css' => 'd4f16145', + 'application-search-view-css' => 'b3e0e5ef', 'auth-css' => '0877ed6e', 'bulk-job-css' => 'df9c1d4a', 'changeset-view-manager' => 'a2828756', @@ -784,7 +786,7 @@ return array( 'phabricator-keyboard-shortcut' => '1ae869f2', 'phabricator-keyboard-shortcut-manager' => '4a021c10', 'phabricator-main-menu-view' => 'b623169f', - 'phabricator-nav-view-css' => 'ac79a758', + 'phabricator-nav-view-css' => '370da55a', 'phabricator-notification' => 'ccf1cbf8', 'phabricator-notification-css' => '3f6c89c9', 'phabricator-notification-menu-css' => 'f31c0bde', @@ -794,7 +796,6 @@ return array( 'phabricator-remarkup-css' => '523d34bb', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', - 'phabricator-side-menu-view-css' => 'dd849797', 'phabricator-slowvote-css' => 'a94b7230', 'phabricator-source-code-view-css' => 'cbeef983', 'phabricator-standard-page-view' => 'e709f6d0', @@ -824,26 +825,27 @@ return array( 'phriction-document-css' => '4282e4ad', 'phui-action-panel-css' => '91c7b835', 'phui-badge-view-css' => '3baef8db', + 'phui-basic-nav-view-css' => 'bfc71dd0', 'phui-big-info-view-css' => 'bd903741', 'phui-box-css' => '5c8387cf', 'phui-button-css' => '4a5fbe3d', - 'phui-calendar-css' => 'daadaf39', + 'phui-calendar-css' => '477acfaa', 'phui-calendar-day-css' => '572b1893', 'phui-calendar-list-css' => 'fcc9fb41', 'phui-calendar-month-css' => '8e10e92c', 'phui-chart-css' => '6bf6f78e', - 'phui-crumbs-view-css' => 'b4fa5755', + 'phui-crumbs-view-css' => '9dac418c', 'phui-curtain-view-css' => '7148ae25', 'phui-document-summary-view-css' => '9ca48bdf', 'phui-document-view-css' => '715aedfb', - 'phui-document-view-pro-css' => 'a3730b94', + 'phui-document-view-pro-css' => 'dc3d46ed', 'phui-feed-story-css' => 'aa49845d', 'phui-font-icon-base-css' => '6449bce8', 'phui-fontkit-css' => '9cda225e', 'phui-form-css' => 'aac1d51d', 'phui-form-view-css' => 'fab0a10f', 'phui-head-thing-view-css' => 'fd311e5f', - 'phui-header-view-css' => '4c7dd8f5', + 'phui-header-view-css' => '06385974', 'phui-hovercard' => '1bd28176', 'phui-hovercard-view-css' => 'de1a2119', 'phui-icon-set-selector-css' => '1ab67aad', @@ -857,7 +859,7 @@ return array( 'phui-object-item-list-view-css' => '8d99e42b', 'phui-pager-css' => 'bea33d23', 'phui-pinboard-view-css' => '2495140e', - 'phui-profile-menu-css' => 'c8557f33', + 'phui-profile-menu-css' => '8a3fc181', 'phui-property-list-view-css' => '6d8e58ac', 'phui-remarkup-preview-css' => '1a8f2591', 'phui-segment-bar-view-css' => '46342871', @@ -2252,7 +2254,7 @@ return array( 'phui-header-view-css', 'phabricator-filetree-view-css', 'phabricator-nav-view-css', - 'phabricator-side-menu-view-css', + 'phui-basic-nav-view-css', 'phui-crumbs-view-css', 'phui-object-item-list-view-css', 'global-drag-and-drop-css', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index 2de94b5a8c..103d395f79 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -113,7 +113,7 @@ return array( 'phui-header-view-css', 'phabricator-filetree-view-css', 'phabricator-nav-view-css', - 'phabricator-side-menu-view-css', + 'phui-basic-nav-view-css', 'phui-crumbs-view-css', 'phui-object-item-list-view-css', 'global-drag-and-drop-css', diff --git a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php index f093130bb4..9420261a59 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php @@ -369,13 +369,13 @@ final class PhabricatorCalendarEventSearchEngine ->setName($from->format('F Y')); $header = id(new PHUIHeaderView()) + ->setProfileHeader(true) ->setHeader($from->format('F Y')); return id(new PhabricatorApplicationSearchResultView()) ->setCrumbs($crumbs) ->setHeader($header) - ->setContent($month_view) - ->setCollapsed(true); + ->setContent($month_view); } private function buildCalendarDayView( @@ -443,13 +443,13 @@ final class PhabricatorCalendarEventSearchEngine ); $header = id(new PHUIHeaderView()) + ->setProfileHeader(true) ->setHeader($from->format('D, F jS')); return id(new PhabricatorApplicationSearchResultView()) ->setCrumbs($crumbs) ->setHeader($header) - ->setContent($day_view) - ->setCollapsed(true); + ->setContent($day_view); } private function getDisplayYearAndMonthAndDay( diff --git a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php index 7eb3b248c1..7b2fed7686 100644 --- a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php +++ b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php @@ -194,6 +194,7 @@ final class CelerityDefaultPostprocessor // Background color for "most" themes. 'page.background' => '#f8f8fb', + 'page.sidenav' => '#f0f0f2', 'menu.profile.text' => 'rgba(255,255,255,.8)', 'menu.profile.text.selected' => 'rgba(255,255,255,1)', diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php index b9ed90e80e..164e9aabbc 100644 --- a/src/applications/search/controller/PhabricatorApplicationSearchController.php +++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php @@ -193,10 +193,12 @@ final class PhabricatorApplicationSearchController } $header = id(new PHUIHeaderView()) - ->setHeader($title); + ->setHeader($title) + ->setProfileHeader(true); $box = id(new PHUIObjectBoxView()) - ->setHeader($header); + ->setHeader($header) + ->addClass('application-search-results'); if ($run_query || $named_query) { $box->setShowHide( @@ -281,9 +283,8 @@ final class PhabricatorApplicationSearchController if ($pager->willShowPagingControls()) { $pager_box = id(new PHUIBoxView()) - ->addPadding(PHUI::PADDING_MEDIUM) - ->addMargin(PHUI::MARGIN_LARGE) - ->setBorder(true) + ->setColor(PHUIBoxView::GREY) + ->addClass('application-search-pager') ->appendChild($pager); $body[] = $pager_box; } @@ -308,7 +309,8 @@ final class PhabricatorApplicationSearchController } $crumbs = $parent - ->buildApplicationCrumbs(); + ->buildApplicationCrumbs() + ->setBorder(true); if ($more_crumbs) { $query_uri = $engine->getQueryResultsPageURI($saved_query->getQueryKey()); @@ -321,12 +323,15 @@ final class PhabricatorApplicationSearchController $crumbs->addTextCrumb($title); } + require_celerity_resource('application-search-view-css'); + return $this->newPage() ->setApplicationMenu($this->buildApplicationMenu()) ->setTitle(pht('Query: %s', $title)) ->setCrumbs($crumbs) ->setNavigation($nav) - ->appendChild($body); + ->appendChild($body) + ->addClass('application-search-view'); } private function processEditRequest() { @@ -403,19 +408,28 @@ final class PhabricatorApplicationSearchController $crumbs = $parent ->buildApplicationCrumbs() - ->addTextCrumb(pht('Saved Queries'), $engine->getQueryManagementURI()); + ->addTextCrumb(pht('Saved Queries'), $engine->getQueryManagementURI()) + ->setBorder(true); $nav->selectFilter('query/edit'); + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Saved Queries')) + ->setProfileHeader(true); + $box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Saved Queries')) - ->setObjectList($list); + ->setHeader($header) + ->setObjectList($list) + ->addClass('application-search-results'); + + require_celerity_resource('application-search-view-css'); return $this->newPage() ->setApplicationMenu($this->buildApplicationMenu()) ->setTitle(pht('Saved Queries')) ->setCrumbs($crumbs) ->setNavigation($nav) + ->addClass('application-search-view') ->appendChild($box); } diff --git a/src/view/control/AphrontCursorPagerView.php b/src/view/control/AphrontCursorPagerView.php index 3574c07fa9..4608158481 100644 --- a/src/view/control/AphrontCursorPagerView.php +++ b/src/view/control/AphrontCursorPagerView.php @@ -148,7 +148,7 @@ final class AphrontCursorPagerView extends AphrontView { ->setHref($first_uri) ->setIcon($icon) ->addClass('mml') - ->setColor(PHUIButtonView::SIMPLE) + ->setColor(PHUIButtonView::GREY) ->setText(pht('First')); } @@ -161,7 +161,7 @@ final class AphrontCursorPagerView extends AphrontView { ->setHref($prev_uri) ->setIcon($icon) ->addClass('mml') - ->setColor(PHUIButtonView::SIMPLE) + ->setColor(PHUIButtonView::GREY) ->setText(pht('Prev')); } @@ -174,7 +174,7 @@ final class AphrontCursorPagerView extends AphrontView { ->setHref($next_uri) ->setIcon($icon, false) ->addClass('mml') - ->setColor(PHUIButtonView::SIMPLE) + ->setColor(PHUIButtonView::GREY) ->setText(pht('Next')); } diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index d59bf5ccac..8baa1f8499 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/src/view/layout/AphrontSideNavFilterView.php @@ -193,24 +193,18 @@ final class AphrontSideNavFilterView extends AphrontView { } } - require_celerity_resource('phabricator-side-menu-view-css'); + require_celerity_resource('phui-basic-nav-view-css'); return $this->renderFlexNav(); } private function renderFlexNav() { require_celerity_resource('phabricator-nav-view-css'); + require_celerity_resource('phui-profile-menu-css'); $nav_classes = array(); $nav_classes[] = 'phabricator-nav'; - if ($this->getIsProfileMenu()) { - require_celerity_resource('phui-profile-menu-css'); - // No class, we're going to put it on the shell instead. - } else { - $nav_classes[] = 'phabricator-basic-nav'; - } - $nav_id = null; $drag_id = null; $content_id = celerity_generate_unique_node_id(); @@ -284,16 +278,6 @@ final class AphrontSideNavFilterView extends AphrontView { $nav_classes = array_merge($nav_classes, $this->classes); - $footer = $this->footer; - - if ($this->getIsProfileMenu()) { - $internal_footer = $footer; - $external_footer = null; - } else { - $internal_footer = null; - $external_footer = $footer; - } - $menu = phutil_tag( 'div', array( @@ -312,26 +296,28 @@ final class AphrontSideNavFilterView extends AphrontView { array( $crumbs, $this->renderChildren(), - $internal_footer, + $this->footer, )), )); + $classes = array(); + $classes[] = 'phui-navigation-shell'; + if ($this->getIsProfileMenu()) { - $shell = phutil_tag( - 'div', - array( - 'class' => 'phui-navigation-shell phui-profile-menu', - ), - array( - $menu, - )); + $classes[] = 'phui-profile-menu'; } else { - $shell = array( - $menu, - $external_footer, - ); + $classes[] = 'phui-basic-nav'; } + $shell = phutil_tag( + 'div', + array( + 'class' => implode(' ', $classes), + ), + array( + $menu, + )); + return $shell; } diff --git a/src/view/phui/PHUIObjectBoxView.php b/src/view/phui/PHUIObjectBoxView.php index 5c6cef49a3..1e1d9898ec 100644 --- a/src/view/phui/PHUIObjectBoxView.php +++ b/src/view/phui/PHUIObjectBoxView.php @@ -204,6 +204,7 @@ final class PHUIObjectBoxView extends AphrontTagView { ->addSigil('reveal-content') ->setID($hide_action_id) ->setStyle($hide_style) + ->setIcon('fa-search') ->setHref($this->showHideHref) ->setMetaData( array( @@ -216,6 +217,7 @@ final class PHUIObjectBoxView extends AphrontTagView { ->setTag('a') ->addSigil('reveal-content') ->setStyle($show_style) + ->setIcon('fa-search') ->setHref('#') ->setID($show_action_id) ->setMetaData( diff --git a/src/view/phui/PHUIPagerView.php b/src/view/phui/PHUIPagerView.php index 1e14522ca7..232d4ab747 100644 --- a/src/view/phui/PHUIPagerView.php +++ b/src/view/phui/PHUIPagerView.php @@ -204,7 +204,7 @@ final class PHUIPagerView extends AphrontView { $rendered_links[] = id(new PHUIButtonView()) ->setTag('a') ->setHref($link) - ->setColor(PHUIButtonView::SIMPLE) + ->setColor(PHUIButtonView::GREY) ->addClass('mml') ->addClass($class) ->setText($label); diff --git a/src/view/phui/calendar/PHUICalendarDayView.php b/src/view/phui/calendar/PHUICalendarDayView.php index a2cab191a5..5a2a8ee178 100644 --- a/src/view/phui/calendar/PHUICalendarDayView.php +++ b/src/view/phui/calendar/PHUICalendarDayView.php @@ -191,6 +191,10 @@ final class PHUICalendarDayView extends AphrontView { ->addColumn($table_box, 'thirds phui-day-view-column') ->setFluidLayout(true); + $layout = id(new PHUIBoxView()) + ->appendChild($layout) + ->addClass('phui-calendar-box'); + return $layout; } diff --git a/src/view/phui/calendar/PHUICalendarMonthView.php b/src/view/phui/calendar/PHUICalendarMonthView.php index 61fd8b1e99..8272eb007f 100644 --- a/src/view/phui/calendar/PHUICalendarMonthView.php +++ b/src/view/phui/calendar/PHUICalendarMonthView.php @@ -183,7 +183,8 @@ final class PHUICalendarMonthView extends AphrontView { $box = id(new PHUIObjectBoxView()) ->setHeader($this->renderCalendarHeader($this->getDateTime())) ->appendChild($table) - ->setFormErrors($warnings); + ->setFormErrors($warnings) + ->addClass('phui-calendar-box'); if ($this->error) { $box->setInfoView($this->error); diff --git a/webroot/rsrc/css/aphront/phabricator-nav-view.css b/webroot/rsrc/css/aphront/phabricator-nav-view.css index 7dbb56da07..1dac52b671 100644 --- a/webroot/rsrc/css/aphront/phabricator-nav-view.css +++ b/webroot/rsrc/css/aphront/phabricator-nav-view.css @@ -20,23 +20,21 @@ display: block; } -.device-desktop .phabricator-side-menu-home .phabricator-nav-content, -.device-tablet .phabricator-side-menu-home .phabricator-nav-content { - margin-left: 205px; +/* Home Sidenav */ +.phui-basic-nav.phui-navigation-shell + .phabricator-side-menu-home .phabricator-nav-local { + padding-top: 16px; + padding-right: 0; + background-color: transparent; + width: 205px; } -.phabricator-nav-local { - width: 205px; - position: absolute; - left: 0; - white-space: nowrap; - overflow-x: hidden; - overflow-y: auto; - margin-top: 8px; -} - -.phabricator-side-menu-home .phabricator-nav-local { - margin-top: 16px; +.device-phone .phui-basic-nav.phui-navigation-shell + .phabricator-side-menu-home .phabricator-nav-local { + padding-top: 0; + padding-right: 0; + background-color: transparent; + width: auto; } .phabricator-nav-drag { @@ -68,8 +66,10 @@ margin-left: 212px; } -.device-desktop .has-local-nav .phabricator-nav-content { - margin-left: 205px; +.device-desktop .phui-navigation-shell .has-drag-nav .phabricator-nav-local { + width: 205px; + padding: 0; + background: transparent; } .device-phone .phabricator-side-menu-home .phabricator-nav-content { diff --git a/webroot/rsrc/css/application/search/application-search-view.css b/webroot/rsrc/css/application/search/application-search-view.css new file mode 100644 index 0000000000..24ee18c847 --- /dev/null +++ b/webroot/rsrc/css/application/search/application-search-view.css @@ -0,0 +1,50 @@ +/** + * @provides application-search-view-css + */ + +.application-search-view { + background-color: #fff; +} + +.application-search-view .application-search-results.phui-object-box { + margin: 0; + padding: 0 16px 24px; +} + +.application-search-view .application-search-results .phui-profile-header { + padding: 16px 8px; + border-bottom: 1px solid {$thinblueborder}; +} + +.application-search-results + .phui-profile-header.phui-header-shell .phui-header-header { + font-size: 20px; +} + +.device-phone.application-search-view .application-search-results + .phui-profile-header { + padding: 12px 0; +} + +.device-phone .application-search-results + .phui-profile-header.phui-header-shell .phui-header-header { + font-size: 16px; +} + +.device-phone.application-search-view + .application-search-results.phui-object-box { + padding: 0 12px; + } + +.application-search-view .phui-box-border { + border: none; +} + +.application-search-pager { + margin: 0 16px 16px 16px; + padding: 8px; +} + +.device-phone .application-search-pager { + margin: 12px; +} diff --git a/webroot/rsrc/css/layout/phabricator-side-menu-view.css b/webroot/rsrc/css/layout/phabricator-side-menu-view.css deleted file mode 100644 index b64fe5fbb5..0000000000 --- a/webroot/rsrc/css/layout/phabricator-side-menu-view.css +++ /dev/null @@ -1,59 +0,0 @@ -/** - * @provides phabricator-side-menu-view-css - */ - -.phabricator-basic-nav .phabricator-side-menu .phui-list-item-view { - display: block; - white-space: nowrap; - text-decoration: none; - font-size: 13px; - -webkit-font-smoothing: antialiased; -} - -.phabricator-basic-nav .phabricator-side-menu .phui-list-item-href { - display: block; - padding: 6px 8px 6px 24px; - color: {$darkbluetext}; - border-top-right-radius: 3px; - border-bottom-right-radius: 3px; -} - -.phabricator-basic-nav .phabricator-side-menu .phui-list-item-icon { - margin-left: -12px; - text-align: center; - width: 24px; -} - -.phabricator-basic-nav .phabricator-side-menu .phui-list-item-selected { - background-color: rgba({$alphablack},.05); - border-left: 4px solid {$sky}; - border-top-right-radius: 3px; - border-bottom-right-radius: 3px; - font-weight: bold; -} - -.device-desktop .phabricator-basic-nav .phabricator-side-menu - .phui-list-item-selected - a.phui-list-item-href:hover { - background-color: rgba({$alphablack},.05); -} - -.phabricator-basic-nav .phabricator-side-menu .phui-list-item-selected - .phui-list-item-href { - padding-left: 20px; -} - -.phabricator-basic-nav .phabricator-side-menu .phui-list-item-type-label { - padding: 6px 8px 4px 12px; - color: {$darkbluetext}; - text-transform: uppercase; - font-size: 12px; - font-weight: bold; - border-style: solid; -} - -.device-desktop .phabricator-basic-nav .phabricator-side-menu - a.phui-list-item-href:hover { - text-decoration: none; - background-color: rgba({$alphablack},.07); -} diff --git a/webroot/rsrc/css/phui/calendar/phui-calendar.css b/webroot/rsrc/css/phui/calendar/phui-calendar.css index 6c369b21df..ab59886fd8 100644 --- a/webroot/rsrc/css/phui/calendar/phui-calendar.css +++ b/webroot/rsrc/css/phui/calendar/phui-calendar.css @@ -8,6 +8,13 @@ background: rgba(255, 255, 255, 0.75); } +.application-search-view div.phui-calendar-box { + border-left: 1px solid {$thinblueborder}; + border-right: 1px solid {$thinblueborder}; + border-bottom: 1px solid {$lightblueborder}; + border-radius: 0; +} + .phui-calendar-list a { color: {$greytext}; } diff --git a/webroot/rsrc/css/phui/phui-basic-nav-view.css b/webroot/rsrc/css/phui/phui-basic-nav-view.css new file mode 100644 index 0000000000..f421579d70 --- /dev/null +++ b/webroot/rsrc/css/phui/phui-basic-nav-view.css @@ -0,0 +1,107 @@ +/** + * @provides phui-basic-nav-view-css + */ + +.device-desktop .phui-navigation-shell { + display: table; + width: 100%; + height: calc(100vh - {$menu.main.height}); +} + +.device-desktop .phui-navigation-shell .phabricator-nav { + display: table-row; +} + +.device-desktop .phui-navigation-shell .phabricator-nav-local { + display: table-cell; + position: relative; + vertical-align: top; + width: {$menu.profile.width}; + max-width: {$menu.profile.width}; + margin-top: 0; + overflow: hidden; +} + +.phui-basic-nav.phui-navigation-shell .phabricator-nav-local { + width: 205px; + padding-top: 4px; + padding-right: 8px; +} + +.phui-basic-nav .phabricator-side-menu { + background-color: {$page.sidenav}; +} + +.phui-two-column-view .phui-basic-nav.phui-navigation-shell + .phabricator-nav-local { + width: {$menu.profile.width}; + max-width: {$menu.profile.width}; + padding-right: 0; + padding-top: 0; +} + +.phui-two-column-view .phui-basic-nav .phabricator-side-menu { + background-color: #fff; +} + +.phui-basic-nav .phabricator-side-menu { + background-color: {$page.sidenav}; +} + +.phui-basic-nav .phabricator-side-menu .phui-list-item-view { + display: block; + white-space: nowrap; + text-decoration: none; + font-size: 13px; + -webkit-font-smoothing: antialiased; +} + +.phui-basic-nav .phabricator-side-menu .phui-list-item-href { + display: block; + padding: 6px 8px 6px 24px; + color: {$darkbluetext}; + border-top-right-radius: 3px; + border-bottom-right-radius: 3px; + overflow: hidden; + text-overflow: ellipsis +} + +.phui-basic-nav .phabricator-side-menu .phui-list-item-icon { + margin-left: -12px; + text-align: center; + width: 24px; +} + +.phui-basic-nav .phabricator-side-menu .phui-list-item-selected { + background-color: rgba({$alphablack},.05); + border-left: 4px solid {$sky}; + border-top-right-radius: 3px; + border-bottom-right-radius: 3px; + font-weight: bold; +} + +.device-desktop .phui-basic-nav .phabricator-side-menu + .phui-list-item-selected + a.phui-list-item-href:hover { + background-color: rgba({$alphablack},.05); +} + +.phui-basic-nav .phabricator-side-menu .phui-list-item-selected + .phui-list-item-href { + padding-left: 20px; +} + +.phui-basic-nav .phabricator-side-menu .phui-list-item-type-label { + padding: 6px 8px 4px 12px; + color: {$darkbluetext}; + text-transform: uppercase; + font-size: 12px; + font-weight: bold; + border-style: solid; +} + +.device-desktop .phui-basic-nav .phabricator-side-menu + a.phui-list-item-href:hover { + text-decoration: none; + background-color: rgba({$alphablack},.07); +} diff --git a/webroot/rsrc/css/phui/phui-crumbs-view.css b/webroot/rsrc/css/phui/phui-crumbs-view.css index dd070647fb..486ed1d5d3 100644 --- a/webroot/rsrc/css/phui/phui-crumbs-view.css +++ b/webroot/rsrc/css/phui/phui-crumbs-view.css @@ -5,11 +5,12 @@ .phui-crumbs-view { overflow: hidden; vertical-align: top; - padding: 0 20px 0 28px; + padding: 0 12px 0 20px; /* TODO: Position this over the slider for Differential's file tree view. Remove this once that gets sorted out. */ position: relative; -webkit-font-smoothing: antialiased; + background-color: {$page.background}; } .phui-crumbs-view, diff --git a/webroot/rsrc/css/phui/phui-document-pro.css b/webroot/rsrc/css/phui/phui-document-pro.css index 1cedd133f9..1027d03a56 100644 --- a/webroot/rsrc/css/phui/phui-document-pro.css +++ b/webroot/rsrc/css/phui/phui-document-pro.css @@ -37,7 +37,7 @@ } .device-phone .phui-document-view.phui-document-view-pro { - padding: 0 8px; + padding: 0 12px; margin: 0 auto; } @@ -132,6 +132,11 @@ a.button.phui-document-toc { color: #000; } +.device-phone .phui-document-view.phui-document-view-pro .phui-header-tall + .phui-header-header { + font-size: 18px; + } + .device-phone .phui-document-view-pro .phui-header-subheader { display: block; padding: 8px 0 0 0; diff --git a/webroot/rsrc/css/phui/phui-header-view.css b/webroot/rsrc/css/phui/phui-header-view.css index 2336ec3696..e773668f0a 100644 --- a/webroot/rsrc/css/phui/phui-header-view.css +++ b/webroot/rsrc/css/phui/phui-header-view.css @@ -323,6 +323,7 @@ body .phui-header-shell.phui-bleed-header .phui-profile-header.phui-header-shell .phui-header-header { font-family: 'Aleo', {$fontfamily}; font-size: 24px; + color: #000; } .phui-profile-header .phui-header-col3 { diff --git a/webroot/rsrc/css/phui/phui-profile-menu.css b/webroot/rsrc/css/phui/phui-profile-menu.css index 41efc4cfb3..275bff1dde 100644 --- a/webroot/rsrc/css/phui/phui-profile-menu.css +++ b/webroot/rsrc/css/phui/phui-profile-menu.css @@ -2,26 +2,6 @@ * @provides phui-profile-menu-css */ -.device-desktop .phui-navigation-shell.phui-profile-menu { - display: table; - width: 100%; - height: calc(100vh - {$menu.main.height}); -} - -.device-desktop .phui-profile-menu .phabricator-nav { - display: table-row; -} - -.device-desktop .phui-profile-menu .phabricator-nav-local { - display: table-cell; - position: relative; - vertical-align: top; - width: {$menu.profile.width}; - max-width: {$menu.profile.width}; - margin-top: 0; - overflow: hidden; -} - .device-desktop .phui-profile-menu-collapsed .phabricator-nav-local { width: {$menu.profile.width.collapsed}; max-width: {$menu.profile.width.collapsed}; @@ -32,9 +12,12 @@ margin-left: 0; } +.phui-profile-menu .phui-basic-nav { + width: 205px; +} + .phui-profile-menu .phabricator-side-menu { background: #525867; - box-shadow: inset -2px 0 2px rgba({$alphablack}, 0.150); width: 240px; } From b53d38315f3fd69ab239e7cbe8e40f1337fea496 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 1 Aug 2016 13:54:39 -0700 Subject: [PATCH 14/23] Fix Differental Filetree browser hotkey Summary: Adds a class for explicitly hiding the sidenav. Test Plan: Set Config to Enable Filetree. View a diff, see tree. Press `f`, see it go away. Reload page, see persistence. Reviewers: avivey, epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16359 --- resources/celerity/map.php | 32 +++++++++---------- src/view/layout/AphrontSideNavFilterView.php | 2 ++ .../rsrc/css/aphront/phabricator-nav-view.css | 4 +-- .../rsrc/js/core/behavior-phabricator-nav.js | 1 + 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index f5334ee6a5..83064c28a5 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,8 +7,8 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'a90e12a7', - 'core.pkg.js' => '13c7e56a', + 'core.pkg.css' => 'f3021640', + 'core.pkg.js' => 'b562c3db', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '3fb7f532', 'differential.pkg.js' => '634399e9', @@ -24,7 +24,7 @@ return array( 'rsrc/css/aphront/multi-column.css' => 'fd18389d', 'rsrc/css/aphront/notification.css' => '3f6c89c9', 'rsrc/css/aphront/panel-view.css' => '8427b78d', - 'rsrc/css/aphront/phabricator-nav-view.css' => '370da55a', + 'rsrc/css/aphront/phabricator-nav-view.css' => '9a498cb0', 'rsrc/css/aphront/table-view.css' => '832656fd', 'rsrc/css/aphront/tokenizer.css' => '056da01b', 'rsrc/css/aphront/tooltip.css' => '1a07aea8', @@ -499,7 +499,7 @@ return array( 'rsrc/js/core/behavior-more.js' => 'a80d0378', 'rsrc/js/core/behavior-object-selector.js' => 'e0ec7f2f', 'rsrc/js/core/behavior-oncopy.js' => '2926fff2', - 'rsrc/js/core/behavior-phabricator-nav.js' => '56a1ca03', + 'rsrc/js/core/behavior-phabricator-nav.js' => '08675c6d', 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '116cf19b', 'rsrc/js/core/behavior-read-only-warning.js' => 'ba158207', 'rsrc/js/core/behavior-refresh-csrf.js' => 'ab2f381b', @@ -660,7 +660,7 @@ return array( 'javelin-behavior-phabricator-keyboard-pager' => 'a8da01f0', 'javelin-behavior-phabricator-keyboard-shortcuts' => '01fca1f0', 'javelin-behavior-phabricator-line-linker' => '1499a8cb', - 'javelin-behavior-phabricator-nav' => '56a1ca03', + 'javelin-behavior-phabricator-nav' => '08675c6d', 'javelin-behavior-phabricator-notification-example' => '8ce821c5', 'javelin-behavior-phabricator-object-selector' => 'e0ec7f2f', 'javelin-behavior-phabricator-oncopy' => '2926fff2', @@ -786,7 +786,7 @@ return array( 'phabricator-keyboard-shortcut' => '1ae869f2', 'phabricator-keyboard-shortcut-manager' => '4a021c10', 'phabricator-main-menu-view' => 'b623169f', - 'phabricator-nav-view-css' => '370da55a', + 'phabricator-nav-view-css' => '9a498cb0', 'phabricator-notification' => 'ccf1cbf8', 'phabricator-notification-css' => '3f6c89c9', 'phabricator-notification-menu-css' => 'f31c0bde', @@ -982,6 +982,16 @@ return array( 'phabricator-prefab', 'phuix-icon-view', ), + '08675c6d' => array( + 'javelin-behavior', + 'javelin-behavior-device', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-magical-init', + 'javelin-vector', + 'javelin-request', + 'javelin-util', + ), '087e919c' => array( 'javelin-install', 'javelin-dom', @@ -1337,16 +1347,6 @@ return array( 'javelin-stratcom', 'javelin-vector', ), - '56a1ca03' => array( - 'javelin-behavior', - 'javelin-behavior-device', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-magical-init', - 'javelin-vector', - 'javelin-request', - 'javelin-util', - ), '58dea2fa' => array( 'javelin-install', 'javelin-util', diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index 8baa1f8499..eb9f4f72f8 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/src/view/layout/AphrontSideNavFilterView.php @@ -254,6 +254,8 @@ final class AphrontSideNavFilterView extends AphrontView { if ($this->flexible) { if (!$this->collapsed) { $nav_classes[] = 'has-drag-nav'; + } else { + $nav_classes[] = 'has-closed-nav'; } Javelin::initBehavior( diff --git a/webroot/rsrc/css/aphront/phabricator-nav-view.css b/webroot/rsrc/css/aphront/phabricator-nav-view.css index 1dac52b671..0e6cc67938 100644 --- a/webroot/rsrc/css/aphront/phabricator-nav-view.css +++ b/webroot/rsrc/css/aphront/phabricator-nav-view.css @@ -6,8 +6,8 @@ cursor: col-resize; } -.phabricator-nav-local, -.phabricator-nav-drag { +.device-desktop .has-closed-nav div.phabricator-nav-local, +.device-desktop .has-closed-nav div.phabricator-nav-drag { display: none; } diff --git a/webroot/rsrc/js/core/behavior-phabricator-nav.js b/webroot/rsrc/js/core/behavior-phabricator-nav.js index d148558b1e..ca7fc0d14a 100644 --- a/webroot/rsrc/js/core/behavior-phabricator-nav.js +++ b/webroot/rsrc/js/core/behavior-phabricator-nav.js @@ -108,6 +108,7 @@ JX.behavior('phabricator-nav', function(config) { collapsed = !collapsed; JX.DOM.alterClass(main, 'has-local-nav', !collapsed); JX.DOM.alterClass(main, 'has-drag-nav', !collapsed); + JX.DOM.alterClass(main, 'has-closed-nav', collapsed); resetdrag(); new JX.Request('/settings/adjust/', JX.bag) .setData({ key : 'nav-collapsed', value : (collapsed ? 1 : 0) }) From 0bb5dd88c87d9031656cb572298789dd6ffa430e Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 2 Aug 2016 11:22:53 -0700 Subject: [PATCH 15/23] Always pre-wrap code blocks Summary: Fixes T11416. Unclear what the side-effects of this would be, so bark if you find something. Previously, we'd have to overflow and scroll, which is kind of a pain since you're hiding content on long code blocks. This just wraps long lines, and preserves line breaks globally. Test Plan: Test feed, profile, comments, inline comments, triple backticks. Reviewers: avivey, epriestley Reviewed By: avivey, epriestley Subscribers: Korvin Maniphest Tasks: T11416 Differential Revision: https://secure.phabricator.com/D16361 --- resources/celerity/map.php | 10 +++++----- webroot/rsrc/css/application/project/project-view.css | 4 ---- webroot/rsrc/css/core/remarkup.css | 1 + 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 83064c28a5..59e56c83fa 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'f3021640', + 'core.pkg.css' => '6b69c820', 'core.pkg.js' => 'b562c3db', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '3fb7f532', @@ -94,7 +94,7 @@ return array( 'rsrc/css/application/policy/policy.css' => '957ea14c', 'rsrc/css/application/ponder/ponder-view.css' => 'fbd45f96', 'rsrc/css/application/project/project-card-view.css' => '9418c97d', - 'rsrc/css/application/project/project-view.css' => 'cbaa10a1', + 'rsrc/css/application/project/project-view.css' => '9ce99f21', 'rsrc/css/application/releeph/releeph-core.css' => '9b3c5733', 'rsrc/css/application/releeph/releeph-preview-branch.css' => 'b7a6f4a5', 'rsrc/css/application/releeph/releeph-request-differential-create-dialog.css' => '8d8b92cd', @@ -105,7 +105,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' => '523d34bb', + 'rsrc/css/core/remarkup.css' => '5ed06ed8', 'rsrc/css/core/syntax.css' => '769d3498', 'rsrc/css/core/z-index.css' => '5b6fcf3f', 'rsrc/css/diviner/diviner-shared.css' => 'aa3656aa', @@ -793,7 +793,7 @@ return array( 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', 'phabricator-prefab' => 'cfd23f37', - 'phabricator-remarkup-css' => '523d34bb', + 'phabricator-remarkup-css' => '5ed06ed8', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', @@ -884,7 +884,7 @@ return array( 'policy-transaction-detail-css' => '82100a43', 'ponder-view-css' => 'fbd45f96', 'project-card-view-css' => '9418c97d', - 'project-view-css' => 'cbaa10a1', + 'project-view-css' => '9ce99f21', 'releeph-core' => '9b3c5733', 'releeph-preview-branch' => 'b7a6f4a5', 'releeph-request-differential-create-dialog' => '8d8b92cd', diff --git a/webroot/rsrc/css/application/project/project-view.css b/webroot/rsrc/css/application/project/project-view.css index 0af3ef3490..2c0fd7d6d8 100644 --- a/webroot/rsrc/css/application/project/project-view.css +++ b/webroot/rsrc/css/application/project/project-view.css @@ -91,7 +91,3 @@ .profile-no-badges { padding: 24px 0; } - -.project-view-home .phabricator-remarkup .remarkup-code-block pre { - white-space: pre-wrap; -} diff --git a/webroot/rsrc/css/core/remarkup.css b/webroot/rsrc/css/core/remarkup.css index 034c5a83c6..42d1d86597 100644 --- a/webroot/rsrc/css/core/remarkup.css +++ b/webroot/rsrc/css/core/remarkup.css @@ -51,6 +51,7 @@ overflow: auto; padding: 12px; border-radius: 3px; + white-space: pre-wrap; } .phabricator-remarkup pre.remarkup-counterexample { From 24a28dd1f30a0f06b96b058ed3821a2d6a7ee566 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 2 Aug 2016 17:28:45 -0700 Subject: [PATCH 16/23] Fix a documentation typo ("repositorie") Summary: This isn't a word! Test Plan: Read carefully. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16362 --- src/docs/user/userguide/diffusion_api.diviner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/docs/user/userguide/diffusion_api.diviner b/src/docs/user/userguide/diffusion_api.diviner index 8ddd6d4bf6..f57d5007aa 100644 --- a/src/docs/user/userguide/diffusion_api.diviner +++ b/src/docs/user/userguide/diffusion_api.diviner @@ -14,7 +14,7 @@ For an introduction to Conduit, see @{article:Conduit API Overview}. In general, you'll use these API methods: - - `diffusion.repository.edit`: Create and edit repositorie. + - `diffusion.repository.edit`: Create and edit repositories. - `diffusion.uri.edit`: Create and edit repository URIs to configure observation, mirroring, and cloning. From 518479a916a8feeee2ac4d3df4d1fc06316104bb Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Wed, 3 Aug 2016 14:00:55 +0100 Subject: [PATCH 17/23] Fix broken link to PHPExcel site phpexcel.net currently serves a 500 page, and the top Google hit for PHPExcel (on codeplex) gives you a site warning you that it is 3 years out of date, and to see GitHub instead. Update the link from Maniphest's 'please install PHPExcel to enable export' prompt. --- .../maniphest/controller/ManiphestExportController.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/applications/maniphest/controller/ManiphestExportController.php b/src/applications/maniphest/controller/ManiphestExportController.php index 88e6849afb..1201c50478 100644 --- a/src/applications/maniphest/controller/ManiphestExportController.php +++ b/src/applications/maniphest/controller/ManiphestExportController.php @@ -31,7 +31,9 @@ final class ManiphestExportController extends ManiphestController { '

%s

'. '
'. '

'. - 'http://www.phpexcel.net/'. + ''. + 'https://github.com/PHPOffice/PHPExcel'. + ''. '

'. '
'. '

%s

', From 182a1462808856126e08b314c6bb77c6d4493bcd Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 3 Aug 2016 07:57:29 -0700 Subject: [PATCH 18/23] Set "history.immutable" to "false" explicitly in .arcconfig Summary: If contributors use "history.immutable" by default (for example, in user/global config) and then try to contribute to Phabricator, they might land in a way that creates merge commits, which we'd prefer to avoid. To make this a bit harder to do unintentionally, set "history.immutable" to "false" explicitly in `.arcconfig`. This will override any user or system setting and keep `land` in squash mode in this project. This might also be a problem in general, and maybe in the long term we need something like one of these warnings: > (when arc landing) You haven't landed changes to this project before, are you sure you want to create a merge commit / are you sure you want to squash? ...or: > (when doing anything) This project doesn't have a history.immutable setting; you should probably set one so user settings don't win out. ...or just not let this be set at the user/system level, although that's annoying in some cases. Also maybe this: > (when arc landing) You're about to create a merge commit, but the last Differential change in history squashed instead. Merge anyway? However, we haven't seen this as a widespread issue outside of this project yet (and even in this project I think it has only happened 2-3 times), so just put up a guard rail in our own configuration for now. Test Plan: - Ran `arc set-config history.immutable true` to set the flag in my user settings. - Ran `arc get-config` to verify that the setting overrode system/user settings: ``` epriestley@orbital ~/dev/phabricator $ arc get-config history.immutable --verbose history.immutable If true, arc will never change repository history (e.g., through amending or rebasing). Defaults to true in Mercurial and false in Git. This setting has no effect in Subversion. Example Value: false Current Value: false Current Source: project local Value: - project Value: false user Value: true system Value: - default Value: - ``` Reviewers: chad, fooishbar Reviewed By: fooishbar Differential Revision: https://secure.phabricator.com/D16364 --- .arcconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.arcconfig b/.arcconfig index f0fa152929..88f04f72c5 100644 --- a/.arcconfig +++ b/.arcconfig @@ -1,4 +1,5 @@ { "phabricator.uri": "https://secure.phabricator.com/", - "load": ["src/"] + "load": ["src/"], + "history.immutable": false } From 4d68c0ae044c596ab908f518b4c117fc5a9ae619 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 1 Aug 2016 13:29:46 -0700 Subject: [PATCH 19/23] Make Herald test workflow modular and more clear Summary: Fixes T9719. Currently, the Herald "Test Console" has a big `instanceof` thing, so new adapters (like a Calendar adapter, or third-party adapters) aren't available automatically. Instead, do a standard modular thing: load the available adapters, ask which ones can test the object the user selected, then let the user pick which one they want to move forward with. Additionally, it isn't very clear that you can't test "commit hook" rules because they rely on push state which we don't really have a good way to simulate. When the user picks a commit, we now show them the "Hook" events, but the options are disabled and explain why they can not be selected. Test Plan: - Ran test rules for revisions, commits, mocks, tasks, wiki documents, questions, and outbound mail. - Plugged in a commit, got a more-helpful choice screen explaining why you do a test run of hook rules. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9719 Differential Revision: https://secure.phabricator.com/D16360 --- .../HeraldDifferentialRevisionAdapter.php | 15 ++ .../diffusion/herald/HeraldCommitAdapter.php | 14 ++ .../herald/HeraldPreCommitAdapter.php | 14 ++ .../herald/adapter/HeraldAdapter.php | 18 +- .../HeraldTestConsoleController.php | 212 +++++++++++++----- .../herald/HeraldManiphestTaskAdapter.php | 15 ++ ...abricatorMailOutboundMailHeraldAdapter.php | 11 + .../pholio/herald/HeraldPholioMockAdapter.php | 15 ++ .../herald/PhrictionDocumentHeraldAdapter.php | 14 ++ .../herald/HeraldPonderQuestionAdapter.php | 15 ++ 10 files changed, 280 insertions(+), 63 deletions(-) diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php index d7b8c5dd33..2aed146cbf 100644 --- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php @@ -20,6 +20,21 @@ final class HeraldDifferentialRevisionAdapter return new DifferentialRevision(); } + public function isTestAdapterForObject($object) { + return ($object instanceof DifferentialRevision); + } + + public function getAdapterTestDescription() { + return pht( + 'Test rules which run when a revision is created or updated.'); + } + + public function newTestAdapter($object) { + return self::newLegacyAdapter( + $object, + $object->loadActiveDiff()); + } + protected function initializeNewAdapter() { $this->revision = $this->newObject(); } diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php index 67234edeab..538753f669 100644 --- a/src/applications/diffusion/herald/HeraldCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php @@ -27,10 +27,24 @@ final class HeraldCommitAdapter return new PhabricatorRepositoryCommit(); } + public function isTestAdapterForObject($object) { + return ($object instanceof PhabricatorRepositoryCommit); + } + + public function getAdapterTestDescription() { + return pht( + 'Test rules which run after a commit is discovered and imported.'); + } + protected function initializeNewAdapter() { $this->commit = $this->newObject(); } + public function setObject($object) { + $this->commit = $object; + return $this; + } + public function getObject() { return $this->commit; } diff --git a/src/applications/diffusion/herald/HeraldPreCommitAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitAdapter.php index 6b5b94b578..3f1bc6bfd0 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitAdapter.php @@ -25,6 +25,20 @@ abstract class HeraldPreCommitAdapter extends HeraldAdapter { return 'PhabricatorDiffusionApplication'; } + public function isTestAdapterForObject($object) { + return ($object instanceof PhabricatorRepositoryCommit); + } + + public function canCreateTestAdapterForObject($object) { + return false; + } + + public function getAdapterTestDescription() { + return pht( + 'Commit hook events depend on repository state which is only available '. + 'at push time, and can not be run in test mode.'); + } + protected function initializeNewAdapter() { $this->log = new PhabricatorRepositoryPushLog(); } diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index e08118a369..9362ce149b 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -189,7 +189,6 @@ abstract class HeraldAdapter extends Phobject { abstract public function getAdapterApplicationClass(); abstract public function getObject(); - /** * Return a new characteristic object for this adapter. * @@ -217,6 +216,23 @@ abstract class HeraldAdapter extends Phobject { return false; } + public function isTestAdapterForObject($object) { + return false; + } + + public function canCreateTestAdapterForObject($object) { + return $this->isTestAdapterForObject($object); + } + + public function newTestAdapter($object) { + return id(clone $this) + ->setObject($object); + } + + public function getAdapterTestDescription() { + return null; + } + public function explainValidTriggerObjects() { return pht('This adapter can not trigger on objects.'); } diff --git a/src/applications/herald/controller/HeraldTestConsoleController.php b/src/applications/herald/controller/HeraldTestConsoleController.php index 1dd6034bb5..fae92864f1 100644 --- a/src/applications/herald/controller/HeraldTestConsoleController.php +++ b/src/applications/herald/controller/HeraldTestConsoleController.php @@ -2,14 +2,76 @@ final class HeraldTestConsoleController extends HeraldController { + private $testObject; + private $testAdapter; + + public function setTestObject($test_object) { + $this->testObject = $test_object; + return $this; + } + + public function getTestObject() { + return $this->testObject; + } + + public function setTestAdapter(HeraldAdapter $test_adapter) { + $this->testAdapter = $test_adapter; + return $this; + } + + public function getTestAdapter() { + return $this->testAdapter; + } + public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); - $object_name = trim($request->getStr('object_name')); + + $response = $this->loadTestObject($request); + if ($response) { + return $response; + } + + $response = $this->loadAdapter($request); + if ($response) { + return $response; + } + + $object = $this->getTestObject(); + $adapter = $this->getTestAdapter(); + + $adapter->setIsNewObject(false); + + $rules = id(new HeraldRuleQuery()) + ->setViewer($viewer) + ->withContentTypes(array($adapter->getAdapterContentType())) + ->withDisabled(false) + ->needConditionsAndActions(true) + ->needAppliedToPHIDs(array($object->getPHID())) + ->needValidateAuthors(true) + ->execute(); + + $engine = id(new HeraldEngine()) + ->setDryRun(true); + + $effects = $engine->applyRules($rules, $adapter); + $engine->applyEffects($effects, $adapter, $rules); + + $xscript = $engine->getTranscript(); + + return id(new AphrontRedirectResponse()) + ->setURI('/herald/transcript/'.$xscript->getID().'/'); + } + + private function loadTestObject(AphrontRequest $request) { + $viewer = $this->getViewer(); $e_name = true; + $v_name = null; $errors = array(); + if ($request->isFormPost()) { - if (!$object_name) { + $v_name = trim($request->getStr('object_name')); + if (!$v_name) { $e_name = pht('Required'); $errors[] = pht('An object name is required.'); } @@ -17,66 +79,18 @@ final class HeraldTestConsoleController extends HeraldController { if (!$errors) { $object = id(new PhabricatorObjectQuery()) ->setViewer($viewer) - ->withNames(array($object_name)) + ->withNames(array($v_name)) ->executeOne(); if (!$object) { $e_name = pht('Invalid'); $errors[] = pht('No object exists with that name.'); } + } - if (!$errors) { - - // TODO: Let the adapters claim objects instead. - - if ($object instanceof DifferentialRevision) { - $adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter( - $object, - $object->loadActiveDiff()); - } else if ($object instanceof PhabricatorRepositoryCommit) { - $adapter = id(new HeraldCommitAdapter()) - ->setCommit($object); - } else if ($object instanceof ManiphestTask) { - $adapter = id(new HeraldManiphestTaskAdapter()) - ->setTask($object); - } else if ($object instanceof PholioMock) { - $adapter = id(new HeraldPholioMockAdapter()) - ->setMock($object); - } else if ($object instanceof PhrictionDocument) { - $adapter = id(new PhrictionDocumentHeraldAdapter()) - ->setDocument($object); - } else if ($object instanceof PonderQuestion) { - $adapter = id(new HeraldPonderQuestionAdapter()) - ->setQuestion($object); - } else if ($object instanceof PhabricatorMetaMTAMail) { - $adapter = id(new PhabricatorMailOutboundMailHeraldAdapter()) - ->setObject($object); - } else { - throw new Exception(pht('Can not build adapter for object!')); - } - - $adapter->setIsNewObject(false); - - $rules = id(new HeraldRuleQuery()) - ->setViewer($viewer) - ->withContentTypes(array($adapter->getAdapterContentType())) - ->withDisabled(false) - ->needConditionsAndActions(true) - ->needAppliedToPHIDs(array($object->getPHID())) - ->needValidateAuthors(true) - ->execute(); - - $engine = id(new HeraldEngine()) - ->setDryRun(true); - - $effects = $engine->applyRules($rules, $adapter); - $engine->applyEffects($effects, $adapter, $rules); - - $xscript = $engine->getTranscript(); - - return id(new AphrontRedirectResponse()) - ->setURI('/herald/transcript/'.$xscript->getID().'/'); - } + if (!$errors) { + $this->setTestObject($object); + return null; } } @@ -92,11 +106,89 @@ final class HeraldTestConsoleController extends HeraldController { ->setLabel(pht('Object Name')) ->setName('object_name') ->setError($e_name) - ->setValue($object_name)) + ->setValue($v_name)) ->appendChild( id(new AphrontFormSubmitControl()) - ->setValue(pht('Test Rules'))); + ->setValue(pht('Continue'))); + return $this->buildTestConsoleResponse($form, $errors); + } + + private function loadAdapter(AphrontRequest $request) { + $viewer = $this->getViewer(); + $object = $this->getTestObject(); + + $adapter_key = $request->getStr('adapter'); + + $adapters = HeraldAdapter::getAllAdapters(); + + $can_select = array(); + $display_adapters = array(); + foreach ($adapters as $key => $adapter) { + if (!$adapter->isTestAdapterForObject($object)) { + continue; + } + + if (!$adapter->isAvailableToUser($viewer)) { + continue; + } + + $display_adapters[$key] = $adapter; + + if ($adapter->canCreateTestAdapterForObject($object)) { + $can_select[$key] = $adapter; + } + } + + if ($request->isFormPost() && $adapter_key) { + if (isset($can_select[$adapter_key])) { + $adapter = $can_select[$adapter_key]->newTestAdapter($object); + $this->setTestAdapter($adapter); + return null; + } + } + + $form = id(new AphrontFormView()) + ->addHiddenInput('object_name', $request->getStr('object_name')) + ->setViewer($viewer); + + $cancel_uri = $this->getApplicationURI(); + + if (!$display_adapters) { + $form + ->appendRemarkupInstructions( + pht('//There are no available Herald events for this object.//')) + ->appendControl( + id(new AphrontFormSubmitControl()) + ->addCancelButton($cancel_uri)); + } else { + $adapter_control = id(new AphrontFormRadioButtonControl()) + ->setLabel(pht('Event')) + ->setName('adapter') + ->setValue(head_key($can_select)); + + foreach ($display_adapters as $adapter_key => $adapter) { + $is_disabled = empty($can_select[$adapter_key]); + + $adapter_control->addButton( + $adapter_key, + $adapter->getAdapterContentName(), + $adapter->getAdapterTestDescription(), + null, + $is_disabled); + } + + $form + ->appendControl($adapter_control) + ->appendControl( + id(new AphrontFormSubmitControl()) + ->setValue(pht('Run Test'))); + } + + return $this->buildTestConsoleResponse($form, array()); + } + + private function buildTestConsoleResponse($form, array $errors) { $box = id(new PHUIObjectBoxView()) ->setFormErrors($errors) ->setForm($form); @@ -118,11 +210,7 @@ final class HeraldTestConsoleController extends HeraldController { return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) - ->appendChild( - array( - $view, - )); - + ->appendChild($view); } } diff --git a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php index d504a71214..8503839e3e 100644 --- a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php +++ b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php @@ -16,6 +16,15 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { return pht('React to tasks being created or updated.'); } + public function isTestAdapterForObject($object) { + return ($object instanceof ManiphestTask); + } + + public function getAdapterTestDescription() { + return pht( + 'Test rules which run when a task is created or updated.'); + } + protected function initializeNewAdapter() { $this->task = $this->newObject(); } @@ -46,10 +55,16 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { $this->task = $task; return $this; } + public function getTask() { return $this->task; } + public function setObject($object) { + $this->task = $object; + return $this; + } + public function getObject() { return $this->task; } diff --git a/src/applications/metamta/herald/PhabricatorMailOutboundMailHeraldAdapter.php b/src/applications/metamta/herald/PhabricatorMailOutboundMailHeraldAdapter.php index 43c0a63300..6ca797833e 100644 --- a/src/applications/metamta/herald/PhabricatorMailOutboundMailHeraldAdapter.php +++ b/src/applications/metamta/herald/PhabricatorMailOutboundMailHeraldAdapter.php @@ -21,6 +21,17 @@ final class PhabricatorMailOutboundMailHeraldAdapter return new PhabricatorMetaMTAMail(); } + public function isTestAdapterForObject($object) { + return ($object instanceof PhabricatorMetaMTAMail); + } + + public function getAdapterTestDescription() { + return pht( + 'Test rules which run when outbound mail is being prepared for '. + 'delivery.'); + } + + public function getObject() { return $this->mail; } diff --git a/src/applications/pholio/herald/HeraldPholioMockAdapter.php b/src/applications/pholio/herald/HeraldPholioMockAdapter.php index 57fa74c3a1..5520a60cdc 100644 --- a/src/applications/pholio/herald/HeraldPholioMockAdapter.php +++ b/src/applications/pholio/herald/HeraldPholioMockAdapter.php @@ -20,6 +20,20 @@ final class HeraldPholioMockAdapter extends HeraldAdapter { return new PholioMock(); } + public function isTestAdapterForObject($object) { + return ($object instanceof PholioMock); + } + + public function getAdapterTestDescription() { + return pht( + 'Test rules which run when a mock is created or updated.'); + } + + public function setObject($object) { + $this->mock = $object; + return $this; + } + public function getObject() { return $this->mock; } @@ -28,6 +42,7 @@ final class HeraldPholioMockAdapter extends HeraldAdapter { $this->mock = $mock; return $this; } + public function getMock() { return $this->mock; } diff --git a/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php b/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php index 575d4b3dd8..74de97d5fc 100644 --- a/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php +++ b/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php @@ -20,6 +20,20 @@ final class PhrictionDocumentHeraldAdapter extends HeraldAdapter { return new PhrictionDocument(); } + public function isTestAdapterForObject($object) { + return ($object instanceof PhrictionDocument); + } + + public function getAdapterTestDescription() { + return pht( + 'Test rules which run when a wiki document is created or updated.'); + } + + public function setObject($object) { + $this->document = $object; + return $this; + } + public function getObject() { return $this->document; } diff --git a/src/applications/ponder/herald/HeraldPonderQuestionAdapter.php b/src/applications/ponder/herald/HeraldPonderQuestionAdapter.php index 3a21457c3e..f3f47681bb 100644 --- a/src/applications/ponder/herald/HeraldPonderQuestionAdapter.php +++ b/src/applications/ponder/herald/HeraldPonderQuestionAdapter.php @@ -20,6 +20,21 @@ final class HeraldPonderQuestionAdapter extends HeraldAdapter { $this->question = $this->newObject(); } + + public function isTestAdapterForObject($object) { + return ($object instanceof PonderQuestion); + } + + public function getAdapterTestDescription() { + return pht( + 'Test rules which run when a question is created or updated.'); + } + + public function setObject($object) { + $this->question = $object; + return $this; + } + public function supportsApplicationEmail() { return true; } From 8f4a63d7083d07836e6851f2aeb238bda5141c98 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 3 Aug 2016 16:14:17 -0700 Subject: [PATCH 20/23] Use consistent tag order in Maniphest list views and workboard cards Summary: Fixes T11420. These are selected in newest-to-oldest order from the database, but we should show them in oldest-to-newest order in the UI. Test Plan: Tagged a couple tasks with "A, B, C" projects, saw correct order in UI: {F1749351} {F1749352} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11420 Differential Revision: https://secure.phabricator.com/D16367 --- src/applications/maniphest/view/ManiphestTaskListView.php | 2 +- src/applications/project/view/ProjectBoardTaskCard.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/applications/maniphest/view/ManiphestTaskListView.php b/src/applications/maniphest/view/ManiphestTaskListView.php index 5c01a2d9fb..6f714d0c5c 100644 --- a/src/applications/maniphest/view/ManiphestTaskListView.php +++ b/src/applications/maniphest/view/ManiphestTaskListView.php @@ -96,7 +96,7 @@ final class ManiphestTaskListView extends ManiphestView { $project_handles = array_select_keys( $handles, - $task->getProjectPHIDs()); + array_reverse($task->getProjectPHIDs())); $item->addAttribute( id(new PHUIHandleTagListView()) diff --git a/src/applications/project/view/ProjectBoardTaskCard.php b/src/applications/project/view/ProjectBoardTaskCard.php index a7ee93dd76..ba5213a623 100644 --- a/src/applications/project/view/ProjectBoardTaskCard.php +++ b/src/applications/project/view/ProjectBoardTaskCard.php @@ -128,6 +128,7 @@ final class ProjectBoardTaskCard extends Phobject { } if ($project_handles) { + $project_handles = array_reverse($project_handles); $tag_list = id(new PHUIHandleTagListView()) ->setSlim(true) ->setHandles($project_handles); From 1dd9c37fdf207c2911cc530369556168658ed06f Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 4 Aug 2016 20:49:16 +0000 Subject: [PATCH 21/23] Clean up some tablet issues with new nav layouts Summary: Makes sidenav disappear again on projects/profiles, but shows it on home again (tablet views). Test Plan: Visit Profile/Projects/Home on mobile, desktop, and tablet. See nav disappear correctly. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16369 --- resources/celerity/map.php | 10 +++++----- .../home/controller/PhabricatorHomeMainController.php | 1 + webroot/rsrc/css/aphront/phabricator-nav-view.css | 6 ++++-- webroot/rsrc/css/phui/phui-basic-nav-view.css | 9 ++++++--- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 59e56c83fa..7d846e4cec 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => '6b69c820', + 'core.pkg.css' => '90c46327', 'core.pkg.js' => 'b562c3db', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '3fb7f532', @@ -24,7 +24,7 @@ return array( 'rsrc/css/aphront/multi-column.css' => 'fd18389d', 'rsrc/css/aphront/notification.css' => '3f6c89c9', 'rsrc/css/aphront/panel-view.css' => '8427b78d', - 'rsrc/css/aphront/phabricator-nav-view.css' => '9a498cb0', + 'rsrc/css/aphront/phabricator-nav-view.css' => '09f3d0db', 'rsrc/css/aphront/table-view.css' => '832656fd', 'rsrc/css/aphront/tokenizer.css' => '056da01b', 'rsrc/css/aphront/tooltip.css' => '1a07aea8', @@ -122,7 +122,7 @@ return array( 'rsrc/css/phui/phui-action-list.css' => 'c5eba19d', 'rsrc/css/phui/phui-action-panel.css' => '91c7b835', 'rsrc/css/phui/phui-badge.css' => '3baef8db', - 'rsrc/css/phui/phui-basic-nav-view.css' => 'bfc71dd0', + 'rsrc/css/phui/phui-basic-nav-view.css' => '7093573b', 'rsrc/css/phui/phui-big-info-view.css' => 'bd903741', 'rsrc/css/phui/phui-box.css' => '5c8387cf', 'rsrc/css/phui/phui-button.css' => '4a5fbe3d', @@ -786,7 +786,7 @@ return array( 'phabricator-keyboard-shortcut' => '1ae869f2', 'phabricator-keyboard-shortcut-manager' => '4a021c10', 'phabricator-main-menu-view' => 'b623169f', - 'phabricator-nav-view-css' => '9a498cb0', + 'phabricator-nav-view-css' => '09f3d0db', 'phabricator-notification' => 'ccf1cbf8', 'phabricator-notification-css' => '3f6c89c9', 'phabricator-notification-menu-css' => 'f31c0bde', @@ -825,7 +825,7 @@ return array( 'phriction-document-css' => '4282e4ad', 'phui-action-panel-css' => '91c7b835', 'phui-badge-view-css' => '3baef8db', - 'phui-basic-nav-view-css' => 'bfc71dd0', + 'phui-basic-nav-view-css' => '7093573b', 'phui-big-info-view-css' => 'bd903741', 'phui-box-css' => '5c8387cf', 'phui-button-css' => '4a5fbe3d', diff --git a/src/applications/home/controller/PhabricatorHomeMainController.php b/src/applications/home/controller/PhabricatorHomeMainController.php index c5387a5fb7..7bc8b06573 100644 --- a/src/applications/home/controller/PhabricatorHomeMainController.php +++ b/src/applications/home/controller/PhabricatorHomeMainController.php @@ -53,6 +53,7 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController { return $this->newPage() ->setTitle('Phabricator') + ->addClass('phabricator-home') ->appendChild($content); } diff --git a/webroot/rsrc/css/aphront/phabricator-nav-view.css b/webroot/rsrc/css/aphront/phabricator-nav-view.css index 0e6cc67938..760c2cd372 100644 --- a/webroot/rsrc/css/aphront/phabricator-nav-view.css +++ b/webroot/rsrc/css/aphront/phabricator-nav-view.css @@ -7,7 +7,9 @@ } .device-desktop .has-closed-nav div.phabricator-nav-local, -.device-desktop .has-closed-nav div.phabricator-nav-drag { +.device-desktop .has-closed-nav div.phabricator-nav-drag, +.device .phui-navigation-shell div.phabricator-nav-local, +.device .phui-navigation-shell div.phabricator-nav-drag { display: none; } @@ -16,7 +18,7 @@ display: block; } -.device .phabricator-side-menu-home .phabricator-nav-local { +.device-phone .phabricator-side-menu-home .phabricator-nav-local { display: block; } diff --git a/webroot/rsrc/css/phui/phui-basic-nav-view.css b/webroot/rsrc/css/phui/phui-basic-nav-view.css index f421579d70..30647e3ece 100644 --- a/webroot/rsrc/css/phui/phui-basic-nav-view.css +++ b/webroot/rsrc/css/phui/phui-basic-nav-view.css @@ -2,17 +2,20 @@ * @provides phui-basic-nav-view-css */ -.device-desktop .phui-navigation-shell { +.device-desktop .phui-navigation-shell, +.phabricator-home.device .phui-navigation-shell { display: table; width: 100%; height: calc(100vh - {$menu.main.height}); } -.device-desktop .phui-navigation-shell .phabricator-nav { +.device-desktop .phui-navigation-shell .phabricator-nav, +.phabricator-home.device .phui-navigation-shell .phabricator-nav { display: table-row; } -.device-desktop .phui-navigation-shell .phabricator-nav-local { +.device-desktop .phui-navigation-shell .phabricator-nav-local, +.phabricator-home.device .phui-navigation-shell .phabricator-nav-local { display: table-cell; position: relative; vertical-align: top; From 0cb9ca5500d68c8c4ced5fe85df2ce9d5acf5467 Mon Sep 17 00:00:00 2001 From: Evaldas Alexander Date: Sat, 6 Aug 2016 06:25:55 -0700 Subject: [PATCH 22/23] Explain reasoning behind kitty gifts Summary: I find this fact very useful for understanding my feline companion Test Plan: Added "Motivator: Cat Facts" to the project, nothing broke Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D16374 --- .../search/profilepanel/PhabricatorMotivatorProfilePanel.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php b/src/applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php index 34c767e99c..327290deba 100644 --- a/src/applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php +++ b/src/applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php @@ -140,7 +140,10 @@ final class PhabricatorMotivatorProfilePanel 'to launch their attacks. Watch out!'), pht('Cats prefer vanilla ice cream.'), pht('Taco cat spelled backwards is taco cat.'), - ); + pht( + 'Cats will often bring you their prey because they feel sorry '. + 'for your inability to hunt.'), + ); } private function selectFact(array $facts) { From 87f663ef77b4120debb1c1de48d24650776d6a60 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 3 Aug 2016 16:40:24 -0700 Subject: [PATCH 23/23] Add basic Herald support to Calendar Summary: Fixes T7939. This doesn't get too fancy, but allows you to write Herald rules against Calendar events. Test Plan: - Wrote an "add red flag to events with party in the name" rule. - Created a "mundane meeting", didn't get flagged. - Created a "cool party", got flagged. - Ran rules from the Herald test console. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7939 Differential Revision: https://secure.phabricator.com/D16368 --- src/__phutil_library_map__.php | 8 +++ .../editor/PhabricatorCalendarEventEditor.php | 14 +++++ .../PhabricatorCalendarEventHeraldAdapter.php | 56 +++++++++++++++++++ .../PhabricatorCalendarEventHeraldField.php | 13 +++++ ...abricatorCalendarEventHeraldFieldGroup.php | 16 ++++++ ...habricatorCalendarEventNameHeraldField.php | 20 +++++++ .../herald/adapter/HeraldAdapter.php | 5 +- 7 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 src/applications/calendar/herald/PhabricatorCalendarEventHeraldAdapter.php create mode 100644 src/applications/calendar/herald/PhabricatorCalendarEventHeraldField.php create mode 100644 src/applications/calendar/herald/PhabricatorCalendarEventHeraldFieldGroup.php create mode 100644 src/applications/calendar/herald/PhabricatorCalendarEventNameHeraldField.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e29c91ae0e..c5059723a5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2044,6 +2044,9 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventEndDateTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php', 'PhabricatorCalendarEventFrequencyTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventFrequencyTransaction.php', 'PhabricatorCalendarEventFulltextEngine' => 'applications/calendar/search/PhabricatorCalendarEventFulltextEngine.php', + 'PhabricatorCalendarEventHeraldAdapter' => 'applications/calendar/herald/PhabricatorCalendarEventHeraldAdapter.php', + 'PhabricatorCalendarEventHeraldField' => 'applications/calendar/herald/PhabricatorCalendarEventHeraldField.php', + 'PhabricatorCalendarEventHeraldFieldGroup' => 'applications/calendar/herald/PhabricatorCalendarEventHeraldFieldGroup.php', 'PhabricatorCalendarEventHostPolicyRule' => 'applications/calendar/policyrule/PhabricatorCalendarEventHostPolicyRule.php', 'PhabricatorCalendarEventHostTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventHostTransaction.php', 'PhabricatorCalendarEventIconTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php', @@ -2054,6 +2057,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventJoinController' => 'applications/calendar/controller/PhabricatorCalendarEventJoinController.php', 'PhabricatorCalendarEventListController' => 'applications/calendar/controller/PhabricatorCalendarEventListController.php', 'PhabricatorCalendarEventMailReceiver' => 'applications/calendar/mail/PhabricatorCalendarEventMailReceiver.php', + 'PhabricatorCalendarEventNameHeraldField' => 'applications/calendar/herald/PhabricatorCalendarEventNameHeraldField.php', 'PhabricatorCalendarEventNameTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventNameTransaction.php', 'PhabricatorCalendarEventPHIDType' => 'applications/calendar/phid/PhabricatorCalendarEventPHIDType.php', 'PhabricatorCalendarEventQuery' => 'applications/calendar/query/PhabricatorCalendarEventQuery.php', @@ -6757,6 +6761,9 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventEndDateTransaction' => 'PhabricatorCalendarEventDateTransaction', 'PhabricatorCalendarEventFrequencyTransaction' => 'PhabricatorCalendarEventTransactionType', 'PhabricatorCalendarEventFulltextEngine' => 'PhabricatorFulltextEngine', + 'PhabricatorCalendarEventHeraldAdapter' => 'HeraldAdapter', + 'PhabricatorCalendarEventHeraldField' => 'HeraldField', + 'PhabricatorCalendarEventHeraldFieldGroup' => 'HeraldFieldGroup', 'PhabricatorCalendarEventHostPolicyRule' => 'PhabricatorPolicyRule', 'PhabricatorCalendarEventHostTransaction' => 'PhabricatorCalendarEventTransactionType', 'PhabricatorCalendarEventIconTransaction' => 'PhabricatorCalendarEventTransactionType', @@ -6770,6 +6777,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventJoinController' => 'PhabricatorCalendarController', 'PhabricatorCalendarEventListController' => 'PhabricatorCalendarController', 'PhabricatorCalendarEventMailReceiver' => 'PhabricatorObjectMailReceiver', + 'PhabricatorCalendarEventNameHeraldField' => 'PhabricatorCalendarEventHeraldField', 'PhabricatorCalendarEventNameTransaction' => 'PhabricatorCalendarEventTransactionType', 'PhabricatorCalendarEventPHIDType' => 'PhabricatorPHIDType', 'PhabricatorCalendarEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php index 2b457418d0..5498efadd3 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php @@ -289,5 +289,19 @@ final class PhabricatorCalendarEventEditor return $body; } + protected function shouldApplyHeraldRules( + PhabricatorLiskDAO $object, + array $xactions) { + return true; + } + + protected function buildHeraldAdapter( + PhabricatorLiskDAO $object, + array $xactions) { + + return id(new PhabricatorCalendarEventHeraldAdapter()) + ->setObject($object); + } + } diff --git a/src/applications/calendar/herald/PhabricatorCalendarEventHeraldAdapter.php b/src/applications/calendar/herald/PhabricatorCalendarEventHeraldAdapter.php new file mode 100644 index 0000000000..49d5f38959 --- /dev/null +++ b/src/applications/calendar/herald/PhabricatorCalendarEventHeraldAdapter.php @@ -0,0 +1,56 @@ +object = $object; + return $this; + } + + public function getObject() { + return $this->object; + } + + public function getAdapterContentName() { + return pht('Calendar Events'); + } + + public function supportsRuleType($rule_type) { + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + return true; + case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: + default: + return false; + } + } + + public function getHeraldName() { + return $this->getObject()->getMonogram(); + } + +} diff --git a/src/applications/calendar/herald/PhabricatorCalendarEventHeraldField.php b/src/applications/calendar/herald/PhabricatorCalendarEventHeraldField.php new file mode 100644 index 0000000000..7d9d32a5de --- /dev/null +++ b/src/applications/calendar/herald/PhabricatorCalendarEventHeraldField.php @@ -0,0 +1,13 @@ +getName(); + } + + protected function getHeraldFieldStandardType() { + return self::STANDARD_TEXT; + } + +} diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 9362ce149b..ac227819f9 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -768,7 +768,10 @@ abstract class HeraldAdapter extends Phobject { ); } - abstract protected function initializeNewAdapter(); + protected function initializeNewAdapter() { + $this->setObject($this->newObject()); + return $this; + } /** * Does this adapter's event fire only once?