From d5f9e49e29c12dd16ba7a005f188906aaff9eb84 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 19 Mar 2016 15:34:31 +0000 Subject: [PATCH 01/31] Use PHUIStatusListView in Diffusion commit list Summary: Fixes T10626. Adds proper wrapper Test Plan: Review spacing on a commit with comitted in the property list. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T10626 Differential Revision: https://secure.phabricator.com/D15498 --- .../diffusion/controller/DiffusionCommitController.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index c6ac32bf7f..890c512870 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -496,9 +496,11 @@ final class DiffusionCommitController extends DiffusionController { $committed_info->setTarget($author_name); } + $committed_list = new PHUIStatusListView(); + $committed_list->addItem($committed_info); $view->addProperty( pht('Committed'), - $committed_info); + $committed_list); if ($push_logs) { $pushed_list = new PHUIStatusListView(); From 76f07ec80bde6a7663120289f000b8a0a6cbba83 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 19 Mar 2016 10:11:32 -0700 Subject: [PATCH 02/31] Only require view permissions for read-only Git LFS requests Summary: Ref T7789. Implement proper detection for read-only requests. Previously, we assumed every request was read/write and required lots of permissions, but we don't need "Can Push" permission if you're only cloning/fetching/pulling. Test Plan: - Set push policy to "no one". - Fetched, got clean data out of LFS. - Tried to push, got useful error. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7789 Differential Revision: https://secure.phabricator.com/D15499 --- .../controller/DiffusionServeController.php | 63 ++++++++++++++++--- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index 31bcb2cbed..80fb224163 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -7,6 +7,7 @@ final class DiffusionServeController extends DiffusionController { private $isGitLFSRequest; private $gitLFSToken; + private $gitLFSInput; public function setServiceViewer(PhabricatorUser $viewer) { $this->getRequest()->setUser($viewer); @@ -284,11 +285,20 @@ final class DiffusionServeController extends DiffusionController { DiffusionPushCapability::CAPABILITY); if (!$can_push) { if ($viewer->isLoggedIn()) { - return new PhabricatorVCSResponse( - 403, - pht( - 'You do not have permission to push to this '. - 'repository.')); + $error_code = 403; + $error_message = pht( + 'You do not have permission to push to this repository ("%s").', + $repository->getDisplayName()); + + if ($this->getIsGitLFSRequest()) { + return DiffusionGitLFSResponse::newErrorResponse( + $error_code, + $error_message); + } else { + return new PhabricatorVCSResponse( + $error_code, + $error_message); + } } else { if ($allow_auth) { return new PhabricatorVCSResponse( @@ -422,7 +432,9 @@ final class DiffusionServeController extends DiffusionController { // TODO: This implementation is safe by default, but very incomplete. - // TODO: This doesn't get the right result for Git LFS yet. + if ($this->getIsGitLFSRequest()) { + return $this->isGitLFSReadOnlyRequest($repository); + } switch ($repository->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: @@ -911,8 +923,7 @@ final class DiffusionServeController extends DiffusionController { PhabricatorRepository $repository, PhabricatorUser $viewer) { - $input = PhabricatorStartup::getRawInput(); - $input = phutil_json_decode($input); + $input = $this->getGitLFSInput(); $operation = idx($input, 'operation'); switch ($operation) { @@ -1061,6 +1072,10 @@ final class DiffusionServeController extends DiffusionController { $oid)); } + // Remove the execution time limit because uploading large files may take + // a while. + set_time_limit(0); + $request_stream = new AphrontRequestStream(); $request_iterator = $request_stream->getIterator(); $hashing_iterator = id(new PhutilHashingIterator($request_iterator)) @@ -1133,4 +1148,36 @@ final class DiffusionServeController extends DiffusionController { return null; } + private function getGitLFSInput() { + if (!$this->gitLFSInput) { + $input = PhabricatorStartup::getRawInput(); + $input = phutil_json_decode($input); + $this->gitLFSInput = $input; + } + + return $this->gitLFSInput; + } + + private function isGitLFSReadOnlyRequest(PhabricatorRepository $repository) { + if (!$this->getIsGitLFSRequest()) { + return false; + } + + $path = $this->getGitLFSRequestPath($repository); + + if ($path === 'objects/batch') { + $input = $this->getGitLFSInput(); + $operation = idx($input, 'operation'); + switch ($operation) { + case 'download': + return true; + default: + return false; + } + } + + return false; + } + + } From 130e1d1f68a4dc4c41559be11971301cbb656317 Mon Sep 17 00:00:00 2001 From: Vlad Albulescu Date: Sun, 20 Mar 2016 10:15:21 -0700 Subject: [PATCH 03/31] Unbreak regex filename search Summary: D9087 adds a nice typeahead but breaks the existing regex search by quoting the pattern. Ideally, this change won't break the typeahead, which as far as I can tell doesn't use the `pattern` argument. Test Plan: Not yet. RFC as to whether this change makes sense, will fix my local setup and resend if so. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D15500 --- .../diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php index 5ca2783838..be2f07f2c6 100644 --- a/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php @@ -79,7 +79,8 @@ final class DiffusionQueryPathsConduitAPIMethod $offset = (int)$request->getValue('offset'); if (strlen($pattern)) { - $pattern = '/'.preg_quote($pattern, '/').'/'; + // Add delimiters to the regex pattern. + $pattern = '('.$pattern.')'; } $results = array(); From 63ab2ad69ba38270de64ac971e6ead7dd1557d3b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 21 Mar 2016 10:13:06 -0700 Subject: [PATCH 04/31] Typo fixed in docs/tech Summary: Spelling mistake fixed - neessary > necessary Test Plan: No Test plan Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D15146 --- src/docs/tech/celerity.diviner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/docs/tech/celerity.diviner b/src/docs/tech/celerity.diviner index e1b744645b..8501b1b536 100644 --- a/src/docs/tech/celerity.diviner +++ b/src/docs/tech/celerity.diviner @@ -51,7 +51,7 @@ generate fewer resource requests, improving performance). It then generates These references point at `/res/` URIs, which are handled by @{class:CelerityResourceController}. It responds to these requests and delivers the relevant resources and packages, managing cache lifetimes and handling any -neessary preprocessing. It uses @{class:CelerityResourceMap} to locate resources +necessary preprocessing. It uses @{class:CelerityResourceMap} to locate resources and read packaging rules. The dependency and packaging maps are generated by `bin/celerity map`, From 66946c09968c24f454e271e9506c8c4dea5f05e6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 21 Mar 2016 11:19:48 -0700 Subject: [PATCH 05/31] Fix unusual use of Remarkup in Maniphest Summary: Fixes T10234. This usage is unusual, out of date, and has some bad interactions with engines and custom rules. Test Plan: - Added `CustomInlineCodeRule` from P1129 as an extension rule. - Put a custom ` ... ` block in a Maniphest task description. - Saw fatal as described in task; applied change; saw rule work properly. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10234 Differential Revision: https://secure.phabricator.com/D15501 --- .../ManiphestTaskDetailController.php | 22 ++++++++----------- .../markup/view/PHUIRemarkupView.php | 14 +++++++++++- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index ceda169489..285f919ce8 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -59,15 +59,9 @@ final class ManiphestTaskDetailController extends ManiphestController { $phids = array_keys($phids); $handles = $viewer->loadHandles($phids); - $engine = id(new PhabricatorMarkupEngine()) - ->setViewer($viewer) - ->setContextObject($task) - ->addObject($task, ManiphestTask::MARKUP_FIELD_DESCRIPTION); - $timeline = $this->buildTransactionTimeline( $task, - new ManiphestTransactionQuery(), - $engine); + new ManiphestTransactionQuery()); $monogram = $task->getMonogram(); $crumbs = $this->buildApplicationCrumbs() @@ -76,7 +70,7 @@ final class ManiphestTaskDetailController extends ManiphestController { $header = $this->buildHeaderView($task); $details = $this->buildPropertyView($task, $field_list, $edges, $handles); - $description = $this->buildDescriptionView($task, $engine); + $description = $this->buildDescriptionView($task); $curtain = $this->buildCurtain($task, $edit_engine); $title = pht('%s %s', $monogram, $task->getTitle()); @@ -346,12 +340,13 @@ final class ManiphestTaskDetailController extends ManiphestController { return null; } - private function buildDescriptionView( - ManiphestTask $task, - PhabricatorMarkupEngine $engine) { + private function buildDescriptionView(ManiphestTask $task) { + $viewer = $this->getViewer(); $section = null; - if (strlen($task->getDescription())) { + + $description = $task->getDescription(); + if (strlen($description)) { $section = new PHUIPropertyListView(); $section->addTextContent( phutil_tag( @@ -359,7 +354,8 @@ final class ManiphestTaskDetailController extends ManiphestController { array( 'class' => 'phabricator-remarkup', ), - $engine->getOutput($task, ManiphestTask::MARKUP_FIELD_DESCRIPTION))); + id(new PHUIRemarkupView($viewer, $description)) + ->setContextObject($task))); } return $section; diff --git a/src/infrastructure/markup/view/PHUIRemarkupView.php b/src/infrastructure/markup/view/PHUIRemarkupView.php index 8a8d9ddf7f..0a07e64a87 100644 --- a/src/infrastructure/markup/view/PHUIRemarkupView.php +++ b/src/infrastructure/markup/view/PHUIRemarkupView.php @@ -13,6 +13,7 @@ final class PHUIRemarkupView extends AphrontView { private $corpus; private $markupType; + private $contextObject; const DOCUMENT = 'document'; @@ -26,16 +27,27 @@ final class PHUIRemarkupView extends AphrontView { return $this; } + public function setContextObject($context_object) { + $this->contextObject = $context_object; + return $this; + } + + public function getContextObject() { + return $this->contextObject; + } + public function render() { $viewer = $this->getUser(); $corpus = $this->corpus; + $context = $this->getContextObject(); $content = PhabricatorMarkupEngine::renderOneObject( id(new PhabricatorMarkupOneOff()) ->setPreserveLinebreaks(true) ->setContent($corpus), 'default', - $viewer); + $viewer, + $context); if ($this->markupType == self::DOCUMENT) { return phutil_tag( From 77368689962aac6fec3fd621a428f3867ac754eb Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 21 Mar 2016 12:56:53 -0700 Subject: [PATCH 06/31] Convert Spaces to two column Summary: Updates Spaces to new two column layout Test Plan: Create a space, edit a space Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D15503 --- .../PhabricatorSpacesEditController.php | 26 +++++++---- .../PhabricatorSpacesViewController.php | 46 +++++++++++-------- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/applications/spaces/controller/PhabricatorSpacesEditController.php b/src/applications/spaces/controller/PhabricatorSpacesEditController.php index 4dfeeed7ed..3f7dae9e82 100644 --- a/src/applications/spaces/controller/PhabricatorSpacesEditController.php +++ b/src/applications/spaces/controller/PhabricatorSpacesEditController.php @@ -162,7 +162,8 @@ final class PhabricatorSpacesEditController ->addCancelButton($cancel_uri)); $box = id(new PHUIObjectBoxView()) - ->setHeaderText($header_text) + ->setHeaderText(pht('Space')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setValidationException($validation_exception) ->appendChild($form); @@ -173,14 +174,21 @@ final class PhabricatorSpacesEditController $cancel_uri); } $crumbs->addTextCrumb($title); + $crumbs->setBorder(true); - return $this->buildApplicationPage( - array( - $crumbs, - $box, - ), - array( - 'title' => $title, - )); + $header = id(new PHUIHeaderView()) + ->setHeader($header_text) + ->setHeaderIcon('fa-pencil'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( + $box, + )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); } } diff --git a/src/applications/spaces/controller/PhabricatorSpacesViewController.php b/src/applications/spaces/controller/PhabricatorSpacesViewController.php index 25bb70f646..8319f19a6e 100644 --- a/src/applications/spaces/controller/PhabricatorSpacesViewController.php +++ b/src/applications/spaces/controller/PhabricatorSpacesViewController.php @@ -18,9 +18,9 @@ final class PhabricatorSpacesViewController return new Aphront404Response(); } - $action_list = $this->buildActionListView($space); + $curtain = $this->buildCurtain($space); $property_list = $this->buildPropertyListView($space); - $property_list->setActionList($action_list); + $title = array($space->getMonogram(), $space->getNamespaceName()); $xactions = id(new PhabricatorSpacesNamespaceTransactionQuery()) ->setViewer($viewer) @@ -35,7 +35,8 @@ final class PhabricatorSpacesViewController $header = id(new PHUIHeaderView()) ->setUser($viewer) ->setHeader($space->getNamespaceName()) - ->setPolicyObject($space); + ->setPolicyObject($space) + ->setHeaderIcon('fa-th-large'); if ($space->getIsArchived()) { $header->setStatus('fa-ban', 'red', pht('Archived')); @@ -44,21 +45,27 @@ final class PhabricatorSpacesViewController } $box = id(new PHUIObjectBoxView()) - ->setHeader($header) + ->setHeaderText(pht('DETAILS')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($property_list); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($space->getMonogram()); + $crumbs->setBorder(true); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setMainColumn(array( + $box, + $timeline, + )) + ->setCurtain($curtain); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); - return $this->buildApplicationPage( - array( - $crumbs, - $box, - $timeline, - ), - array( - 'title' => array($space->getMonogram(), $space->getNamespaceName()), - )); } private function buildPropertyListView(PhabricatorSpacesNamespace $space) { @@ -93,18 +100,17 @@ final class PhabricatorSpacesViewController return $list; } - private function buildActionListView(PhabricatorSpacesNamespace $space) { + private function buildCurtain(PhabricatorSpacesNamespace $space) { $viewer = $this->getRequest()->getUser(); - $list = id(new PhabricatorActionListView()) - ->setUser($viewer); + $curtain = $this->newCurtainView($space); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $space, PhabricatorPolicyCapability::CAN_EDIT); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Space')) ->setIcon('fa-pencil') @@ -115,7 +121,7 @@ final class PhabricatorSpacesViewController $id = $space->getID(); if ($space->getIsArchived()) { - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Activate Space')) ->setIcon('fa-check') @@ -123,7 +129,7 @@ final class PhabricatorSpacesViewController ->setDisabled(!$can_edit) ->setWorkflow(true)); } else { - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Archive Space')) ->setIcon('fa-ban') @@ -132,7 +138,7 @@ final class PhabricatorSpacesViewController ->setWorkflow(true)); } - return $list; + return $curtain; } } From 5a604538ca4020c78548eab6d405ec2ffe384961 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Mar 2016 09:08:26 -0700 Subject: [PATCH 07/31] Fix an initialization issue in Herald rules in Chrome Summary: Fixes T10646. When you load the page or click "New Condition" or "New Action", we try to add a condition and action with some default values. Currently, the logic just sets everything to `null` or `'default'`. This technically works in Safari, but is less successful in Chrome. (I think Safari prevents you from picking an invalid value.) Instead of relying on the browser to pick the right value, set the correct value explicitly. Test Plan: - Created a new rule in Chrome, Safari. - Added fields and conditions in Chrome, Safari. - Edited existing rules in Chrome, Safari. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10646 Differential Revision: https://secure.phabricator.com/D15507 --- resources/celerity/map.php | 22 ++--- .../controller/HeraldRuleController.php | 85 +++++++++++-------- .../js/application/herald/HeraldRuleEditor.js | 16 +++- 3 files changed, 73 insertions(+), 50 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 48a8367edb..d5d2616f0c 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -401,7 +401,7 @@ return array( 'rsrc/js/application/drydock/drydock-live-operation-status.js' => '901935ef', 'rsrc/js/application/files/behavior-icon-composer.js' => '8499b6ab', 'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888', - 'rsrc/js/application/herald/HeraldRuleEditor.js' => '746ca158', + 'rsrc/js/application/herald/HeraldRuleEditor.js' => 'd6a7e717', 'rsrc/js/application/herald/PathTypeahead.js' => 'f7fc67ec', 'rsrc/js/application/herald/herald-rule-editor.js' => '7ebaeed3', 'rsrc/js/application/maniphest/behavior-batch-editor.js' => '782ab6e7', @@ -563,7 +563,7 @@ return array( 'global-drag-and-drop-css' => '5c1b47c2', 'harbormaster-css' => '834879db', 'herald-css' => 'dc31f6e9', - 'herald-rule-editor' => '746ca158', + 'herald-rule-editor' => 'd6a7e717', 'herald-test-css' => 'a52e323e', 'inline-comment-summary-css' => '51efda3a', 'javelin-aphlict' => '5359e785', @@ -1435,15 +1435,6 @@ return array( 'javelin-vector', 'javelin-dom', ), - '746ca158' => array( - 'multirow-row-manager', - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-json', - 'phabricator-prefab', - ), '76b9fc3e' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1923,6 +1914,15 @@ return array( 'javelin-dom', 'javelin-stratcom', ), + 'd6a7e717' => array( + 'multirow-row-manager', + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-json', + 'phabricator-prefab', + ), 'd75709e6' => array( 'javelin-behavior', 'javelin-workflow', diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php index d8e5e42df3..d7963a5193 100644 --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -364,43 +364,6 @@ final class HeraldRuleController extends HeraldController { array $handles, HeraldAdapter $adapter) { - $serial_conditions = array( - array('default', 'default', ''), - ); - - if ($rule->getConditions()) { - $serial_conditions = array(); - foreach ($rule->getConditions() as $condition) { - $value = $adapter->getEditorValueForCondition( - $this->getViewer(), - $condition); - - $serial_conditions[] = array( - $condition->getFieldName(), - $condition->getFieldCondition(), - $value, - ); - } - } - - $serial_actions = array( - array('default', ''), - ); - - if ($rule->getActions()) { - $serial_actions = array(); - foreach ($rule->getActions() as $action) { - $value = $adapter->getEditorValueForAction( - $this->getViewer(), - $action); - - $serial_actions[] = array( - $action->getAction(), - $value, - ); - } - } - $all_rules = $this->loadRulesThisRuleMayDependUpon($rule); $all_rules = mpull($all_rules, 'getName', 'getPHID'); asort($all_rules); @@ -492,10 +455,58 @@ final class HeraldRuleController extends HeraldController { $config_info['targets'][$action] = $value_key; } + $default_group = head($config_info['fields']); + $default_field = head_key($default_group['options']); + $default_condition = head($config_info['conditionMap'][$default_field]); + $default_actions = head($config_info['actions']); + $default_action = head_key($default_actions['options']); + + if ($rule->getConditions()) { + $serial_conditions = array(); + foreach ($rule->getConditions() as $condition) { + $value = $adapter->getEditorValueForCondition( + $this->getViewer(), + $condition); + + $serial_conditions[] = array( + $condition->getFieldName(), + $condition->getFieldCondition(), + $value, + ); + } + } else { + $serial_conditions = array( + array($default_field, $default_condition, null), + ); + } + + if ($rule->getActions()) { + $serial_actions = array(); + foreach ($rule->getActions() as $action) { + $value = $adapter->getEditorValueForAction( + $this->getViewer(), + $action); + + $serial_actions[] = array( + $action->getAction(), + $value, + ); + } + } else { + $serial_actions = array( + array($default_action, null), + ); + } + Javelin::initBehavior( 'herald-rule-editor', array( 'root' => 'herald-rule-edit-form', + 'default' => array( + 'field' => $default_field, + 'condition' => $default_condition, + 'action' => $default_action, + ), 'conditions' => (object)$serial_conditions, 'actions' => (object)$serial_actions, 'template' => $this->buildTokenizerTemplates() + array( diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index ba01c9bef3..8c6b46ba44 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -323,7 +323,14 @@ JX.install('HeraldRuleEditor', { _newCondition : function(data) { var row = this._conditionsRowManager.addRow([]); var row_id = this._conditionsRowManager.getRowID(row); - this._config.conditions[row_id] = data || [null, null, '']; + + var default_condition = [ + this._config.default.field, + this._config.default.condition, + null + ]; + this._config.conditions[row_id] = data || default_condition; + var r = this._conditionsRowManager.updateRow( row_id, this._renderCondition(row_id)); @@ -369,7 +376,12 @@ JX.install('HeraldRuleEditor', { }, _newAction : function(data) { - data = data || []; + var default_action = [ + this._config.default.action, + null + ]; + + data = data || default_action; var temprow = this._actionsRowManager.addRow([]); var row_id = this._actionsRowManager.getRowID(temprow); this._config.actions[row_id] = data; From 86720b4595a71818e6ef21f7210ae3403032c69b Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Mar 2016 09:20:32 -0700 Subject: [PATCH 08/31] Fix tag limit logic in PHUIHandleTagListView Summary: Fixes T10648. This was goofed and always did a meaningless no-op slice -- I mucked it up while doing the disabled project stuff elsewhere. Test Plan: - Tagged something with 5 projects. - Saw the list sliced to 4 (the limit) with "...". Reviewers: chad Reviewed By: chad Maniphest Tasks: T10648 Differential Revision: https://secure.phabricator.com/D15508 --- src/applications/phid/view/PHUIHandleTagListView.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/applications/phid/view/PHUIHandleTagListView.php b/src/applications/phid/view/PHUIHandleTagListView.php index b789fa1bf1..eb909d4b86 100644 --- a/src/applications/phid/view/PHUIHandleTagListView.php +++ b/src/applications/phid/view/PHUIHandleTagListView.php @@ -61,7 +61,7 @@ final class PHUIHandleTagListView extends AphrontTagView { } } - if ($this->limit && ($this->limit > count($handles))) { + if ($this->limit && (count($handles) > $this->limit)) { if (!is_array($handles)) { $handles = iterator_to_array($handles); } @@ -85,7 +85,7 @@ final class PHUIHandleTagListView extends AphrontTagView { } if ($this->limit) { - if ($this->limit < count($this->handles)) { + if (count($this->handles) > $this->limit) { $tip_text = implode(', ', mpull($this->handles, 'getName')); $more = $this->newPlaceholderTag() From 63d755723bcf531b51dc90bbabde5371c3a38fbf Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 14 Oct 2015 18:34:55 -0700 Subject: [PATCH 09/31] Add a "Token" Credential type Summary: Ref T9456. This is just a convenience type for things like API tokens, to make it harder for users to make mistakes and keep SSH keys out of the dropdown for "choose your API token". Test Plan: {F879820} Reviewers: chad Reviewed By: chad Subscribers: joshuaspence Maniphest Tasks: T9456 Differential Revision: https://secure.phabricator.com/D14284 --- src/__phutil_library_map__.php | 2 + .../PassphraseCredentialViewController.php | 10 ++--- .../PassphraseTokenCredentialType.php | 37 +++++++++++++++++++ .../PassphraseCredentialTransactionEditor.php | 2 +- .../passphrase/keys/PassphraseAbstractKey.php | 6 +-- .../query/PassphraseCredentialQuery.php | 11 ++++++ .../storage/PassphraseCredential.php | 10 +++++ 7 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 src/applications/passphrase/credentialtype/PassphraseTokenCredentialType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 02a011ec72..5ef56bd01a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1645,6 +1645,7 @@ phutil_register_library_map(array( 'PassphraseSSHPrivateKeyTextCredentialType' => 'applications/passphrase/credentialtype/PassphraseSSHPrivateKeyTextCredentialType.php', 'PassphraseSchemaSpec' => 'applications/passphrase/storage/PassphraseSchemaSpec.php', 'PassphraseSecret' => 'applications/passphrase/storage/PassphraseSecret.php', + 'PassphraseTokenCredentialType' => 'applications/passphrase/credentialtype/PassphraseTokenCredentialType.php', 'PasteConduitAPIMethod' => 'applications/paste/conduit/PasteConduitAPIMethod.php', 'PasteCreateConduitAPIMethod' => 'applications/paste/conduit/PasteCreateConduitAPIMethod.php', 'PasteCreateMailReceiver' => 'applications/paste/mail/PasteCreateMailReceiver.php', @@ -5949,6 +5950,7 @@ phutil_register_library_map(array( 'PassphraseSSHPrivateKeyTextCredentialType' => 'PassphraseSSHPrivateKeyCredentialType', 'PassphraseSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PassphraseSecret' => 'PassphraseDAO', + 'PassphraseTokenCredentialType' => 'PassphraseCredentialType', 'PasteConduitAPIMethod' => 'ConduitAPIMethod', 'PasteCreateConduitAPIMethod' => 'PasteConduitAPIMethod', 'PasteCreateMailReceiver' => 'PhabricatorMailReceiver', diff --git a/src/applications/passphrase/controller/PassphraseCredentialViewController.php b/src/applications/passphrase/controller/PassphraseCredentialViewController.php index 46bb13a688..db31964773 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialViewController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialViewController.php @@ -14,20 +14,16 @@ final class PassphraseCredentialViewController extends PassphraseController { return new Aphront404Response(); } - $type = PassphraseCredentialType::getTypeByConstant( - $credential->getCredentialType()); - if (!$type) { - throw new Exception(pht('Credential has invalid type "%s"!', $type)); - } + $type = $credential->getImplementation(); $timeline = $this->buildTransactionTimeline( $credential, new PassphraseCredentialTransactionQuery()); $timeline->setShouldTerminate(true); - $title = pht('%s %s', 'K'.$credential->getID(), $credential->getName()); + $title = pht('%s %s', $credential->getMonogram(), $credential->getName()); $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb('K'.$credential->getID()); + $crumbs->addTextCrumb($credential->getMonogram()); $crumbs->setBorder(true); $header = $this->buildHeaderView($credential); diff --git a/src/applications/passphrase/credentialtype/PassphraseTokenCredentialType.php b/src/applications/passphrase/credentialtype/PassphraseTokenCredentialType.php new file mode 100644 index 0000000000..d057d61eba --- /dev/null +++ b/src/applications/passphrase/credentialtype/PassphraseTokenCredentialType.php @@ -0,0 +1,37 @@ +getCredentialTypeImplementation(); + $credential_type = $object->getImplementation(); if (!$credential_type->shouldRequireUsername()) { break; } diff --git a/src/applications/passphrase/keys/PassphraseAbstractKey.php b/src/applications/passphrase/keys/PassphraseAbstractKey.php index da56de15af..e3d902236d 100644 --- a/src/applications/passphrase/keys/PassphraseAbstractKey.php +++ b/src/applications/passphrase/keys/PassphraseAbstractKey.php @@ -32,13 +32,13 @@ abstract class PassphraseAbstractKey extends Phobject { PassphraseCredential $credential, $provides_type) { - $type = $credential->getCredentialTypeImplementation(); + $type = $credential->getImplementation(); if (!$type) { throw new Exception( pht( 'Credential "%s" is of unknown type "%s"!', - 'K'.$credential->getID(), + $credential->getMonogram(), $credential->getCredentialType())); } @@ -46,7 +46,7 @@ abstract class PassphraseAbstractKey extends Phobject { throw new Exception( pht( 'Credential "%s" must provide "%s", but provides "%s"!', - 'K'.$credential->getID(), + $credential->getMonogram(), $provides_type, $type->getProvidesType())); } diff --git a/src/applications/passphrase/query/PassphraseCredentialQuery.php b/src/applications/passphrase/query/PassphraseCredentialQuery.php index 9411fa8a77..3a78ac0d13 100644 --- a/src/applications/passphrase/query/PassphraseCredentialQuery.php +++ b/src/applications/passphrase/query/PassphraseCredentialQuery.php @@ -89,6 +89,17 @@ final class PassphraseCredentialQuery } } + foreach ($page as $key => $credential) { + $type = PassphraseCredentialType::getTypeByConstant( + $credential->getCredentialType()); + if (!$type) { + unset($page[$key]); + continue; + } + + $credential->attachImplementation(clone $type); + } + return $page; } diff --git a/src/applications/passphrase/storage/PassphraseCredential.php b/src/applications/passphrase/storage/PassphraseCredential.php index 8384f5bd52..88bc4595bd 100644 --- a/src/applications/passphrase/storage/PassphraseCredential.php +++ b/src/applications/passphrase/storage/PassphraseCredential.php @@ -25,6 +25,7 @@ final class PassphraseCredential extends PassphraseDAO protected $spacePHID; private $secret = self::ATTACHABLE; + private $implementation = self::ATTACHABLE; public static function initializeNewCredential(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) @@ -98,6 +99,15 @@ final class PassphraseCredential extends PassphraseDAO return PassphraseCredentialType::getTypeByConstant($type); } + public function attachImplementation(PassphraseCredentialType $impl) { + $this->implementation = $impl; + return $this; + } + + public function getImplementation() { + return $this->assertAttached($this->implementation); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ From f82db7524b32deb63c807746eba08d52eb8e7344 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 14 Oct 2015 20:01:22 -0700 Subject: [PATCH 10/31] Add a "Build with CircleCI" build step Summary: Ref T9456. Some rough edges and we can't complete the build yet since I haven't written a webhook, but this mostly seems to be working. Test Plan: - Ran this build on some stuff. - Ran a normal HTTP step build to make sure I didn't break that. {F880301} {F880302} {F880303} Reviewers: chad Reviewed By: chad Subscribers: JustinTulloss, joshma Maniphest Tasks: T9456 Differential Revision: https://secure.phabricator.com/D14286 --- src/__phutil_library_map__.php | 5 + .../differential/storage/DifferentialDiff.php | 67 +++++ .../HarbormasterStepEditController.php | 20 +- ...HarbormasterCircleCIBuildableInterface.php | 12 + .../HarbormasterBuildStepImplementation.php | 35 +++ ...rmasterCircleCIBuildStepImplementation.php | 246 ++++++++++++++++++ ...sterHTTPRequestBuildStepImplementation.php | 24 +- .../storage/PhabricatorRepositoryCommit.php | 47 ++++ 8 files changed, 427 insertions(+), 29 deletions(-) create mode 100644 src/applications/harbormaster/interface/HarbormasterCircleCIBuildableInterface.php create mode 100644 src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5ef56bd01a..0d6d91a67f 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1106,6 +1106,8 @@ phutil_register_library_map(array( 'HarbormasterBuildableTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php', 'HarbormasterBuildableViewController' => 'applications/harbormaster/controller/HarbormasterBuildableViewController.php', 'HarbormasterBuiltinBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterBuiltinBuildStepGroup.php', + 'HarbormasterCircleCIBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php', + 'HarbormasterCircleCIBuildableInterface' => 'applications/harbormaster/interface/HarbormasterCircleCIBuildableInterface.php', 'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php', 'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php', 'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php', @@ -4517,6 +4519,7 @@ phutil_register_library_map(array( 'DifferentialDAO', 'PhabricatorPolicyInterface', 'HarbormasterBuildableInterface', + 'HarbormasterCircleCIBuildableInterface', 'PhabricatorApplicationTransactionInterface', 'PhabricatorDestructibleInterface', ), @@ -5334,6 +5337,7 @@ phutil_register_library_map(array( 'HarbormasterBuildableTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'HarbormasterBuildableViewController' => 'HarbormasterController', 'HarbormasterBuiltinBuildStepGroup' => 'HarbormasterBuildStepGroup', + 'HarbormasterCircleCIBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod', 'HarbormasterController' => 'PhabricatorController', 'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod', @@ -7652,6 +7656,7 @@ phutil_register_library_map(array( 'PhabricatorSubscribableInterface', 'PhabricatorMentionableInterface', 'HarbormasterBuildableInterface', + 'HarbormasterCircleCIBuildableInterface', 'PhabricatorCustomFieldInterface', 'PhabricatorApplicationTransactionInterface', 'PhabricatorFulltextInterface', diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index 913abad3ab..0e32855c26 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -5,6 +5,7 @@ final class DifferentialDiff implements PhabricatorPolicyInterface, HarbormasterBuildableInterface, + HarbormasterCircleCIBuildableInterface, PhabricatorApplicationTransactionInterface, PhabricatorDestructibleInterface { @@ -524,6 +525,72 @@ final class DifferentialDiff ); } + +/* -( HarbormasterCircleCIBuildableInterface )----------------------------- */ + + + public function getCircleCIGitHubRepositoryURI() { + $diff_phid = $this->getPHID(); + $repository_phid = $this->getRepositoryPHID(); + if (!$repository_phid) { + throw new Exception( + pht( + 'This diff ("%s") is not associated with a repository. A diff '. + 'must belong to a tracked repository to be built by CircleCI.', + $diff_phid)); + } + + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs(array($repository_phid)) + ->executeOne(); + if (!$repository) { + throw new Exception( + pht( + 'This diff ("%s") is associated with a repository ("%s") which '. + 'could not be loaded.', + $diff_phid, + $repository_phid)); + } + + $staging_uri = $repository->getStagingURI(); + if (!$staging_uri) { + throw new Exception( + pht( + 'This diff ("%s") is associated with a repository ("%s") that '. + 'does not have a Staging Area configured. You must configure a '. + 'Staging Area to use CircleCI integration.', + $diff_phid, + $repository_phid)); + } + + $path = HarbormasterCircleCIBuildStepImplementation::getGitHubPath( + $staging_uri); + if (!$path) { + throw new Exception( + pht( + 'This diff ("%s") is associated with a repository ("%s") that '. + 'does not have a Staging Area ("%s") that is hosted on GitHub. '. + 'CircleCI can only build from GitHub, so the Staging Area for '. + 'the repository must be hosted there.', + $diff_phid, + $repository_phid, + $staging_uri)); + } + + return $staging_uri; + } + + public function getCircleCIBuildIdentifierType() { + return 'tag'; + } + + public function getCircleCIBuildIdentifier() { + $ref = $this->getStagingRef(); + $ref = preg_replace('(^refs/tags/)', '', $ref); + return $ref; + } + public function getStagingRef() { // TODO: We're just hoping to get lucky. Instead, `arc` should store // where it sent changes and we should only provide staging details diff --git a/src/applications/harbormaster/controller/HarbormasterStepEditController.php b/src/applications/harbormaster/controller/HarbormasterStepEditController.php index 49c4563183..10f35e5fe4 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php @@ -136,13 +136,19 @@ final class HarbormasterStepEditController } $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild( - id(new AphrontFormTextControl()) - ->setName('name') - ->setLabel(pht('Name')) - ->setError($e_name) - ->setValue($v_name)); + ->setUser($viewer); + + $instructions = $implementation->getEditInstructions(); + if (strlen($instructions)) { + $form->appendRemarkupInstructions($instructions); + } + + $form->appendChild( + id(new AphrontFormTextControl()) + ->setName('name') + ->setLabel(pht('Name')) + ->setError($e_name) + ->setValue($v_name)); $form->appendChild(id(new AphrontFormDividerControl())); diff --git a/src/applications/harbormaster/interface/HarbormasterCircleCIBuildableInterface.php b/src/applications/harbormaster/interface/HarbormasterCircleCIBuildableInterface.php new file mode 100644 index 0000000000..f02c4cfff0 --- /dev/null +++ b/src/applications/harbormaster/interface/HarbormasterCircleCIBuildableInterface.php @@ -0,0 +1,12 @@ +getGenericDescription(); } + public function getEditInstructions() { + return null; + } + /** * Run the build target against the specified build. */ @@ -265,6 +269,37 @@ abstract class HarbormasterBuildStepImplementation extends Phobject { } + protected function logHTTPResponse( + HarbormasterBuild $build, + HarbormasterBuildTarget $build_target, + BaseHTTPFuture $future, + $label) { + + list($status, $body, $headers) = $future->resolve(); + + $header_lines = array(); + + // TODO: We don't currently preserve the entire "HTTP" response header, but + // should. Once we do, reproduce it here faithfully. + $status_code = $status->getStatusCode(); + $header_lines[] = "HTTP {$status_code}"; + + foreach ($headers as $header) { + list($head, $tail) = $header; + $header_lines[] = "{$head}: {$tail}"; + } + $header_lines = implode("\n", $header_lines); + + $build_target + ->newLog($label, 'http.head') + ->append($header_lines); + + $build_target + ->newLog($label, 'http.body') + ->append($body); + } + + /* -( Automatic Targets )-------------------------------------------------- */ diff --git a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php new file mode 100644 index 0000000000..d4124cee20 --- /dev/null +++ b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php @@ -0,0 +1,246 @@ +getDomain(); + + if (!strlen($domain)) { + $uri_object = new PhutilGitURI($uri); + $domain = $uri_object->getDomain(); + } + + $domain = phutil_utf8_strtolower($domain); + switch ($domain) { + case 'github.com': + case 'www.github.com': + return $uri_object->getPath(); + default: + return null; + } + } + + public function execute( + HarbormasterBuild $build, + HarbormasterBuildTarget $build_target) { + $viewer = PhabricatorUser::getOmnipotentUser(); + + $buildable = $build->getBuildable(); + + $object = $buildable->getBuildableObject(); + $object_phid = $object->getPHID(); + if (!($object instanceof HarbormasterCircleCIBuildableInterface)) { + throw new Exception( + pht( + 'Object ("%s") does not implement interface "%s". Only objects '. + 'which implement this interface can be built with CircleCI.', + $object_phid, + 'HarbormasterCircleCIBuildableInterface')); + } + + $github_uri = $object->getCircleCIGitHubRepositoryURI(); + $build_type = $object->getCircleCIBuildIdentifierType(); + $build_identifier = $object->getCircleCIBuildIdentifier(); + + $path = self::getGitHubPath($github_uri); + if ($path === null) { + throw new Exception( + pht( + 'Object ("%s") claims "%s" is a GitHub repository URI, but the '. + 'domain does not appear to be GitHub.', + $object_phid, + $github_uri)); + } + + $path_parts = trim($path, '/'); + $path_parts = explode('/', $path_parts); + if (count($path_parts) < 2) { + throw new Exception( + pht( + 'Object ("%s") claims "%s" is a GitHub repository URI, but the '. + 'path ("%s") does not have enough components (expected at least '. + 'two).', + $object_phid, + $github_uri, + $path)); + } + + list($github_namespace, $github_name) = $path_parts; + $github_name = preg_replace('(\\.git$)', '', $github_name); + + $credential_phid = $this->getSetting('token'); + $api_token = id(new PassphraseCredentialQuery()) + ->setViewer($viewer) + ->withPHIDs(array($credential_phid)) + ->needSecrets(true) + ->executeOne(); + if (!$api_token) { + throw new Exception( + pht( + 'Unable to load API token ("%s")!', + $credential_phid)); + } + + // When we pass "revision", the branch is ignored (and does not even need + // to exist), and only shows up in the UI. Use a cute string which will + // certainly never break anything or cause any kind of problem. + $ship = "\xF0\x9F\x9A\xA2"; + $branch = "{$ship}Harbormaster"; + + $token = $api_token->getSecret()->openEnvelope(); + $parts = array( + 'https://circleci.com/api/v1/project', + phutil_escape_uri($github_namespace), + phutil_escape_uri($github_name)."?circle-token={$token}", + ); + + $uri = implode('/', $parts); + + $data_structure = array(); + switch ($build_type) { + case 'tag': + $data_structure['tag'] = $build_identifier; + break; + case 'revision': + $data_structure['revision'] = $build_identifier; + break; + default: + throw new Exception( + pht( + 'Unknown CircleCI build type "%s". Expected "%s" or "%s".', + $build_type, + 'tag', + 'revision')); + } + + $data_structure['build_parameters'] = array( + 'HARBORMASTER_BUILD_TARGET_PHID' => $build_target->getPHID(), + ); + + $json_data = phutil_json_encode($data_structure); + + $future = id(new HTTPSFuture($uri, $json_data)) + ->setMethod('POST') + ->addHeader('Content-Type', 'application/json') + ->addHeader('Accept', 'application/json') + ->setTimeout(60); + + $this->resolveFutures( + $build, + $build_target, + array($future)); + + $this->logHTTPResponse($build, $build_target, $future, pht('CircleCI')); + + list($status, $body) = $future->resolve(); + if ($status->isError()) { + throw new HarbormasterBuildFailureException(); + } + + $response = phutil_json_decode($body); + $build_uri = idx($response, 'build_url'); + if (!$build_uri) { + throw new Exception( + pht( + 'CircleCI did not return a "%s"!', + 'build_url')); + } + + $target_phid = $build_target->getPHID(); + + // Write an artifact to create a link to the external build in CircleCI. + + $api_method = 'harbormaster.createartifact'; + $api_params = array( + 'buildTargetPHID' => $target_phid, + 'artifactType' => HarbormasterURIArtifact::ARTIFACTCONST, + 'artifactKey' => 'circleci.uri', + 'artifactData' => array( + 'uri' => $build_uri, + 'name' => pht('View in CircleCI'), + 'ui.external' => true, + ), + ); + + id(new ConduitCall($api_method, $api_params)) + ->setUser($viewer) + ->execute(); + } + + public function getFieldSpecifications() { + return array( + 'token' => array( + 'name' => pht('API Token'), + 'type' => 'credential', + 'credential.type' + => PassphraseTokenCredentialType::CREDENTIAL_TYPE, + 'credential.provides' + => PassphraseTokenCredentialType::PROVIDES_TYPE, + 'required' => true, + ), + ); + } + + public function supportsWaitForMessage() { + // NOTE: We always wait for a message, but don't need to show the UI + // control since "Wait" is the only valid choice. + return false; + } + + public function shouldWaitForMessage(HarbormasterBuildTarget $target) { + return true; + } + +} diff --git a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php index 10d2cb08b2..016f29642e 100644 --- a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php @@ -72,29 +72,9 @@ final class HarbormasterHTTPRequestBuildStepImplementation $build_target, array($future)); - list($status, $body, $headers) = $future->resolve(); - - $header_lines = array(); - - // TODO: We don't currently preserve the entire "HTTP" response header, but - // should. Once we do, reproduce it here faithfully. - $status_code = $status->getStatusCode(); - $header_lines[] = "HTTP {$status_code}"; - - foreach ($headers as $header) { - list($head, $tail) = $header; - $header_lines[] = "{$head}: {$tail}"; - } - $header_lines = implode("\n", $header_lines); - - $build_target - ->newLog($uri, 'http.head') - ->append($header_lines); - - $build_target - ->newLog($uri, 'http.body') - ->append($body); + $this->logHTTPResponse($build, $build_target, $future, $uri); + list($status) = $future->resolve(); if ($status->isError()) { throw new HarbormasterBuildFailureException(); } diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php index 18732e5e4c..6e56cc711a 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -10,6 +10,7 @@ final class PhabricatorRepositoryCommit PhabricatorSubscribableInterface, PhabricatorMentionableInterface, HarbormasterBuildableInterface, + HarbormasterCircleCIBuildableInterface, PhabricatorCustomFieldInterface, PhabricatorApplicationTransactionInterface, PhabricatorFulltextInterface { @@ -411,6 +412,52 @@ final class PhabricatorRepositoryCommit } +/* -( HarbormasterCircleCIBuildableInterface )----------------------------- */ + + + public function getCircleCIGitHubRepositoryURI() { + $repository = $this->getRepository(); + + $commit_phid = $this->getPHID(); + $repository_phid = $repository->getPHID(); + + if ($repository->isHosted()) { + throw new Exception( + pht( + 'This commit ("%s") is associated with a hosted repository '. + '("%s"). Repositories must be imported from GitHub to be built '. + 'with CircleCI.', + $commit_phid, + $repository_phid)); + } + + $remote_uri = $repository->getRemoteURI(); + $path = HarbormasterCircleCIBuildStepImplementation::getGitHubPath( + $remote_uri); + if (!$path) { + throw new Exception( + pht( + 'This commit ("%s") is associated with a repository ("%s") that '. + 'with a remote URI ("%s") that does not appear to be hosted on '. + 'GitHub. Repositories must be hosted on GitHub to be built with '. + 'CircleCI.', + $commit_phid, + $repository_phid, + $remote_uri)); + } + + return $remote_uri; + } + + public function getCircleCIBuildIdentifierType() { + return 'revision'; + } + + public function getCircleCIBuildIdentifier() { + return $this->getCommitIdentifier(); + } + + /* -( PhabricatorCustomFieldInterface )------------------------------------ */ From 7868c5d7d0fd28b6859cb11e5d831ba7a97409aa Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 15 Oct 2015 09:07:31 -0700 Subject: [PATCH 11/31] Add a CircleCI webhook Summary: Ref T9456. This makes everything work, except that CircleCI doesn't fetch tags which are not ancestors of branch heads. Test Plan: Ran passing builds through CircleCI. Reviewers: chad Reviewed By: chad Subscribers: dpaola2, JustinTulloss Maniphest Tasks: T9456 Differential Revision: https://secure.phabricator.com/D14288 --- src/__phutil_library_map__.php | 2 + .../PhabricatorHarbormasterApplication.php | 3 + .../HarbormasterCircleCIHookController.php | 87 +++++++++++++++++++ ...rmasterCircleCIBuildStepImplementation.php | 16 +++- .../storage/HarbormasterBuildMessage.php | 7 +- 5 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 src/applications/harbormaster/controller/HarbormasterCircleCIHookController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0d6d91a67f..ad187d74f0 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1108,6 +1108,7 @@ phutil_register_library_map(array( 'HarbormasterBuiltinBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterBuiltinBuildStepGroup.php', 'HarbormasterCircleCIBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php', 'HarbormasterCircleCIBuildableInterface' => 'applications/harbormaster/interface/HarbormasterCircleCIBuildableInterface.php', + 'HarbormasterCircleCIHookController' => 'applications/harbormaster/controller/HarbormasterCircleCIHookController.php', 'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php', 'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php', 'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php', @@ -5338,6 +5339,7 @@ phutil_register_library_map(array( 'HarbormasterBuildableViewController' => 'HarbormasterController', 'HarbormasterBuiltinBuildStepGroup' => 'HarbormasterBuildStepGroup', 'HarbormasterCircleCIBuildStepImplementation' => 'HarbormasterBuildStepImplementation', + 'HarbormasterCircleCIHookController' => 'HarbormasterController', 'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod', 'HarbormasterController' => 'PhabricatorController', 'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod', diff --git a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php index d111f266a6..00fb570e03 100644 --- a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php +++ b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php @@ -91,6 +91,9 @@ final class PhabricatorHarbormasterApplication extends PhabricatorApplication { 'lint/' => array( '(?P\d+)/' => 'HarbormasterLintMessagesController', ), + 'hook/' => array( + 'circleci/' => 'HarbormasterCircleCIHookController', + ), ), ); } diff --git a/src/applications/harbormaster/controller/HarbormasterCircleCIHookController.php b/src/applications/harbormaster/controller/HarbormasterCircleCIHookController.php new file mode 100644 index 0000000000..7f6d4900fe --- /dev/null +++ b/src/applications/harbormaster/controller/HarbormasterCircleCIHookController.php @@ -0,0 +1,87 @@ +setViewer($viewer) + ->withPHIDs(array($target_phid)) + ->needBuildSteps(true) + ->executeOne(); + if ($target) { + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $this->updateTarget($target, $payload); + } + } + + $response = new AphrontWebpageResponse(); + $response->setContent(pht("Request OK\n")); + return $response; + } + + private function updateTarget( + HarbormasterBuildTarget $target, + array $payload) { + + $step = $target->getBuildStep(); + $impl = $step->getStepImplementation(); + if (!($impl instanceof HarbormasterCircleCIBuildStepImplementation)) { + throw new Exception( + pht( + 'Build target ("%s") has the wrong type of build step. Only '. + 'CircleCI build steps may be updated via the CircleCI webhook.', + $target->getPHID())); + } + + switch (idx($payload, 'status')) { + case 'success': + case 'fixed': + $message_type = HarbormasterMessageType::MESSAGE_PASS; + break; + default: + $message_type = HarbormasterMessageType::MESSAGE_FAIL; + break; + } + + $viewer = PhabricatorUser::getOmnipotentUser(); + + $api_method = 'harbormaster.sendmessage'; + $api_params = array( + 'buildTargetPHID' => $target->getPHID(), + 'type' => $message_type, + ); + + id(new ConduitCall($api_method, $api_params)) + ->setUser($viewer) + ->execute(); + } + +} diff --git a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php index d4124cee20..365cf657de 100644 --- a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php @@ -20,6 +20,9 @@ final class HarbormasterCircleCIBuildStepImplementation } public function getEditInstructions() { + $hook_uri = '/harbormaster/hook/circleci/'; + $hook_uri = PhabricatorEnv::getProductionURI($hook_uri); + return pht(<<getPHID(); + if (!$actor_phid) { + $actor_phid = id(new PhabricatorHarbormasterApplication())->getPHID(); + } + return id(new HarbormasterBuildMessage()) - ->setAuthorPHID($actor->getPHID()) + ->setAuthorPHID($actor_phid) ->setIsConsumed(0); } From 44c3f06ab96d168f6df6fd665e7d35983060a38b Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 22 Mar 2016 12:21:14 -0700 Subject: [PATCH 12/31] Fix Phortune cart fatal Summary: This is failing locally for me, set to getViewer and pull up cart. Test Plan: View cart with a description. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D15509 --- src/applications/phortune/controller/PhortuneCartController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/phortune/controller/PhortuneCartController.php b/src/applications/phortune/controller/PhortuneCartController.php index 9fefc7db6e..628a466056 100644 --- a/src/applications/phortune/controller/PhortuneCartController.php +++ b/src/applications/phortune/controller/PhortuneCartController.php @@ -48,7 +48,7 @@ abstract class PhortuneCartController return null; } - $output = new PHUIRemarkupView($this->getUser(), $description); + $output = new PHUIRemarkupView($this->getViewer(), $description); $box = id(new PHUIBoxView()) ->addMargin(PHUI::MARGIN_LARGE) From 47dedfb152d0b83e936a857e83530d3754bfa335 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 9 Mar 2016 08:40:19 -0800 Subject: [PATCH 13/31] Introduce "bridged" objects Summary: Ref T10537. These are objects which are bound to some external object, like a Maniphest task which is a representation of a GitHub issue. This doesn't do much yet and may change, but my thinking is: - I'm putting these on-object instead of on edges because I think we want to actively change the UI for them (e.g., clearly call out that the object is bridged) but don't want every page to need to do extra queries in the common case where zero bridged objects exist anywhere in the system. - I'm making these one-to-one, more or less: an issue can't be bridged to a bunch of tasks, nor can a bunch of tasks be bridged to a single issue. Pretty sure this makes sense? I can't come up with any reasonable, realistic cases where you want a single GitHub issue to publish to multiple different tasks in Maniphest. - Technically, one type of each bridgable object could be bridged, but I expect this to never actually occur. Hopefully. Test Plan: Ran storage upgrade, loaded some pages. Reviewers: chad Reviewed By: chad Subscribers: Luke081515.2 Maniphest Tasks: T10537 Differential Revision: https://secure.phabricator.com/D15502 --- .../20160321.nuance.01.taskbridge.sql | 2 + src/__phutil_library_map__.php | 2 + .../DoorkeeperBridgedObjectInterface.php | 8 +++ .../maniphest/query/ManiphestTaskQuery.php | 13 +++++ .../maniphest/storage/ManiphestTask.php | 24 ++++++++- .../policy/PhabricatorPolicyAwareQuery.php | 51 +++++++++++++++++++ 6 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 resources/sql/autopatches/20160321.nuance.01.taskbridge.sql create mode 100644 src/applications/doorkeeper/bridge/DoorkeeperBridgedObjectInterface.php diff --git a/resources/sql/autopatches/20160321.nuance.01.taskbridge.sql b/resources/sql/autopatches/20160321.nuance.01.taskbridge.sql new file mode 100644 index 0000000000..53b80fe7d9 --- /dev/null +++ b/resources/sql/autopatches/20160321.nuance.01.taskbridge.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_maniphest.maniphest_task + ADD bridgedObjectPHID VARBINARY(64); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ad187d74f0..74dd5958eb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -845,6 +845,7 @@ phutil_register_library_map(array( 'DoorkeeperBridgeGitHubIssue' => 'applications/doorkeeper/bridge/DoorkeeperBridgeGitHubIssue.php', 'DoorkeeperBridgeJIRA' => 'applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php', 'DoorkeeperBridgeJIRATestCase' => 'applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php', + 'DoorkeeperBridgedObjectInterface' => 'applications/doorkeeper/bridge/DoorkeeperBridgedObjectInterface.php', 'DoorkeeperDAO' => 'applications/doorkeeper/storage/DoorkeeperDAO.php', 'DoorkeeperExternalObject' => 'applications/doorkeeper/storage/DoorkeeperExternalObject.php', 'DoorkeeperExternalObjectQuery' => 'applications/doorkeeper/query/DoorkeeperExternalObjectQuery.php', @@ -5636,6 +5637,7 @@ phutil_register_library_map(array( 'PhabricatorSpacesInterface', 'PhabricatorConduitResultInterface', 'PhabricatorFulltextInterface', + 'DoorkeeperBridgedObjectInterface', ), 'ManiphestTaskAssignHeraldAction' => 'HeraldAction', 'ManiphestTaskAssignOtherHeraldAction' => 'ManiphestTaskAssignHeraldAction', diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgedObjectInterface.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgedObjectInterface.php new file mode 100644 index 0000000000..9d829642fc --- /dev/null +++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgedObjectInterface.php @@ -0,0 +1,8 @@ +bridgedObjectPHIDs = $phids; + return $this; + } + public function newResultObject() { return new ManiphestTask(); } @@ -417,6 +423,13 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { $this->subpriorityMax); } + if ($this->bridgedObjectPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'task.bridgedObjectPHID IN (%Ls)', + $this->bridgedObjectPHIDs); + } + return $where; } diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index 7718bdfcf8..9ac7190782 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -15,7 +15,8 @@ final class ManiphestTask extends ManiphestDAO PhabricatorProjectInterface, PhabricatorSpacesInterface, PhabricatorConduitResultInterface, - PhabricatorFulltextInterface { + PhabricatorFulltextInterface, + DoorkeeperBridgedObjectInterface { const MARKUP_FIELD_DESCRIPTION = 'markup:desc'; @@ -36,6 +37,7 @@ final class ManiphestTask extends ManiphestDAO protected $ownerOrdering; protected $spacePHID; + protected $bridgedObjectPHID; protected $properties = array(); protected $points; @@ -43,6 +45,7 @@ final class ManiphestTask extends ManiphestDAO private $groupByProjectPHID = self::ATTACHABLE; private $customFields = self::ATTACHABLE; private $edgeProjectPHIDs = self::ATTACHABLE; + private $bridgedObject = self::ATTACHABLE; public static function initializeNewTask(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) @@ -82,6 +85,7 @@ final class ManiphestTask extends ManiphestDAO 'originalEmailSource' => 'text255?', 'subpriority' => 'double', 'points' => 'double?', + 'bridgedObjectPHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, @@ -116,6 +120,10 @@ final class ManiphestTask extends ManiphestDAO 'key_title' => array( 'columns' => array('title(64)'), ), + 'key_bridgedobject' => array( + 'columns' => array('bridgedObjectPHID'), + 'unique' => true, + ), ), ) + parent::getConfiguration(); } @@ -504,4 +512,18 @@ final class ManiphestTask extends ManiphestDAO return new ManiphestTaskFulltextEngine(); } + +/* -( DoorkeeperBridgedObjectInterface )----------------------------------- */ + + + public function getBridgedObject() { + return $this->assertAttached($this->bridgedObject); + } + + public function attachBridgedObject( + DoorkeeperExternalObject $object = null) { + $this->bridgedObject = $object; + return $this; + } + } diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php index 9512acbc22..42d8001ee2 100644 --- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php @@ -234,6 +234,9 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { if ($page) { $maybe_visible = $this->willFilterPage($page); + if ($maybe_visible) { + $maybe_visible = $this->applyWillFilterPageExtensions($maybe_visible); + } } else { $maybe_visible = array(); } @@ -672,4 +675,52 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { return PhabricatorApplication::isClassInstalledForViewer($class, $viewer); } + private function applyWillFilterPageExtensions(array $page) { + $bridges = array(); + foreach ($page as $key => $object) { + if ($object instanceof DoorkeeperBridgedObjectInterface) { + $bridges[$key] = $object; + } + } + + if ($bridges) { + $external_phids = array(); + foreach ($bridges as $bridge) { + $external_phid = $bridge->getBridgedObjectPHID(); + if ($external_phid) { + $external_phids[$key] = $external_phid; + } + } + + if ($external_phids) { + $external_objects = id(new DoorkeeperExternalObjectQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($external_phids) + ->execute(); + $external_objects = mpull($external_objects, null, 'getPHID'); + } else { + $external_objects = array(); + } + + foreach ($bridges as $key => $bridge) { + $external_phid = idx($external_phids, $key); + if (!$external_phid) { + $bridge->attachBridgedObject(null); + continue; + } + + $external_object = idx($external_objects, $external_phid); + if (!$external_object) { + $this->didRejectResult($bridge); + unset($page[$key]); + continue; + } + + $bridge->attachBridgedObject($external_object); + } + } + + return $page; + } + } From e523585811c82ba21892467ed543e4cdcbea36f0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 21 Mar 2016 17:38:22 -0700 Subject: [PATCH 14/31] Allow Nuance item types to provide actions for items Summary: Ref T10537. This allows item types to expose item actions. Eventually these actions might be things like "promote to task", "tweet reply", "ban user forever", etc. For now, provide a simple action which shows a raw item in a dialog. Test Plan: {F1185573} Reviewers: chad Reviewed By: chad Subscribers: Luke081515.2 Maniphest Tasks: T10537 Differential Revision: https://secure.phabricator.com/D15504 --- src/__phutil_library_map__.php | 2 + .../PhabricatorNuanceApplication.php | 2 + .../controller/NuanceItemActionController.php | 26 ++++++++++++ .../controller/NuanceItemViewController.php | 7 ++++ .../nuance/item/NuanceGitHubEventItemType.php | 42 +++++++++++++++++++ .../nuance/item/NuanceItemType.php | 36 ++++++++++++++++ 6 files changed, 115 insertions(+) create mode 100644 src/applications/nuance/controller/NuanceItemActionController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 74dd5958eb..92290ecd77 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1439,6 +1439,7 @@ phutil_register_library_map(array( 'NuanceImportCursorDataQuery' => 'applications/nuance/query/NuanceImportCursorDataQuery.php', 'NuanceImportCursorPHIDType' => 'applications/nuance/phid/NuanceImportCursorPHIDType.php', 'NuanceItem' => 'applications/nuance/storage/NuanceItem.php', + 'NuanceItemActionController' => 'applications/nuance/controller/NuanceItemActionController.php', 'NuanceItemController' => 'applications/nuance/controller/NuanceItemController.php', 'NuanceItemEditor' => 'applications/nuance/editor/NuanceItemEditor.php', 'NuanceItemListController' => 'applications/nuance/controller/NuanceItemListController.php', @@ -5726,6 +5727,7 @@ phutil_register_library_map(array( 'PhabricatorPolicyInterface', 'PhabricatorApplicationTransactionInterface', ), + 'NuanceItemActionController' => 'NuanceController', 'NuanceItemController' => 'NuanceController', 'NuanceItemEditor' => 'PhabricatorApplicationTransactionEditor', 'NuanceItemListController' => 'NuanceItemController', diff --git a/src/applications/nuance/application/PhabricatorNuanceApplication.php b/src/applications/nuance/application/PhabricatorNuanceApplication.php index 92e89f8255..27f683380f 100644 --- a/src/applications/nuance/application/PhabricatorNuanceApplication.php +++ b/src/applications/nuance/application/PhabricatorNuanceApplication.php @@ -43,6 +43,8 @@ final class PhabricatorNuanceApplication extends PhabricatorApplication { $this->getQueryRoutePattern() => 'NuanceItemListController', 'view/(?P[1-9]\d*)/' => 'NuanceItemViewController', 'manage/(?P[1-9]\d*)/' => 'NuanceItemManageController', + 'action/(?P[1-9]\d*)/(?P[^/]+)/' + => 'NuanceItemActionController', ), 'source/' => array( $this->getQueryRoutePattern() => 'NuanceSourceListController', diff --git a/src/applications/nuance/controller/NuanceItemActionController.php b/src/applications/nuance/controller/NuanceItemActionController.php new file mode 100644 index 0000000000..c64ac5f6ac --- /dev/null +++ b/src/applications/nuance/controller/NuanceItemActionController.php @@ -0,0 +1,26 @@ +getViewer(); + $id = $request->getURIData('id'); + + $item = id(new NuanceItemQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if (!$item) { + return new Aphront404Response(); + } + + $action = $request->getURIData('action'); + + $impl = $item->getImplementation(); + $impl->setViewer($viewer); + $impl->setController($this); + + return $impl->buildActionResponse($item, $action); + } + +} diff --git a/src/applications/nuance/controller/NuanceItemViewController.php b/src/applications/nuance/controller/NuanceItemViewController.php index fd8fbc4563..e8e3967545 100644 --- a/src/applications/nuance/controller/NuanceItemViewController.php +++ b/src/applications/nuance/controller/NuanceItemViewController.php @@ -58,6 +58,13 @@ final class NuanceItemViewController extends NuanceController { ->setIcon('fa-cogs') ->setHref($this->getApplicationURI("item/manage/{$id}/"))); + $impl = $item->getImplementation(); + $impl->setViewer($viewer); + + foreach ($impl->getItemActions($item) as $action) { + $curtain->addAction($action); + } + return $curtain; } diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php index 1e6a72b3b3..cec3d366ec 100644 --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -144,4 +144,46 @@ final class NuanceGitHubEventItemType return NuanceGitHubRawEvent::newEvent($type, $raw); } + public function getItemActions(NuanceItem $item) { + $actions = array(); + + $actions[] = $this->newItemAction($item, 'raw') + ->setName(pht('View Raw Event')) + ->setWorkflow(true) + ->setIcon('fa-code'); + + return $actions; + } + + protected function handleAction(NuanceItem $item, $action) { + $controller = $this->getController(); + + switch ($action) { + case 'raw': + $raw = array( + 'api.type' => $item->getItemProperty('api.type'), + 'api.raw' => $item->getItemProperty('api.raw'), + ); + + $raw_output = id(new PhutilJSON())->encodeFormatted($raw); + + $raw_box = id(new AphrontFormTextAreaControl()) + ->setCustomClass('PhabricatorMonospaced') + ->setLabel(pht('Raw Event')) + ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_TALL) + ->setValue($raw_output); + + $form = id(new AphrontFormView()) + ->appendChild($raw_box); + + return $controller->newDialog() + ->setWidth(AphrontDialogView::WIDTH_FULL) + ->setTitle(pht('GitHub Raw Event')) + ->appendForm($form) + ->addCancelButton($item->getURI(), pht('Done')); + } + + return null; + } + } diff --git a/src/applications/nuance/item/NuanceItemType.php b/src/applications/nuance/item/NuanceItemType.php index 1e0cc7a95c..2f348dee1f 100644 --- a/src/applications/nuance/item/NuanceItemType.php +++ b/src/applications/nuance/item/NuanceItemType.php @@ -4,6 +4,7 @@ abstract class NuanceItemType extends Phobject { private $viewer; + private $controller; public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; @@ -14,6 +15,15 @@ abstract class NuanceItemType return $this->viewer; } + public function setController(PhabricatorController $controller) { + $this->controller = $controller; + return $this; + } + + public function getController() { + return $this->controller; + } + public function canUpdateItems() { return false; } @@ -30,6 +40,10 @@ abstract class NuanceItemType return null; } + public function getItemActions(NuanceItem $item) { + return array(); + } + abstract public function getItemTypeDisplayName(); abstract public function getItemDisplayName(NuanceItem $item); @@ -60,4 +74,26 @@ abstract class NuanceItemType ->execute(); } + final protected function newItemAction(NuanceItem $item, $key) { + $id = $item->getID(); + $action_uri = "/nuance/item/action/{$id}/{$key}/"; + + return id(new PhabricatorActionView()) + ->setHref($action_uri); + } + + final public function buildActionResponse(NuanceItem $item, $action) { + $response = $this->handleAction($item, $action); + + if ($response === null) { + return new Aphront404Response(); + } + + return $response; + } + + protected function handleAction(NuanceItem $item, $action) { + return null; + } + } From a90daf5d304881aa69dc9adcb08c32721c162313 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 21 Mar 2016 19:06:46 -0700 Subject: [PATCH 15/31] Add very basic item rendering for GitHub events, parse IDs + URIs Summary: Ref T10538. This extracts and renders URIs for GitHub events so we can link to the original thing on GitHub. Test Plan: {F1186332} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10538 Differential Revision: https://secure.phabricator.com/D15505 --- .../nuance/github/NuanceGitHubRawEvent.php | 62 +++++++++++++++++++ .../NuanceGitHubRawEventTestCase.php | 2 + .../github/__tests__/issueevents/assigned.txt | 4 +- .../github/__tests__/issueevents/closed.txt | 4 +- .../__tests__/issueevents/demilestoned.txt | 4 +- .../github/__tests__/issueevents/labeled.txt | 4 +- .../github/__tests__/issueevents/locked.txt | 4 +- .../__tests__/issueevents/milestoned.txt | 4 +- .../github/__tests__/issueevents/renamed.txt | 4 +- .../github/__tests__/issueevents/reopened.txt | 4 +- .../__tests__/issueevents/unassigned.txt | 4 +- .../__tests__/issueevents/unlabeled.txt | 4 +- .../github/__tests__/issueevents/unlocked.txt | 4 +- .../IssueCommentEvent.created.pull.txt | 4 +- .../IssueCommentEvent.created.txt | 4 +- .../repositoryevents/IssuesEvent.closed.txt | 4 +- .../repositoryevents/IssuesEvent.opened.txt | 4 +- .../repositoryevents/IssuesEvent.reopened.txt | 4 +- .../PullRequestEvent.opened.txt | 4 +- .../__tests__/repositoryevents/PushEvent.txt | 4 +- .../repositoryevents/WatchEvent.started.txt | 4 +- .../nuance/item/NuanceGitHubEventItemType.php | 43 +++++++++++++ .../nuance/item/NuanceItemType.php | 2 +- 23 files changed, 165 insertions(+), 20 deletions(-) diff --git a/src/applications/nuance/github/NuanceGitHubRawEvent.php b/src/applications/nuance/github/NuanceGitHubRawEvent.php index 50ab9020ef..0e8fe44c2e 100644 --- a/src/applications/nuance/github/NuanceGitHubRawEvent.php +++ b/src/applications/nuance/github/NuanceGitHubRawEvent.php @@ -78,6 +78,68 @@ final class NuanceGitHubRawEvent extends Phobject { return $this->getRawIssueNumber(); } + + public function getID() { + $raw = $this->raw; + + $id = idx($raw, 'id'); + if ($id) { + return (int)$id; + } + + return null; + } + + public function getURI() { + $raw = $this->raw; + + if ($this->isIssueEvent()) { + if ($this->type == self::TYPE_ISSUE) { + $uri = idxv($raw, array('issue', 'html_url')); + $uri = $uri.'#event-'.$this->getID(); + } else { + $uri = idxv($raw, array('payload', 'issue', 'html_url')); + $uri = $uri.'#event-'.$this->getID(); + } + } else if ($this->isPullRequestEvent()) { + if ($this->type == self::TYPE_ISSUE) { + $uri = idxv($raw, array('issue', 'html_url')); + $uri = $uri.'#event-'.$this->getID(); + } else { + // The format of pull request events varies so we need to fish around + // a bit to find the correct URI. + $uri = idxv($raw, array('payload', 'pull_request', 'html_url')); + if (!$uri) { + $uri = idxv($raw, array('payload', 'issue', 'html_url')); + } + $uri = $uri.'#event-'.$this->getID(); + } + } else { + switch ($this->getIssueRawKind()) { + case 'PushEvent': + // These don't really have a URI since there may be multiple commits + // involved and GitHub doesn't bundle the push as an object on its + // own. Just try to find the URI for the log. The API also does + // not return any HTML URI for these events. + + $head = idxv($raw, array('payload', 'head')); + if ($head === null) { + return null; + } + + $repo = $this->getRepositoryFullRawName(); + return "https://github.com/{$repo}/commits/{$head}"; + case 'WatchEvent': + // These have no reasonable URI. + return null; + default: + return null; + } + } + + return $uri; + } + private function getRepositoryFullRawName() { $raw = $this->raw; diff --git a/src/applications/nuance/github/__tests__/NuanceGitHubRawEventTestCase.php b/src/applications/nuance/github/__tests__/NuanceGitHubRawEventTestCase.php index f956079b6a..5ce199cb8c 100644 --- a/src/applications/nuance/github/__tests__/NuanceGitHubRawEventTestCase.php +++ b/src/applications/nuance/github/__tests__/NuanceGitHubRawEventTestCase.php @@ -48,6 +48,8 @@ final class NuanceGitHubRawEventTestCase 'is.pull' => $event->isPullRequestEvent(), 'issue.number' => $event->getIssueNumber(), 'pull.number' => $event->getPullRequestNumber(), + 'id' => $event->getID(), + 'uri' => $event->getURI(), ); // Only verify the keys which are actually present in the test. This diff --git a/src/applications/nuance/github/__tests__/issueevents/assigned.txt b/src/applications/nuance/github/__tests__/issueevents/assigned.txt index bdf5046e00..7c1c89435f 100644 --- a/src/applications/nuance/github/__tests__/issueevents/assigned.txt +++ b/src/applications/nuance/github/__tests__/issueevents/assigned.txt @@ -110,5 +110,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583217900, + "uri": "https://github.com/epriestley/poems/issues/1#event-583217900" } diff --git a/src/applications/nuance/github/__tests__/issueevents/closed.txt b/src/applications/nuance/github/__tests__/issueevents/closed.txt index 7651d07172..e13c9a1ffd 100644 --- a/src/applications/nuance/github/__tests__/issueevents/closed.txt +++ b/src/applications/nuance/github/__tests__/issueevents/closed.txt @@ -72,5 +72,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583218864, + "uri": "https://github.com/epriestley/poems/issues/1#event-583218864" } diff --git a/src/applications/nuance/github/__tests__/issueevents/demilestoned.txt b/src/applications/nuance/github/__tests__/issueevents/demilestoned.txt index bbde0e7d24..1c9bab5725 100644 --- a/src/applications/nuance/github/__tests__/issueevents/demilestoned.txt +++ b/src/applications/nuance/github/__tests__/issueevents/demilestoned.txt @@ -75,5 +75,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583218613, + "uri": "https://github.com/epriestley/poems/issues/1#event-583218613" } diff --git a/src/applications/nuance/github/__tests__/issueevents/labeled.txt b/src/applications/nuance/github/__tests__/issueevents/labeled.txt index bf41262ac4..92cd7cd4f0 100644 --- a/src/applications/nuance/github/__tests__/issueevents/labeled.txt +++ b/src/applications/nuance/github/__tests__/issueevents/labeled.txt @@ -76,5 +76,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583217784, + "uri": "https://github.com/epriestley/poems/issues/1#event-583217784" } diff --git a/src/applications/nuance/github/__tests__/issueevents/locked.txt b/src/applications/nuance/github/__tests__/issueevents/locked.txt index 440eec0d95..536d95af8e 100644 --- a/src/applications/nuance/github/__tests__/issueevents/locked.txt +++ b/src/applications/nuance/github/__tests__/issueevents/locked.txt @@ -72,5 +72,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583218006, + "uri": "https://github.com/epriestley/poems/issues/1#event-583218006" } diff --git a/src/applications/nuance/github/__tests__/issueevents/milestoned.txt b/src/applications/nuance/github/__tests__/issueevents/milestoned.txt index e8b32b1113..748beddda9 100644 --- a/src/applications/nuance/github/__tests__/issueevents/milestoned.txt +++ b/src/applications/nuance/github/__tests__/issueevents/milestoned.txt @@ -75,5 +75,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583217866, + "uri": "https://github.com/epriestley/poems/issues/1#event-583217866" } diff --git a/src/applications/nuance/github/__tests__/issueevents/renamed.txt b/src/applications/nuance/github/__tests__/issueevents/renamed.txt index 0cbbd1ebb8..e4a4614ebb 100644 --- a/src/applications/nuance/github/__tests__/issueevents/renamed.txt +++ b/src/applications/nuance/github/__tests__/issueevents/renamed.txt @@ -76,5 +76,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583218162, + "uri": "https://github.com/epriestley/poems/issues/1#event-583218162" } diff --git a/src/applications/nuance/github/__tests__/issueevents/reopened.txt b/src/applications/nuance/github/__tests__/issueevents/reopened.txt index bc778728d8..baab332450 100644 --- a/src/applications/nuance/github/__tests__/issueevents/reopened.txt +++ b/src/applications/nuance/github/__tests__/issueevents/reopened.txt @@ -72,5 +72,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583218814, + "uri": "https://github.com/epriestley/poems/issues/1#event-583218814" } diff --git a/src/applications/nuance/github/__tests__/issueevents/unassigned.txt b/src/applications/nuance/github/__tests__/issueevents/unassigned.txt index bc8b9e1df9..43c610b576 100644 --- a/src/applications/nuance/github/__tests__/issueevents/unassigned.txt +++ b/src/applications/nuance/github/__tests__/issueevents/unassigned.txt @@ -110,5 +110,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583218511, + "uri": "https://github.com/epriestley/poems/issues/1#event-583218511" } diff --git a/src/applications/nuance/github/__tests__/issueevents/unlabeled.txt b/src/applications/nuance/github/__tests__/issueevents/unlabeled.txt index e3435605e0..8d40ba5702 100644 --- a/src/applications/nuance/github/__tests__/issueevents/unlabeled.txt +++ b/src/applications/nuance/github/__tests__/issueevents/unlabeled.txt @@ -76,5 +76,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583218703, + "uri": "https://github.com/epriestley/poems/issues/1#event-583218703" } diff --git a/src/applications/nuance/github/__tests__/issueevents/unlocked.txt b/src/applications/nuance/github/__tests__/issueevents/unlocked.txt index e59ba6e93f..080fbd79e8 100644 --- a/src/applications/nuance/github/__tests__/issueevents/unlocked.txt +++ b/src/applications/nuance/github/__tests__/issueevents/unlocked.txt @@ -72,5 +72,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 583218062, + "uri": "https://github.com/epriestley/poems/issues/1#event-583218062" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt index 0068f7c092..9c784f21be 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt @@ -157,5 +157,7 @@ "is.issue": false, "is.pull": true, "issue.number": null, - "pull.number": 2 + "pull.number": 2, + "id": 3740938746, + "uri": "https://github.com/epriestley/poems/pull/2#event-3740938746" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt index c6fa5ccb54..b5034ef453 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt @@ -94,5 +94,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 3733510485, + "uri": "https://github.com/epriestley/poems/issues/1#event-3733510485" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.closed.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.closed.txt index 8373e2ee52..78fc81be95 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.closed.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.closed.txt @@ -66,5 +66,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 3740905151, + "uri": "https://github.com/epriestley/poems/issues/1#event-3740905151" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.opened.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.opened.txt index 91068727e7..7ba07a591b 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.opened.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.opened.txt @@ -66,5 +66,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 3733509737, + "uri": "https://github.com/epriestley/poems/issues/1#event-3733509737" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.reopened.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.reopened.txt index 6ab81e1028..797eb84078 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.reopened.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.reopened.txt @@ -66,5 +66,7 @@ "repository.name.full": "epriestley/poems", "is.issue": true, "is.pull": false, - "issue.number": 1 + "issue.number": 1, + "id": 3740908680, + "uri": "https://github.com/epriestley/poems/issues/1#event-3740908680" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/PullRequestEvent.opened.txt b/src/applications/nuance/github/__tests__/repositoryevents/PullRequestEvent.opened.txt index 848ed63afe..c2f892f090 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/PullRequestEvent.opened.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/PullRequestEvent.opened.txt @@ -330,5 +330,7 @@ "is.issue": false, "is.pull": true, "issue.number": null, - "pull.number": 2 + "pull.number": 2, + "id": 3740936638, + "uri": "https://github.com/epriestley/poems/pull/2#event-3740936638" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/PushEvent.txt b/src/applications/nuance/github/__tests__/repositoryevents/PushEvent.txt index f36fed2f52..d7c1b5ecad 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/PushEvent.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/PushEvent.txt @@ -41,5 +41,7 @@ "repository.name.full": "epriestley/poems", "is.issue": false, "is.pull": false, - "issue.number": null + "issue.number": null, + "id": 3498724127, + "uri": "https://github.com/epriestley/poems/commits/c829132d37c4c1da80d319942a5a1e500632b52f" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/WatchEvent.started.txt b/src/applications/nuance/github/__tests__/repositoryevents/WatchEvent.started.txt index 7cc6ed8164..c65a5ee771 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/WatchEvent.started.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/WatchEvent.started.txt @@ -25,5 +25,7 @@ "is.issue": false, "is.pull": false, "issue.number": null, - "pull.number": null + "pull.number": null, + "id": 3740950917, + "uri": null } diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php index cec3d366ec..dd52dc1c15 100644 --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -186,4 +186,47 @@ final class NuanceGitHubEventItemType return null; } + protected function newItemView(NuanceItem $item) { + $content = array(); + + $content[] = $this->newGitHubEventItemPropertyBox($item); + + return $content; + } + + private function newGitHubEventItemPropertyBox($item) { + $viewer = $this->getViewer(); + + $property_list = id(new PHUIPropertyListView()) + ->setViewer($viewer); + + $event = $this->newRawEvent($item); + + $property_list->addProperty( + pht('GitHub Event ID'), + $event->getID()); + + $event_uri = $event->getURI(); + if ($event_uri && PhabricatorEnv::isValidRemoteURIForLink($event_uri)) { + $event_uri = phutil_tag( + 'a', + array( + 'href' => $event_uri, + ), + $event_uri); + } + + if ($event_uri) { + $property_list->addProperty( + pht('GitHub Event URI'), + $event_uri); + } + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Event Properties')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($property_list); + } + + } diff --git a/src/applications/nuance/item/NuanceItemType.php b/src/applications/nuance/item/NuanceItemType.php index 2f348dee1f..14b2fc5324 100644 --- a/src/applications/nuance/item/NuanceItemType.php +++ b/src/applications/nuance/item/NuanceItemType.php @@ -32,7 +32,7 @@ abstract class NuanceItemType return $this->newItemView($item); } - protected function newItemView() { + protected function newItemView(NuanceItem $item) { return null; } From 1885c4e03bb62bc71c119299010ca4b84dcdd15b Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Mar 2016 06:51:22 -0700 Subject: [PATCH 16/31] Add an ItemCommand queue to Nuance Summary: Ref T10537. Generally, when users interact with Nuance items we'll dump a command into a queue and apply it in the background. This avoids race conditions with multiple users interacting with an item, which Nuance is more subject to than other applications because it has an import/external component. The "sync" command doesn't actually do anything yet. Test Plan: {F1186365} Reviewers: chad Reviewed By: chad Subscribers: Luke081515.2 Maniphest Tasks: T10537 Differential Revision: https://secure.phabricator.com/D15506 --- .../20160322.nuance.01.itemcommand.sql | 8 +++ src/__phutil_library_map__.php | 7 +++ .../controller/NuanceItemViewController.php | 45 ++++++++++++++- .../nuance/editor/NuanceItemEditor.php | 7 +++ .../nuance/item/NuanceGitHubEventItemType.php | 14 +++++ .../nuance/item/NuanceItemType.php | 39 +++++++++++++ .../nuance/query/NuanceItemCommandQuery.php | 47 ++++++++++++++++ .../nuance/storage/NuanceItem.php | 17 ++++++ .../nuance/storage/NuanceItemCommand.php | 55 +++++++++++++++++++ .../nuance/storage/NuanceItemTransaction.php | 7 +++ .../nuance/worker/NuanceItemUpdateWorker.php | 19 +++++++ 11 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 resources/sql/autopatches/20160322.nuance.01.itemcommand.sql create mode 100644 src/applications/nuance/query/NuanceItemCommandQuery.php create mode 100644 src/applications/nuance/storage/NuanceItemCommand.php diff --git a/resources/sql/autopatches/20160322.nuance.01.itemcommand.sql b/resources/sql/autopatches/20160322.nuance.01.itemcommand.sql new file mode 100644 index 0000000000..f356db4947 --- /dev/null +++ b/resources/sql/autopatches/20160322.nuance.01.itemcommand.sql @@ -0,0 +1,8 @@ +CREATE TABLE {$NAMESPACE}_nuance.nuance_itemcommand ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + itemPHID VARBINARY(64) NOT NULL, + authorPHID VARBINARY(64) NOT NULL, + command VARCHAR(64) NOT NULL COLLATE {$COLLATE_TEXT}, + parameters LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT}, + KEY `key_item` (itemPHID) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 92290ecd77..3932c6a4c1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1440,6 +1440,8 @@ phutil_register_library_map(array( 'NuanceImportCursorPHIDType' => 'applications/nuance/phid/NuanceImportCursorPHIDType.php', 'NuanceItem' => 'applications/nuance/storage/NuanceItem.php', 'NuanceItemActionController' => 'applications/nuance/controller/NuanceItemActionController.php', + 'NuanceItemCommand' => 'applications/nuance/storage/NuanceItemCommand.php', + 'NuanceItemCommandQuery' => 'applications/nuance/query/NuanceItemCommandQuery.php', 'NuanceItemController' => 'applications/nuance/controller/NuanceItemController.php', 'NuanceItemEditor' => 'applications/nuance/editor/NuanceItemEditor.php', 'NuanceItemListController' => 'applications/nuance/controller/NuanceItemListController.php', @@ -5728,6 +5730,11 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', ), 'NuanceItemActionController' => 'NuanceController', + 'NuanceItemCommand' => array( + 'NuanceDAO', + 'PhabricatorPolicyInterface', + ), + 'NuanceItemCommandQuery' => 'NuanceQuery', 'NuanceItemController' => 'NuanceController', 'NuanceItemEditor' => 'PhabricatorApplicationTransactionEditor', 'NuanceItemListController' => 'NuanceItemController', diff --git a/src/applications/nuance/controller/NuanceItemViewController.php b/src/applications/nuance/controller/NuanceItemViewController.php index e8e3967545..091ade2d6b 100644 --- a/src/applications/nuance/controller/NuanceItemViewController.php +++ b/src/applications/nuance/controller/NuanceItemViewController.php @@ -26,6 +26,17 @@ final class NuanceItemViewController extends NuanceController { $curtain = $this->buildCurtain($item); $content = $this->buildContent($item); + $commands = $this->buildCommands($item); + + $timeline = $this->buildTransactionTimeline( + $item, + new NuanceItemTransactionQuery()); + + $main = array( + $commands, + $content, + $timeline, + ); $header = id(new PHUIHeaderView()) ->setHeader($name); @@ -33,7 +44,7 @@ final class NuanceItemViewController extends NuanceController { $view = id(new PHUITwoColumnView()) ->setHeader($header) ->setCurtain($curtain) - ->setMainColumn($content); + ->setMainColumn($main); return $this->newPage() ->setTitle($title) @@ -76,4 +87,36 @@ final class NuanceItemViewController extends NuanceController { return $impl->buildItemView($item); } + private function buildCommands(NuanceItem $item) { + $viewer = $this->getViewer(); + + $commands = id(new NuanceItemCommandQuery()) + ->setViewer($viewer) + ->withItemPHIDs(array($item->getPHID())) + ->execute(); + $commands = msort($commands, 'getID'); + + if (!$commands) { + return null; + } + + $rows = array(); + foreach ($commands as $command) { + $rows[] = array( + $command->getCommand(), + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Command'), + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Pending Commands')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setTable($table); + } + } diff --git a/src/applications/nuance/editor/NuanceItemEditor.php b/src/applications/nuance/editor/NuanceItemEditor.php index a331a65196..45288052c2 100644 --- a/src/applications/nuance/editor/NuanceItemEditor.php +++ b/src/applications/nuance/editor/NuanceItemEditor.php @@ -19,6 +19,7 @@ final class NuanceItemEditor $types[] = NuanceItemTransaction::TYPE_REQUESTOR; $types[] = NuanceItemTransaction::TYPE_PROPERTY; $types[] = NuanceItemTransaction::TYPE_QUEUE; + $types[] = NuanceItemTransaction::TYPE_COMMAND; $types[] = PhabricatorTransactions::TYPE_EDGE; $types[] = PhabricatorTransactions::TYPE_COMMENT; @@ -45,6 +46,8 @@ final class NuanceItemEditor $key = $xaction->getMetadataValue( NuanceItemTransaction::PROPERTY_KEY); return $object->getNuanceProperty($key); + case NuanceItemTransaction::TYPE_COMMAND: + return null; } return parent::getCustomTransactionOldValue($object, $xaction); @@ -60,6 +63,7 @@ final class NuanceItemEditor case NuanceItemTransaction::TYPE_OWNER: case NuanceItemTransaction::TYPE_PROPERTY: case NuanceItemTransaction::TYPE_QUEUE: + case NuanceItemTransaction::TYPE_COMMAND: return $xaction->getNewValue(); } @@ -88,6 +92,8 @@ final class NuanceItemEditor NuanceItemTransaction::PROPERTY_KEY); $object->setNuanceProperty($key, $xaction->getNewValue()); break; + case NuanceItemTransaction::TYPE_COMMAND: + break; } } @@ -101,6 +107,7 @@ final class NuanceItemEditor case NuanceItemTransaction::TYPE_OWNER: case NuanceItemTransaction::TYPE_PROPERTY: case NuanceItemTransaction::TYPE_QUEUE: + case NuanceItemTransaction::TYPE_COMMAND: return; } diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php index dd52dc1c15..3ee2e91489 100644 --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -147,6 +147,12 @@ final class NuanceGitHubEventItemType public function getItemActions(NuanceItem $item) { $actions = array(); + $actions[] = $this->newItemAction($item, 'sync') + ->setName(pht('Import to Maniphest')) + ->setIcon('fa-anchor') + ->setWorkflow(true) + ->setRenderAsForm(true); + $actions[] = $this->newItemAction($item, 'raw') ->setName(pht('View Raw Event')) ->setWorkflow(true) @@ -156,6 +162,7 @@ final class NuanceGitHubEventItemType } protected function handleAction(NuanceItem $item, $action) { + $viewer = $this->getViewer(); $controller = $this->getController(); switch ($action) { @@ -181,6 +188,9 @@ final class NuanceGitHubEventItemType ->setTitle(pht('GitHub Raw Event')) ->appendForm($form) ->addCancelButton($item->getURI(), pht('Done')); + case 'sync': + $item->issueCommand($viewer->getPHID(), $action); + return id(new AphrontRedirectResponse())->setURI($item->getURI()); } return null; @@ -228,5 +238,9 @@ final class NuanceGitHubEventItemType ->appendChild($property_list); } + protected function handleCommand(NuanceItem $item, $action) { + return null; + } + } diff --git a/src/applications/nuance/item/NuanceItemType.php b/src/applications/nuance/item/NuanceItemType.php index 14b2fc5324..144d6e86f5 100644 --- a/src/applications/nuance/item/NuanceItemType.php +++ b/src/applications/nuance/item/NuanceItemType.php @@ -96,4 +96,43 @@ abstract class NuanceItemType return null; } + final public function applyCommand( + NuanceItem $item, + NuanceItemCommand $command) { + + $result = $this->handleCommand($item, $command); + + if ($result === null) { + return; + } + + $xaction = id(new NuanceItemTransaction()) + ->setTransactionType(NuanceItemTransaction::TYPE_COMMAND) + ->setNewValue( + array( + 'command' => $command->getCommand(), + 'parameters' => $command->getParameters(), + 'result' => $result, + )); + + $viewer = $this->getViewer(); + + // TODO: Maybe preserve the actor's original content source? + $source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_DAEMON, + array()); + + $editor = id(new NuanceItemEditor()) + ->setActor($viewer) + ->setActingAsPHID($command->getAuthorPHID()) + ->setContentSource($source) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($item, array($xaction)); + } + + protected function handleCommand(NuanceItem $item, $action) { + return null; + } + } diff --git a/src/applications/nuance/query/NuanceItemCommandQuery.php b/src/applications/nuance/query/NuanceItemCommandQuery.php new file mode 100644 index 0000000000..cb20610187 --- /dev/null +++ b/src/applications/nuance/query/NuanceItemCommandQuery.php @@ -0,0 +1,47 @@ +ids = $ids; + return $this; + } + + public function withItemPHIDs(array $item_phids) { + $this->itemPHIDs = $item_phids; + return $this; + } + + public function newResultObject() { + return new NuanceItemCommand(); + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->ids !== null) { + $where[] = qsprintf( + $conn, + 'id IN (%Ld)', + $this->ids); + } + + if ($this->itemPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'itemPHID IN (%Ls)', + $this->itemPHIDs); + } + + return $where; + } + +} diff --git a/src/applications/nuance/storage/NuanceItem.php b/src/applications/nuance/storage/NuanceItem.php index a83db3ec70..a21a5443c2 100644 --- a/src/applications/nuance/storage/NuanceItem.php +++ b/src/applications/nuance/storage/NuanceItem.php @@ -154,6 +154,23 @@ final class NuanceItem )); } + public function issueCommand( + $author_phid, + $command, + array $parameters = array()) { + + $command = id(NuanceItemCommand::initializeNewCommand()) + ->setItemPHID($this->getPHID()) + ->setAuthorPHID($author_phid) + ->setCommand($command) + ->setParameters($parameters) + ->save(); + + $this->scheduleUpdate(); + + return $this; + } + public function getImplementation() { return $this->assertAttached($this->implementation); } diff --git a/src/applications/nuance/storage/NuanceItemCommand.php b/src/applications/nuance/storage/NuanceItemCommand.php new file mode 100644 index 0000000000..94409dd89a --- /dev/null +++ b/src/applications/nuance/storage/NuanceItemCommand.php @@ -0,0 +1,55 @@ + false, + self::CONFIG_SERIALIZATION => array( + 'parameters' => self::SERIALIZATION_JSON, + ), + self::CONFIG_COLUMN_SCHEMA => array( + 'command' => 'text64', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_item' => array( + 'columns' => array('itemPHID'), + ), + ), + ) + parent::getConfiguration(); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + return PhabricatorPolicies::getMostOpenPolicy(); + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + + public function describeAutomaticCapability($capability) { + return null; + } + +} diff --git a/src/applications/nuance/storage/NuanceItemTransaction.php b/src/applications/nuance/storage/NuanceItemTransaction.php index 183596402f..77471fcdb9 100644 --- a/src/applications/nuance/storage/NuanceItemTransaction.php +++ b/src/applications/nuance/storage/NuanceItemTransaction.php @@ -10,6 +10,7 @@ final class NuanceItemTransaction const TYPE_SOURCE = 'nuance.item.source'; const TYPE_PROPERTY = 'nuance.item.property'; const TYPE_QUEUE = 'nuance.item.queue'; + const TYPE_COMMAND = 'nuance.item.command'; public function getApplicationTransactionType() { return NuanceItemPHIDType::TYPECONST; @@ -65,6 +66,12 @@ final class NuanceItemTransaction '%s routed this item to the %s queue.', $this->renderHandleLink($author_phid), $this->renderHandleLink($new)); + case self::TYPE_COMMAND: + // TODO: Give item types a chance to render this properly. + return pht( + '%s applied command "%s" to this item.', + $this->renderHandleLink($author_phid), + idx($new, 'command')); } return parent::getTitle(); diff --git a/src/applications/nuance/worker/NuanceItemUpdateWorker.php b/src/applications/nuance/worker/NuanceItemUpdateWorker.php index c8c65e33ea..57be20edae 100644 --- a/src/applications/nuance/worker/NuanceItemUpdateWorker.php +++ b/src/applications/nuance/worker/NuanceItemUpdateWorker.php @@ -15,6 +15,7 @@ final class NuanceItemUpdateWorker $item = $this->loadItem($item_phid); $this->updateItem($item); $this->routeItem($item); + $this->applyCommands($item); } catch (Exception $ex) { $lock->unlock(); throw $ex; @@ -51,4 +52,22 @@ final class NuanceItemUpdateWorker ->save(); } + private function applyCommands(NuanceItem $item) { + $viewer = $this->getViewer(); + + $impl = $item->getImplementation(); + $impl->setViewer($viewer); + + $commands = id(new NuanceItemCommandQuery()) + ->setViewer($viewer) + ->withItemPHIDs(array($item->getPHID())) + ->execute(); + $commands = msort($commands, 'getID'); + + foreach ($commands as $command) { + $impl->applyCommand($item, $command); + $command->delete(); + } + } + } From dac07921f71a72c2f6c17f8a51f923f82edc4ffc Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Mar 2016 15:20:33 -0700 Subject: [PATCH 17/31] Pick better GitHub URIs for comment events Summary: Ref T10538. Boundless joy. Test Plan: Unit tests. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10538 Differential Revision: https://secure.phabricator.com/D15510 --- .../nuance/github/NuanceGitHubRawEvent.php | 25 +++++++++++-------- .../IssueCommentEvent.created.pull.txt | 2 +- .../IssueCommentEvent.created.txt | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/applications/nuance/github/NuanceGitHubRawEvent.php b/src/applications/nuance/github/NuanceGitHubRawEvent.php index 0e8fe44c2e..9e170bf092 100644 --- a/src/applications/nuance/github/NuanceGitHubRawEvent.php +++ b/src/applications/nuance/github/NuanceGitHubRawEvent.php @@ -93,15 +93,7 @@ final class NuanceGitHubRawEvent extends Phobject { public function getURI() { $raw = $this->raw; - if ($this->isIssueEvent()) { - if ($this->type == self::TYPE_ISSUE) { - $uri = idxv($raw, array('issue', 'html_url')); - $uri = $uri.'#event-'.$this->getID(); - } else { - $uri = idxv($raw, array('payload', 'issue', 'html_url')); - $uri = $uri.'#event-'.$this->getID(); - } - } else if ($this->isPullRequestEvent()) { + if ($this->isIssueEvent() || $this->isPullRequestEvent()) { if ($this->type == self::TYPE_ISSUE) { $uri = idxv($raw, array('issue', 'html_url')); $uri = $uri.'#event-'.$this->getID(); @@ -109,10 +101,23 @@ final class NuanceGitHubRawEvent extends Phobject { // The format of pull request events varies so we need to fish around // a bit to find the correct URI. $uri = idxv($raw, array('payload', 'pull_request', 'html_url')); + $need_anchor = true; + + // For comments, we get a different anchor to link to the comment. In + // this case, the URI comes with an anchor already. + if (!$uri) { + $uri = idxv($raw, array('payload', 'comment', 'html_url')); + $need_anchor = false; + } + if (!$uri) { $uri = idxv($raw, array('payload', 'issue', 'html_url')); + $need_anchor = true; + } + + if ($need_anchor) { + $uri = $uri.'#event-'.$this->getID(); } - $uri = $uri.'#event-'.$this->getID(); } } else { switch ($this->getIssueRawKind()) { diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt index 9c784f21be..7b3e7d3708 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt @@ -159,5 +159,5 @@ "issue.number": null, "pull.number": 2, "id": 3740938746, - "uri": "https://github.com/epriestley/poems/pull/2#event-3740938746" + "uri": "https://github.com/epriestley/poems/pull/2#issuecomment-194282800" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt index b5034ef453..acb40f0971 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt @@ -96,5 +96,5 @@ "is.pull": false, "issue.number": 1, "id": 3733510485, - "uri": "https://github.com/epriestley/poems/issues/1#event-3733510485" + "uri": "https://github.com/epriestley/poems/issues/1#issuecomment-193528669" } From e3f89279f9126e09f23b1d9d7d0488751830aa83 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Mar 2016 18:51:39 -0700 Subject: [PATCH 18/31] Attach credential impelementations when initializing new credentials Summary: Fixes T10651. Test Plan: Created a new API token credential. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10651 Differential Revision: https://secure.phabricator.com/D15512 --- .../controller/PassphraseCredentialEditController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/applications/passphrase/controller/PassphraseCredentialEditController.php b/src/applications/passphrase/controller/PassphraseCredentialEditController.php index a749e82af3..72c21a9e39 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialEditController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialEditController.php @@ -36,7 +36,8 @@ final class PassphraseCredentialEditController extends PassphraseController { $credential = PassphraseCredential::initializeNewCredential($viewer) ->setCredentialType($type->getCredentialType()) - ->setProvidesType($type->getProvidesType()); + ->setProvidesType($type->getProvidesType()) + ->attachImplementation($type); $is_new = true; From 881785aba4bc05f613535565ce7b6b105a2582a1 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 23 Mar 2016 11:03:30 -0700 Subject: [PATCH 19/31] Update Phortune for two column, spruce up UI Summary: Moves everything I could find in Phortune to new UI layouts. Test Plan: Tested every page I could get two, unclear how to test subscriptions. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D15513 --- resources/celerity/map.php | 10 +- .../PhortuneAccountEditController.php | 22 ++- .../PhortuneAccountListController.php | 31 ++-- .../PhortuneAccountViewController.php | 142 +++++++++++------- .../PhortuneCartCheckoutController.php | 21 ++- .../controller/PhortuneCartController.php | 1 + .../controller/PhortuneCartViewController.php | 77 +++++----- .../PhortuneMerchantEditController.php | 26 +++- ...hortuneMerchantInvoiceCreateController.php | 22 ++- .../PhortuneMerchantViewController.php | 96 ++++++++---- .../PhortunePaymentMethodCreateController.php | 23 ++- .../PhortunePaymentMethodEditController.php | 23 ++- .../PhortuneProductListController.php | 30 +++- .../PhortuneProductViewController.php | 26 ++-- .../PhortuneProviderEditController.php | 45 ++++-- .../PhortuneSubscriptionEditController.php | 22 ++- .../PhortuneSubscriptionViewController.php | 40 ++--- .../query/PhortuneMerchantSearchEngine.php | 5 +- src/view/phui/PHUIObjectItemListView.php | 9 ++ .../css/phui/phui-object-item-list-view.css | 41 +++++ .../rsrc/css/phui/phui-two-column-view.css | 5 + 21 files changed, 473 insertions(+), 244 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index d5d2616f0c..a89bebd270 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'a93de192', + 'core.pkg.css' => 'befbf333', 'core.pkg.js' => '7d8faf57', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '7ba78475', @@ -145,7 +145,7 @@ return array( 'rsrc/css/phui/phui-info-view.css' => '28efab79', 'rsrc/css/phui/phui-list.css' => '9da2aa00', 'rsrc/css/phui/phui-object-box.css' => '6b487c57', - 'rsrc/css/phui/phui-object-item-list-view.css' => '18b2ce8e', + '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' => '7e92a89a', @@ -156,7 +156,7 @@ return array( 'rsrc/css/phui/phui-status.css' => '37309046', 'rsrc/css/phui/phui-tag-view.css' => '6bbd83e2', 'rsrc/css/phui/phui-timeline-view.css' => 'a0173eba', - 'rsrc/css/phui/phui-two-column-view.css' => '37d704f3', + 'rsrc/css/phui/phui-two-column-view.css' => '9c43b599', 'rsrc/css/phui/workboards/phui-workboard-color.css' => 'ac6fe6a7', 'rsrc/css/phui/workboards/phui-workboard.css' => 'e6d89647', 'rsrc/css/phui/workboards/phui-workcard.css' => '3646fb96', @@ -834,7 +834,7 @@ return array( 'phui-inline-comment-view-css' => '5953c28e', 'phui-list-view-css' => '9da2aa00', 'phui-object-box-css' => '6b487c57', - 'phui-object-item-list-view-css' => '18b2ce8e', + 'phui-object-item-list-view-css' => '8d99e42b', 'phui-pager-css' => 'bea33d23', 'phui-pinboard-view-css' => '2495140e', 'phui-profile-menu-css' => '7e92a89a', @@ -846,7 +846,7 @@ return array( 'phui-tag-view-css' => '6bbd83e2', 'phui-theme-css' => '027ba77e', 'phui-timeline-view-css' => 'a0173eba', - 'phui-two-column-view-css' => '37d704f3', + 'phui-two-column-view-css' => '9c43b599', 'phui-workboard-color-css' => 'ac6fe6a7', 'phui-workboard-view-css' => 'e6d89647', 'phui-workcard-view-css' => '3646fb96', diff --git a/src/applications/phortune/controller/PhortuneAccountEditController.php b/src/applications/phortune/controller/PhortuneAccountEditController.php index f51f1293d9..104623219b 100644 --- a/src/applications/phortune/controller/PhortuneAccountEditController.php +++ b/src/applications/phortune/controller/PhortuneAccountEditController.php @@ -74,6 +74,7 @@ final class PhortuneAccountEditController extends PhortuneController { } $crumbs = $this->buildApplicationCrumbs(); + $crumbs->setBorder(true); if ($is_new) { $cancel_uri = $this->getApplicationURI('account/'); @@ -112,18 +113,25 @@ final class PhortuneAccountEditController extends PhortuneController { ->addCancelButton($cancel_uri)); $box = id(new PHUIObjectBoxView()) - ->setHeaderText($title) + ->setHeaderText(pht('Account')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setValidationException($validation_exception) ->setForm($form); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setHeaderIcon('fa-pencil'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); } } diff --git a/src/applications/phortune/controller/PhortuneAccountListController.php b/src/applications/phortune/controller/PhortuneAccountListController.php index 9b095186cc..4b7521ceaa 100644 --- a/src/applications/phortune/controller/PhortuneAccountListController.php +++ b/src/applications/phortune/controller/PhortuneAccountListController.php @@ -24,6 +24,7 @@ final class PhortuneAccountListController extends PhortuneController { $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('Accounts')); + $crumbs->setBorder(true); $payment_list = id(new PHUIObjectItemListView()) ->setUser($viewer) @@ -34,10 +35,11 @@ final class PhortuneAccountListController extends PhortuneController { foreach ($accounts as $account) { $item = id(new PHUIObjectItemView()) - ->setObjectName(pht('Account %d', $account->getID())) + ->setSubhead(pht('Account %d', $account->getID())) ->setHeader($account->getName()) ->setHref($this->getApplicationURI($account->getID().'/')) - ->setObject($account); + ->setObject($account) + ->setIcon('fa-credit-card'); $payment_list->addItem($item); } @@ -53,6 +55,7 @@ final class PhortuneAccountListController extends PhortuneController { $payment_box = id(new PHUIObjectBoxView()) ->setHeader($payment_header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($payment_list); $merchant_list = id(new PHUIObjectItemListView()) @@ -64,10 +67,11 @@ final class PhortuneAccountListController extends PhortuneController { foreach ($merchants as $merchant) { $item = id(new PHUIObjectItemView()) - ->setObjectName(pht('Merchant %d', $merchant->getID())) + ->setSubhead(pht('Merchant %d', $merchant->getID())) ->setHeader($merchant->getName()) ->setHref($this->getApplicationURI('/merchant/'.$merchant->getID().'/')) - ->setObject($merchant); + ->setObject($merchant) + ->setIcon('fa-bank'); $merchant_list->addItem($item); } @@ -83,17 +87,24 @@ final class PhortuneAccountListController extends PhortuneController { $merchant_box = id(new PHUIObjectBoxView()) ->setHeader($merchant_header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($merchant_list); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Accounts')); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $payment_box, $merchant_box, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } } diff --git a/src/applications/phortune/controller/PhortuneAccountViewController.php b/src/applications/phortune/controller/PhortuneAccountViewController.php index 001db3bebc..97ea1beea8 100644 --- a/src/applications/phortune/controller/PhortuneAccountViewController.php +++ b/src/applications/phortune/controller/PhortuneAccountViewController.php @@ -36,29 +36,61 @@ final class PhortuneAccountViewController extends PhortuneController { $crumbs = $this->buildApplicationCrumbs(); $this->addAccountCrumb($crumbs, $account, $link = false); + $crumbs->setBorder(true); $header = id(new PHUIHeaderView()) - ->setHeader($title); + ->setHeader($title) + ->setHeaderIcon('fa-credit-card'); + + $curtain = $this->buildCurtainView($account, $invoices); + $invoices = $this->buildInvoicesSection($account, $invoices); + $purchase_history = $this->buildPurchaseHistorySection($account); + $charge_history = $this->buildChargeHistorySection($account); + $subscriptions = $this->buildSubscriptionsSection($account); + $payment_methods = $this->buildPaymentMethodsSection($account); + + $timeline = $this->buildTransactionTimeline( + $account, + new PhortuneAccountTransactionQuery()); + $timeline->setShouldTerminate(true); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $invoices, + $purchase_history, + $charge_history, + $subscriptions, + $payment_methods, + $timeline, + )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + + } + + private function buildCurtainView(PhortuneAccount $account, $invoices) { + $viewer = $this->getViewer(); + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $account, + PhabricatorPolicyCapability::CAN_EDIT); $edit_uri = $this->getApplicationURI('account/edit/'.$account->getID().'/'); - $actions = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Edit Account')) - ->setIcon('fa-pencil') - ->setHref($edit_uri) - ->setDisabled(!$can_edit) - ->setWorkflow(!$can_edit)); - - $properties = id(new PHUIPropertyListView()) - ->setObject($account) - ->setUser($viewer); - - $properties->addProperty( - pht('Members'), - $viewer->renderHandleList($account->getMemberPHIDs())); + $curtain = $this->newCurtainView($account); + $curtain->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Edit Account')) + ->setIcon('fa-pencil') + ->setHref($edit_uri) + ->setDisabled(!$can_edit) + ->setWorkflow(!$can_edit)); $status_items = $this->getStatusItemsForAccount($account, $invoices); $status_view = new PHUIStatusListView(); @@ -72,46 +104,39 @@ final class PhortuneAccountViewController extends PhortuneController { ->setTarget(idx($item, 'target')) ->setNote(idx($item, 'note'))); } - $properties->addProperty( - pht('Status'), - $status_view); - $properties->setActionList($actions); + $member_phids = $account->getMemberPHIDs(); + $handles = $viewer->loadHandles($member_phids); - $invoices = $this->buildInvoicesSection($account, $invoices); - $purchase_history = $this->buildPurchaseHistorySection($account); - $charge_history = $this->buildChargeHistorySection($account); - $subscriptions = $this->buildSubscriptionsSection($account); - $payment_methods = $this->buildPaymentMethodsSection($account); + $member_list = id(new PHUIObjectItemListView()) + ->setSimple(true); - $timeline = $this->buildTransactionTimeline( - $account, - new PhortuneAccountTransactionQuery()); - $timeline->setShouldTerminate(true); + foreach ($member_phids as $member_phid) { + $image_uri = $handles[$member_phid]->getImageURI(); + $image_href = $handles[$member_phid]->getURI(); + $person = $handles[$member_phid]; - $object_box = id(new PHUIObjectBoxView()) - ->setHeader($header) - ->addPropertyList($properties); + $member = id(new PHUIObjectItemView()) + ->setImageURI($image_uri) + ->setHref($image_href) + ->setHeader($person->getFullName()); - return $this->buildApplicationPage( - array( - $crumbs, - $object_box, - $invoices, - $purchase_history, - $charge_history, - $subscriptions, - $payment_methods, - $timeline, - ), - array( - 'title' => $title, - )); + $member_list->addItem($member); + } + + $curtain->newPanel() + ->setHeaderText(pht('Status')) + ->appendChild($status_view); + + $curtain->newPanel() + ->setHeaderText(pht('Members')) + ->appendChild($member_list); + + return $curtain; } private function buildPaymentMethodsSection(PhortuneAccount $account) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, @@ -179,6 +204,7 @@ final class PhortuneAccountViewController extends PhortuneController { return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($list); } @@ -186,8 +212,7 @@ final class PhortuneAccountViewController extends PhortuneController { PhortuneAccount $account, array $carts) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $phids = array(); foreach ($carts as $cart) { @@ -211,12 +236,12 @@ final class PhortuneAccountViewController extends PhortuneController { return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } private function buildPurchaseHistorySection(PhortuneAccount $account) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $carts = id(new PhortuneCartQuery()) ->setViewer($viewer) @@ -260,12 +285,12 @@ final class PhortuneAccountViewController extends PhortuneController { return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } private function buildChargeHistorySection(PhortuneAccount $account) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $charges = id(new PhortuneChargeQuery()) ->setViewer($viewer) @@ -302,12 +327,12 @@ final class PhortuneAccountViewController extends PhortuneController { return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } private function buildSubscriptionsSection(PhortuneAccount $account) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $subscriptions = id(new PhortuneSubscriptionQuery()) ->setViewer($viewer) @@ -338,6 +363,7 @@ final class PhortuneAccountViewController extends PhortuneController { return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/phortune/controller/PhortuneCartCheckoutController.php b/src/applications/phortune/controller/PhortuneCartCheckoutController.php index e22a9521fd..fdc19dae44 100644 --- a/src/applications/phortune/controller/PhortuneCartCheckoutController.php +++ b/src/applications/phortune/controller/PhortuneCartCheckoutController.php @@ -107,6 +107,7 @@ final class PhortuneCartCheckoutController $cart_box = id(new PHUIObjectBoxView()) ->setFormErrors($errors) ->setHeaderText(pht('Cart Contents')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($cart_table); $title = $cart->getName(); @@ -200,6 +201,7 @@ final class PhortuneCartCheckoutController $payment_box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Choose Payment Method')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($form) ->appendChild($provider_form); @@ -208,17 +210,24 @@ final class PhortuneCartCheckoutController $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('Checkout')); $crumbs->addTextCrumb($title); + $crumbs->setBorder(true); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setHeaderIcon('fa-shopping-cart'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $cart_box, $description_box, $payment_box, - ), - array( - 'title' => $title, )); + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } } diff --git a/src/applications/phortune/controller/PhortuneCartController.php b/src/applications/phortune/controller/PhortuneCartController.php index 628a466056..b8f926d3b2 100644 --- a/src/applications/phortune/controller/PhortuneCartController.php +++ b/src/applications/phortune/controller/PhortuneCartController.php @@ -56,6 +56,7 @@ abstract class PhortuneCartController return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Description')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($box); } diff --git a/src/applications/phortune/controller/PhortuneCartViewController.php b/src/applications/phortune/controller/PhortuneCartViewController.php index 3beaa657c6..8d85ec70fe 100644 --- a/src/applications/phortune/controller/PhortuneCartViewController.php +++ b/src/applications/phortune/controller/PhortuneCartViewController.php @@ -108,22 +108,28 @@ final class PhortuneCartViewController break; case PhortuneCart::STATUS_PURCHASED: $error_view = id(new PHUIInfoView()) - ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) + ->setSeverity(PHUIInfoView::SEVERITY_SUCCESS) ->appendChild(pht('This purchase has been completed.')); break; } - $properties = $this->buildPropertyListView($cart); - $actions = $this->buildActionListView( + if ($errors) { + $error_view = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->appendChild($errors); + } + + $details = $this->buildDetailsView($cart); + $curtain = $this->buildCurtainView( $cart, $can_edit, $authority, $resume_uri); - $properties->setActionList($actions); $header = id(new PHUIHeaderView()) ->setUser($viewer) - ->setHeader(pht('Order Detail')); + ->setHeader(pht('Order Detail')) + ->setHeaderIcon('fa-shopping-cart'); if ($cart->getStatus() == PhortuneCart::STATUS_PURCHASED) { $done_uri = $cart->getDoneURI(); @@ -138,16 +144,10 @@ final class PhortuneCartViewController } $cart_box = id(new PHUIObjectBoxView()) - ->setHeader($header) - ->addPropertyList($properties) + ->setHeaderText(pht('Cart Items')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($cart_table); - if ($errors) { - $cart_box->setFormErrors($errors); - } else if ($error_view) { - $cart_box->setInfoView($error_view); - } - $description = $this->renderCartDescription($cart); $charges = id(new PhortuneChargeQuery()) @@ -173,6 +173,7 @@ final class PhortuneCartViewController $charges = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Charges')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($charges_table); $account = $cart->getAccount(); @@ -184,6 +185,7 @@ final class PhortuneCartViewController $this->addAccountCrumb($crumbs, $cart->getAccount()); } $crumbs->addTextCrumb(pht('Cart %d', $cart->getID())); + $crumbs->setBorder(true); $timeline = $this->buildTransactionTimeline( $cart, @@ -191,23 +193,28 @@ final class PhortuneCartViewController $timeline ->setShouldTerminate(true); - return $this->buildApplicationPage( - array( - $crumbs, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $error_view, + $details, $cart_box, $description, $charges, $timeline, - ), - array( - 'title' => pht('Cart'), )); + return $this->newPage() + ->setTitle(pht('Cart %d', $cart->getID())) + ->setCrumbs($crumbs) + ->appendChild($view); + } - private function buildPropertyListView(PhortuneCart $cart) { + private function buildDetailsView(PhortuneCart $cart) { - $viewer = $this->getRequest()->getUser(); + $viewer = $this->getViewer(); $view = id(new PHUIPropertyListView()) ->setUser($viewer) @@ -239,21 +246,21 @@ final class PhortuneCartViewController pht('Updated'), phabricator_datetime($cart->getDateModified(), $viewer)); - return $view; + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Details')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($view); } - private function buildActionListView( + private function buildCurtainView( PhortuneCart $cart, $can_edit, $authority, $resume_uri) { - $viewer = $this->getRequest()->getUser(); + $viewer = $this->getViewer(); $id = $cart->getID(); - - $view = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->setObject($cart); + $curtain = $this->newCurtainView($cart); $can_cancel = ($can_edit && $cart->canCancelOrder()); @@ -269,7 +276,7 @@ final class PhortuneCartViewController $accept_uri = $this->getApplicationURI("{$prefix}cart/{$id}/accept/"); $print_uri = $this->getApplicationURI("{$prefix}cart/{$id}/?__print__=1"); - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Cancel Order')) ->setIcon('fa-times') @@ -279,7 +286,7 @@ final class PhortuneCartViewController if ($authority) { if ($cart->getStatus() == PhortuneCart::STATUS_REVIEW) { - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Accept Order')) ->setIcon('fa-check') @@ -287,7 +294,7 @@ final class PhortuneCartViewController ->setHref($accept_uri)); } - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Refund Order')) ->setIcon('fa-reply') @@ -295,28 +302,28 @@ final class PhortuneCartViewController ->setHref($refund_uri)); } - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Update Status')) ->setIcon('fa-refresh') ->setHref($update_uri)); if ($can_edit && $resume_uri) { - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Continue Checkout')) ->setIcon('fa-shopping-cart') ->setHref($resume_uri)); } - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Printable Version')) ->setHref($print_uri) ->setOpenInNewWindow(true) ->setIcon('fa-print')); - return $view; + return $curtain; } } diff --git a/src/applications/phortune/controller/PhortuneMerchantEditController.php b/src/applications/phortune/controller/PhortuneMerchantEditController.php index d3b396ddb0..3a6aad215f 100644 --- a/src/applications/phortune/controller/PhortuneMerchantEditController.php +++ b/src/applications/phortune/controller/PhortuneMerchantEditController.php @@ -145,29 +145,39 @@ final class PhortuneMerchantEditController ->setValue($button_text) ->addCancelButton($cancel_uri)); + $header = id(new PHUIHeaderView()) + ->setHeader($title); + $crumbs = $this->buildApplicationCrumbs(); if ($is_new) { $crumbs->addTextCrumb(pht('Create Merchant')); + $header->setHeaderIcon('fa-plus-square'); } else { $crumbs->addTextCrumb( pht('Merchant %d', $merchant->getID()), $this->getApplicationURI('/merchant/'.$merchant->getID().'/')); $crumbs->addTextCrumb(pht('Edit')); + $header->setHeaderIcon('fa-pencil'); } + $crumbs->setBorder(true); $box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Merchant')) ->setValidationException($validation_exception) - ->setHeaderText($title) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setForm($form); - return $this->buildApplicationPage( - array( - $crumbs, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, - ), - array( - 'title' => $title, )); - } + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + + } } diff --git a/src/applications/phortune/controller/PhortuneMerchantInvoiceCreateController.php b/src/applications/phortune/controller/PhortuneMerchantInvoiceCreateController.php index acb0d1be08..ccdc4f7ac1 100644 --- a/src/applications/phortune/controller/PhortuneMerchantInvoiceCreateController.php +++ b/src/applications/phortune/controller/PhortuneMerchantInvoiceCreateController.php @@ -89,6 +89,7 @@ final class PhortuneMerchantInvoiceCreateController $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($merchant->getName()); + $crumbs->setBorder(true); $v_title = $request->getStr('title'); $e_title = true; @@ -229,18 +230,25 @@ final class PhortuneMerchantInvoiceCreateController ->setValue(pht('Send Invoice'))); $box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('New Invoice')) + ->setHeaderText(pht('Details')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setFormErrors($errors) ->setForm($form); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setHeaderIcon('fa-plus-square'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); } } diff --git a/src/applications/phortune/controller/PhortuneMerchantViewController.php b/src/applications/phortune/controller/PhortuneMerchantViewController.php index 1921f0c5a5..59d9273eaa 100644 --- a/src/applications/phortune/controller/PhortuneMerchantViewController.php +++ b/src/applications/phortune/controller/PhortuneMerchantViewController.php @@ -17,6 +17,7 @@ final class PhortuneMerchantViewController $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($merchant->getName()); + $crumbs->setBorder(true); $title = pht( 'Merchant %d %s', @@ -26,43 +27,44 @@ final class PhortuneMerchantViewController $header = id(new PHUIHeaderView()) ->setHeader($merchant->getName()) ->setUser($viewer) - ->setPolicyObject($merchant); + ->setPolicyObject($merchant) + ->setHeaderIcon('fa-bank'); $providers = id(new PhortunePaymentProviderConfigQuery()) ->setViewer($viewer) ->withMerchantPHIDs(array($merchant->getPHID())) ->execute(); - $properties = $this->buildPropertyListView($merchant, $providers); - $actions = $this->buildActionListView($merchant); - $properties->setActionList($actions); + $details = $this->buildDetailsView($merchant, $providers); + $description = $this->buildDescriptionView($merchant); + $curtain = $this->buildCurtainView($merchant); $provider_list = $this->buildProviderList( $merchant, $providers); - $box = id(new PHUIObjectBoxView()) - ->setHeader($header) - ->addPropertyList($properties); - $timeline = $this->buildTransactionTimeline( $merchant, new PhortuneMerchantTransactionQuery()); $timeline->setShouldTerminate(true); - return $this->buildApplicationPage( - array( - $crumbs, - $box, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $details, + $description, $provider_list, $timeline, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); } - private function buildPropertyListView( + private function buildDetailsView( PhortuneMerchant $merchant, array $providers) { @@ -128,24 +130,31 @@ final class PhortuneMerchantViewController $view->addProperty(pht('Status'), $status_view); - $view->addProperty( - pht('Members'), - $viewer->renderHandleList($merchant->getMemberPHIDs())); + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('DETAILS')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($view); + } - $view->invokeWillRenderEvent(); + private function buildDescriptionView(PhortuneMerchant $merchant) { + $viewer = $this->getViewer(); + $view = id(new PHUIPropertyListView()) + ->setUser($viewer); $description = $merchant->getDescription(); if (strlen($description)) { $description = new PHUIRemarkupView($viewer, $description); - $view->addSectionHeader( - pht('Description'), PHUIPropertyListView::ICON_SUMMARY); $view->addTextContent($description); + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('DESCRIPTION')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($view); } - return $view; + return null; } - private function buildActionListView(PhortuneMerchant $merchant) { + private function buildCurtainView(PhortuneMerchant $merchant) { $viewer = $this->getRequest()->getUser(); $id = $merchant->getID(); @@ -154,11 +163,9 @@ final class PhortuneMerchantViewController $merchant, PhabricatorPolicyCapability::CAN_EDIT); - $view = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->setObject($merchant); + $curtain = $this->newCurtainView($merchant); - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Merchant')) ->setIcon('fa-pencil') @@ -166,7 +173,7 @@ final class PhortuneMerchantViewController ->setWorkflow(!$can_edit) ->setHref($this->getApplicationURI("merchant/edit/{$id}/"))); - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('View Orders')) ->setIcon('fa-shopping-cart') @@ -174,7 +181,7 @@ final class PhortuneMerchantViewController ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('View Subscriptions')) ->setIcon('fa-moon-o') @@ -182,8 +189,7 @@ final class PhortuneMerchantViewController ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); - - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('New Invoice')) ->setIcon('fa-fax') @@ -191,7 +197,30 @@ final class PhortuneMerchantViewController ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); - return $view; + $member_phids = $merchant->getMemberPHIDs(); + $handles = $viewer->loadHandles($member_phids); + + $member_list = id(new PHUIObjectItemListView()) + ->setSimple(true); + + foreach ($member_phids as $member_phid) { + $image_uri = $handles[$member_phid]->getImageURI(); + $image_href = $handles[$member_phid]->getURI(); + $person = $handles[$member_phid]; + + $member = id(new PHUIObjectItemView()) + ->setImageURI($image_uri) + ->setHref($image_href) + ->setHeader($person->getFullName()); + + $member_list->addItem($member); + } + + $curtain->newPanel() + ->setHeaderText(pht('Members')) + ->appendChild($member_list); + + return $curtain; } private function buildProviderList( @@ -283,6 +312,7 @@ final class PhortuneMerchantViewController return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($provider_list); } diff --git a/src/applications/phortune/controller/PhortunePaymentMethodCreateController.php b/src/applications/phortune/controller/PhortunePaymentMethodCreateController.php index dd783d4c51..794f97aa8e 100644 --- a/src/applications/phortune/controller/PhortunePaymentMethodCreateController.php +++ b/src/applications/phortune/controller/PhortunePaymentMethodCreateController.php @@ -158,20 +158,29 @@ final class PhortunePaymentMethodCreateController ->addCancelButton($cancel_uri)); $box = id(new PHUIObjectBoxView()) - ->setHeaderText($provider->getPaymentMethodDescription()) + ->setHeaderText(pht('Method')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setForm($form); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('Add Payment Method')); + $crumbs->setBorder(true); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Add Payment Method')) + ->setHeaderIcon('fa-plus-square'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, - ), - array( - 'title' => $provider->getPaymentMethodDescription(), )); + + return $this->newPage() + ->setTitle($provider->getPaymentMethodDescription()) + ->setCrumbs($crumbs) + ->appendChild($view); + } private function renderSelectProvider( diff --git a/src/applications/phortune/controller/PhortunePaymentMethodEditController.php b/src/applications/phortune/controller/PhortunePaymentMethodEditController.php index 27edd72a59..dc23b81ad0 100644 --- a/src/applications/phortune/controller/PhortunePaymentMethodEditController.php +++ b/src/applications/phortune/controller/PhortunePaymentMethodEditController.php @@ -58,22 +58,31 @@ final class PhortunePaymentMethodEditController ->setValue(pht('Save Changes'))); $box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Edit Payment Method')) + ->setHeaderText(pht('Payment Method')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setForm($form); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($account->getName(), $account_uri); $crumbs->addTextCrumb($method->getDisplayName()); $crumbs->addTextCrumb(pht('Edit')); + $crumbs->setBorder(true); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Edit Payment Method')) + ->setHeaderIcon('fa-pencil'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, - ), - array( - 'title' => pht('Edit Payment Method'), )); + + return $this->newPage() + ->setTitle(pht('Edit Payment Method')) + ->setCrumbs($crumbs) + ->appendChild($view); + } } diff --git a/src/applications/phortune/controller/PhortuneProductListController.php b/src/applications/phortune/controller/PhortuneProductListController.php index a82effa6d4..eeb594d650 100644 --- a/src/applications/phortune/controller/PhortuneProductListController.php +++ b/src/applications/phortune/controller/PhortuneProductListController.php @@ -24,6 +24,7 @@ final class PhortuneProductListController extends PhabricatorController { ->setName(pht('Create Product')) ->setHref($this->getApplicationURI('product/edit/')) ->setIcon('fa-plus-square')); + $crumbs->setBorder(true); $product_list = id(new PHUIObjectItemListView()) ->setUser($viewer) @@ -39,20 +40,33 @@ final class PhortuneProductListController extends PhabricatorController { ->setObjectName($product->getID()) ->setHeader($product->getProductName()) ->setHref($view_uri) - ->addAttribute($price->formatForDisplay()); + ->addAttribute($price->formatForDisplay()) + ->setIcon('fa-gift'); $product_list->addItem($item); } - return $this->buildApplicationPage( - array( - $crumbs, - $product_list, + $box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Products')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setObjectList($product_list); + + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Products')) + ->setHeaderIcon('fa-gift'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( + $box, $pager, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } } diff --git a/src/applications/phortune/controller/PhortuneProductViewController.php b/src/applications/phortune/controller/PhortuneProductViewController.php index 56bf0736d3..0bf022e373 100644 --- a/src/applications/phortune/controller/PhortuneProductViewController.php +++ b/src/applications/phortune/controller/PhortuneProductViewController.php @@ -17,13 +17,11 @@ final class PhortuneProductViewController extends PhortuneController { $title = pht('Product: %s', $product->getProductName()); $header = id(new PHUIHeaderView()) - ->setHeader($product->getProductName()); + ->setHeader($product->getProductName()) + ->setHeaderIcon('fa-gift'); $edit_uri = $this->getApplicationURI('product/edit/'.$product->getID().'/'); - $actions = id(new PhabricatorActionListView()) - ->setUser($viewer); - $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb( pht('Products'), @@ -31,26 +29,30 @@ final class PhortuneProductViewController extends PhortuneController { $crumbs->addTextCrumb( pht('#%d', $product->getID()), $request->getRequestURI()); + $crumbs->setBorder(true); $properties = id(new PHUIPropertyListView()) ->setUser($viewer) - ->setActionList($actions) ->addProperty( pht('Price'), $product->getPriceAsCurrency()->formatForDisplay()); $object_box = id(new PHUIObjectBoxView()) - ->setHeader($header) + ->setHeaderText(pht('DETAILS')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); - return $this->buildApplicationPage( - array( - $crumbs, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $object_box, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } } diff --git a/src/applications/phortune/controller/PhortuneProviderEditController.php b/src/applications/phortune/controller/PhortuneProviderEditController.php index f956c41d68..f7ad2486c4 100644 --- a/src/applications/phortune/controller/PhortuneProviderEditController.php +++ b/src/applications/phortune/controller/PhortuneProviderEditController.php @@ -177,6 +177,7 @@ final class PhortuneProviderEditController $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($merchant->getName(), $cancel_uri); + $crumbs->setBorder(true); if ($is_new) { $crumbs->addTextCrumb(pht('Add Provider')); @@ -185,19 +186,27 @@ final class PhortuneProviderEditController pht('Edit Provider %d', $provider_config->getID())); } + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setHeaderIcon('fa-pencil'); + $box = id(new PHUIObjectBoxView()) ->setFormErrors($errors) - ->setHeaderText($title) + ->setHeaderText(pht('Properties')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($form); - return $this->buildApplicationPage( - array( - $crumbs, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } private function processChooseClassRequest( @@ -266,20 +275,28 @@ final class PhortuneProviderEditController $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($merchant->getName(), $cancel_uri); $crumbs->addTextCrumb($title); + $crumbs->setBorder(true); $box = id(new PHUIObjectBoxView()) - ->setHeaderText($title) + ->setHeaderText(pht('Provider')) ->setFormErrors($errors) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setForm($form); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setHeaderIcon('fa-plus-square'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); } } diff --git a/src/applications/phortune/controller/PhortuneSubscriptionEditController.php b/src/applications/phortune/controller/PhortuneSubscriptionEditController.php index a6500cdf91..2eb6908fde 100644 --- a/src/applications/phortune/controller/PhortuneSubscriptionEditController.php +++ b/src/applications/phortune/controller/PhortuneSubscriptionEditController.php @@ -140,18 +140,26 @@ final class PhortuneSubscriptionEditController extends PhortuneController { $box = id(new PHUIObjectBoxView()) ->setUser($viewer) - ->setHeaderText(pht('Edit %s', $subscription->getSubscriptionName())) + ->setHeaderText(pht('Subscription')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setFormErrors($errors) ->appendChild($form); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Edit %s', $subscription->getSubscriptionName())) + ->setHeaderIcon('fa-pencil'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } diff --git a/src/applications/phortune/controller/PhortuneSubscriptionViewController.php b/src/applications/phortune/controller/PhortuneSubscriptionViewController.php index 8ecc97c301..0e1bc55b62 100644 --- a/src/applications/phortune/controller/PhortuneSubscriptionViewController.php +++ b/src/applications/phortune/controller/PhortuneSubscriptionViewController.php @@ -35,14 +35,13 @@ final class PhortuneSubscriptionViewController extends PhortuneController { $title = $subscription->getSubscriptionFullName(); $header = id(new PHUIHeaderView()) - ->setHeader($title); - - $actions = id(new PhabricatorActionListView()) - ->setUser($viewer); + ->setHeader($title) + ->setHeaderIcon('fa-calendar-o'); + $curtain = $this->newCurtainView($subscription); $edit_uri = $subscription->getEditURI(); - $actions->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setIcon('fa-pencil') ->setName(pht('Edit Subscription')) @@ -50,7 +49,6 @@ final class PhortuneSubscriptionViewController extends PhortuneController { ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); - $crumbs = $this->buildApplicationCrumbs(); if ($authority) { $this->addMerchantCrumb($crumbs, $merchant); @@ -58,10 +56,10 @@ final class PhortuneSubscriptionViewController extends PhortuneController { $this->addAccountCrumb($crumbs, $account); } $crumbs->addTextCrumb($subscription->getSubscriptionCrumbName()); + $crumbs->setBorder(true); $properties = id(new PHUIPropertyListView()) - ->setUser($viewer) - ->setActionList($actions); + ->setUser($viewer); $next_invoice = $subscription->getTrigger()->getNextEventPrediction(); $properties->addProperty( @@ -83,23 +81,27 @@ final class PhortuneSubscriptionViewController extends PhortuneController { pht('Autopay With'), $autopay_method); - $object_box = id(new PHUIObjectBoxView()) - ->setHeader($header) + $details = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('DETAILS')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); $due_box = $this->buildDueInvoices($subscription, $authority); $invoice_box = $this->buildPastInvoices($subscription, $authority); - return $this->buildApplicationPage( - array( - $crumbs, - $object_box, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $details, $due_box, $invoice_box, - ), - array( - 'title' => $title, - )); + )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); } private function buildDueInvoices( @@ -136,6 +138,7 @@ final class PhortuneSubscriptionViewController extends PhortuneController { return id(new PHUIObjectBoxView()) ->setHeader($invoice_header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($invoice_table); } @@ -199,6 +202,7 @@ final class PhortuneSubscriptionViewController extends PhortuneController { return id(new PHUIObjectBoxView()) ->setHeader($invoice_header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($invoice_table); } diff --git a/src/applications/phortune/query/PhortuneMerchantSearchEngine.php b/src/applications/phortune/query/PhortuneMerchantSearchEngine.php index 257e74484d..1806af371e 100644 --- a/src/applications/phortune/query/PhortuneMerchantSearchEngine.php +++ b/src/applications/phortune/query/PhortuneMerchantSearchEngine.php @@ -70,10 +70,11 @@ final class PhortuneMerchantSearchEngine $list->setUser($viewer); foreach ($merchants as $merchant) { $item = id(new PHUIObjectItemView()) - ->setObjectName(pht('Merchant %d', $merchant->getID())) + ->setSubhead(pht('Merchant %d', $merchant->getID())) ->setHeader($merchant->getName()) ->setHref('/phortune/merchant/'.$merchant->getID().'/') - ->setObject($merchant); + ->setObject($merchant) + ->setIcon('fa-bank'); $list->addItem($item); } diff --git a/src/view/phui/PHUIObjectItemListView.php b/src/view/phui/PHUIObjectItemListView.php index c21ea558a2..55c0c8aede 100644 --- a/src/view/phui/PHUIObjectItemListView.php +++ b/src/view/phui/PHUIObjectItemListView.php @@ -7,6 +7,7 @@ final class PHUIObjectItemListView extends AphrontTagView { private $pager; private $noDataString; private $flush; + private $simple; private $allowEmptyList; private $states; private $itemClass = 'phui-object-item-standard'; @@ -35,6 +36,11 @@ final class PHUIObjectItemListView extends AphrontTagView { return $this; } + public function setSimple($simple) { + $this->simple = $simple; + return $this; + } + public function setNoDataString($no_data_string) { $this->noDataString = $no_data_string; return $this; @@ -69,6 +75,9 @@ final class PHUIObjectItemListView extends AphrontTagView { if ($this->flush) { $classes[] = 'phui-object-list-flush'; } + if ($this->simple) { + $classes[] = 'phui-object-list-simple'; + } return array( 'class' => $classes, diff --git a/webroot/rsrc/css/phui/phui-object-item-list-view.css b/webroot/rsrc/css/phui/phui-object-item-list-view.css index 17894f6f9d..3556d48672 100644 --- a/webroot/rsrc/css/phui/phui-object-item-list-view.css +++ b/webroot/rsrc/css/phui/phui-object-item-list-view.css @@ -782,3 +782,44 @@ ul.phui-object-item-list-view .phui-object-item-selected padding: 0 8px 8px; text-align: left; } + +/* - Simple List------------------------------------------------------------- */ + +.phui-object-list-simple .phui-object-item-with-image .phui-object-item-frame { + min-height: 26px; +} + +.phui-object-list-simple .phui-object-item-image { + height: 26px; + width: 26px; + margin: 0; +} + +.phui-object-list-simple .phui-object-item-with-image + .phui-object-item-content-box { + margin-left: 32px; +} + +.phui-object-list-simple .phui-object-item-name { + padding: 2px 0; +} + +.phui-object-list-simple .phui-object-item-name a { + color: {$darkbluetext}; +} + +.phui-object-item-list-view.phui-object-list-simple .phui-object-item-frame { + border: none; + margin-bottom: 4px; +} + +.phui-object-item-list-view.phui-object-list-simple li:last-child + .phui-object-item-frame { + margin: 0; +} + +.phui-object-list-simple .phui-object-item-actions { + top: 2px; + bottom: 2px; + right: 2px; +} diff --git a/webroot/rsrc/css/phui/phui-two-column-view.css b/webroot/rsrc/css/phui/phui-two-column-view.css index 7997b196fe..3389d3e574 100644 --- a/webroot/rsrc/css/phui/phui-two-column-view.css +++ b/webroot/rsrc/css/phui/phui-two-column-view.css @@ -197,3 +197,8 @@ .phui-info-view { margin: 0; } + +.phui-two-column-view .phui-box-blue-property + .phui-header-shell + .phui-info-view { + margin: 16px; +} From c0cb52dd785a56b75dead3488f440487052ae8d2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 23 Mar 2016 10:47:16 -0700 Subject: [PATCH 20/31] Fix Phortune Subscription high-security checkpoint URI Summary: This URI is currently a little whack. Test Plan: - With MFA, clicked "Edit Subscription" on a subscription. - Clicked "Cancel". - Before: Sent to `/phortune/phortune/edit/...`, a 404. - After: Properly returned to subscription detail page. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D15514 --- .../phortune/controller/PhortuneSubscriptionEditController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/phortune/controller/PhortuneSubscriptionEditController.php b/src/applications/phortune/controller/PhortuneSubscriptionEditController.php index 2eb6908fde..8ba0592be4 100644 --- a/src/applications/phortune/controller/PhortuneSubscriptionEditController.php +++ b/src/applications/phortune/controller/PhortuneSubscriptionEditController.php @@ -21,7 +21,7 @@ final class PhortuneSubscriptionEditController extends PhortuneController { id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( $viewer, $request, - $this->getApplicationURI($subscription->getEditURI())); + $subscription->getURI()); $merchant = $subscription->getMerchant(); $account = $subscription->getAccount(); From 4a6589524b4af07db771d045b9b06318b2a430d6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 23 Mar 2016 12:16:31 -0700 Subject: [PATCH 21/31] Add `amazon-ses.endpoint` configuration Summary: Fixes T5116. Test Plan: Will test in production. Reviewers: chad Maniphest Tasks: T5116 Differential Revision: https://secure.phabricator.com/D15515 --- .../config/check/PhabricatorMailSetupCheck.php | 13 +++++++++++++ .../config/option/PhabricatorAWSConfigOptions.php | 12 ++++++++++++ ...habricatorMailImplementationAmazonSESAdapter.php | 3 ++- .../configuring_outbound_email.diviner | 1 + 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/applications/config/check/PhabricatorMailSetupCheck.php b/src/applications/config/check/PhabricatorMailSetupCheck.php index ff789d4aee..2b8e4e12d5 100644 --- a/src/applications/config/check/PhabricatorMailSetupCheck.php +++ b/src/applications/config/check/PhabricatorMailSetupCheck.php @@ -64,6 +64,19 @@ final class PhabricatorMailSetupCheck extends PhabricatorSetupCheck { ->addPhabricatorConfig('amazon-ses.secret-key'); } + if (!PhabricatorEnv::getEnvConfig('amazon-ses.endpoint')) { + $message = pht( + 'Amazon SES is selected as the mail adapter, but no SES endpoint '. + 'is configured. Provide an SES endpoint or choose a different '. + 'mail adapter.'); + + $this->newIssue('config.amazon-ses.endpoint') + ->setName(pht('Amazon SES Endpoint Not Set')) + ->setMessage($message) + ->addRelatedPhabricatorConfig('metamta.mail-adapter') + ->addPhabricatorConfig('amazon-ses.endpoint'); + } + $address_key = 'metamta.default-address'; $options = PhabricatorApplicationConfigOptions::loadAllOptions(); $default = $options[$address_key]->getDefault(); diff --git a/src/applications/config/option/PhabricatorAWSConfigOptions.php b/src/applications/config/option/PhabricatorAWSConfigOptions.php index faed807ae3..6647930859 100644 --- a/src/applications/config/option/PhabricatorAWSConfigOptions.php +++ b/src/applications/config/option/PhabricatorAWSConfigOptions.php @@ -27,6 +27,18 @@ final class PhabricatorAWSConfigOptions $this->newOption('amazon-ses.secret-key', 'string', null) ->setHidden(true) ->setDescription(pht('Secret key for Amazon SES.')), + $this->newOption('amazon-ses.endpoint', 'string', null) + ->setLocked(true) + ->setDescription( + pht( + 'SES endpoint domain name. You can find a list of available '. + 'regions and endpoints in the AWS documentation.')) + ->addExample( + 'email.us-east-1.amazonaws.com', + pht('US East (N. Virginia, Older default endpoint)')) + ->addExample( + 'email.us-west-2.amazonaws.com', + pht('US West (Oregon)')), $this->newOption('amazon-s3.access-key', 'string', null) ->setLocked(true) ->setDescription(pht('Access key for Amazon S3.')), diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationAmazonSESAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationAmazonSESAdapter.php index 59c7b08933..5b03cd86ac 100644 --- a/src/applications/metamta/adapter/PhabricatorMailImplementationAmazonSESAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationAmazonSESAdapter.php @@ -23,12 +23,13 @@ final class PhabricatorMailImplementationAmazonSESAdapter public function executeSend($body) { $key = PhabricatorEnv::getEnvConfig('amazon-ses.access-key'); $secret = PhabricatorEnv::getEnvConfig('amazon-ses.secret-key'); + $endpoint = PhabricatorEnv::getEnvConfig('amazon-ses.endpoint'); $root = phutil_get_library_root('phabricator'); $root = dirname($root); require_once $root.'/externals/amazon-ses/ses.php'; - $service = new SimpleEmailService($key, $secret); + $service = new SimpleEmailService($key, $secret, $endpoint); $service->enableUseExceptions(true); return $service->sendRawEmail($body); } diff --git a/src/docs/user/configuration/configuring_outbound_email.diviner b/src/docs/user/configuration/configuring_outbound_email.diviner index 55c51837c7..2a95f49bc3 100644 --- a/src/docs/user/configuration/configuring_outbound_email.diviner +++ b/src/docs/user/configuration/configuring_outbound_email.diviner @@ -135,6 +135,7 @@ To configure Phabricator to use Amazon SES, set these configuration keys: "PhabricatorMailImplementationAmazonSESAdapter". - **amazon-ses.access-key**: set to your Amazon SES access key. - **amazon-ses.secret-key**: set to your Amazon SES secret key. + - **amazon-ses.endpoint**: Set to your Amazon SES endpoint. NOTE: Amazon SES **requires you to verify your "From" address**. Configure which "From" address to use by setting "`metamta.default-address`" in your config, From 6cd747f77c3331cb61f8112b27bf4107faeba31c Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Mar 2016 15:36:59 -0700 Subject: [PATCH 22/31] Kinda start bridging data in from GitHub via Nuance Summary: Ref T10538. Very sloppy, but starting to sort of work. This sort of gets a piece of framework into a reasonable spot, next couple of diffs are going to be "extract comment text" and "show stuff in the UI" sorts of things. Test Plan: {F1186726} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10538 Differential Revision: https://secure.phabricator.com/D15511 --- .../bridge/DoorkeeperBridgeGitHubIssue.php | 16 +- .../nuance/github/NuanceGitHubRawEvent.php | 4 + .../nuance/item/NuanceGitHubEventItemType.php | 198 ++++++++++++++++-- .../nuance/item/NuanceItemType.php | 4 +- 4 files changed, 196 insertions(+), 26 deletions(-) diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHubIssue.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHubIssue.php index 50dc28bf9d..0558b7d8f6 100644 --- a/src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHubIssue.php +++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeGitHubIssue.php @@ -76,9 +76,6 @@ final class DoorkeeperBridgeGitHubIssue $ref->setAttribute('name', $body['title']); $obj = $ref->getExternalObject(); - if ($obj->getID()) { - continue; - } $this->fillObjectFromData($obj, $result); @@ -92,6 +89,19 @@ final class DoorkeeperBridgeGitHubIssue $body = $result->getBody(); $uri = $body['html_url']; $obj->setObjectURI($uri); + + $title = idx($body, 'title'); + $description = idx($body, 'body'); + + $created = idx($body, 'created_at'); + $created = strtotime($created); + + $state = idx($body, 'state'); + + $obj->setProperty('task.title', $title); + $obj->setProperty('task.description', $description); + $obj->setProperty('task.created', $created); + $obj->setProperty('task.state', $state); } } diff --git a/src/applications/nuance/github/NuanceGitHubRawEvent.php b/src/applications/nuance/github/NuanceGitHubRawEvent.php index 9e170bf092..2c0d62c7ca 100644 --- a/src/applications/nuance/github/NuanceGitHubRawEvent.php +++ b/src/applications/nuance/github/NuanceGitHubRawEvent.php @@ -90,6 +90,10 @@ final class NuanceGitHubRawEvent extends Phobject { return null; } + public function getComment() { + return 'TODO: Actually extract comment text.'; + } + public function getURI() { $raw = $this->raw; diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php index 3ee2e91489..1c6d90bbf3 100644 --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -5,6 +5,8 @@ final class NuanceGitHubEventItemType const ITEMTYPE = 'github.event'; + private $externalObject; + public function getItemTypeDisplayName() { return pht('GitHub Event'); } @@ -79,29 +81,13 @@ final class NuanceGitHubEventItemType // TODO: Link up the requestor, etc. - $source = $item->getSource(); - $token = $source->getSourceProperty('github.token'); - $token = new PhutilOpaqueEnvelope($token); + $is_dirty = false; - $ref = $this->getDoorkeeperRef($item); - if ($ref) { - $ref = id(new DoorkeeperImportEngine()) - ->setViewer($viewer) - ->setRefs(array($ref)) - ->setThrowOnMissingLink(true) - ->setContextProperty('github.token', $token) - ->executeOne(); + $xobj = $this->reloadExternalObject($item); - if ($ref->getSyncFailed()) { - $xobj = null; - } else { - $xobj = $ref->getExternalObject(); - } - - if ($xobj) { - $item->setItemProperty('doorkeeper.xobj.phid', $xobj->getPHID()); - $is_dirty = true; - } + if ($xobj) { + $item->setItemProperty('doorkeeper.xobj.phid', $xobj->getPHID()); + $is_dirty = true; } if ($item->getStatus() == NuanceItem::STATUS_IMPORTING) { @@ -137,6 +123,56 @@ final class NuanceGitHubEventItemType ->setObjectID($full_ref); } + private function reloadExternalObject(NuanceItem $item, $local = false) { + $ref = $this->getDoorkeeperRef($item); + if (!$ref) { + return null; + } + + $source = $item->getSource(); + $token = $source->getSourceProperty('github.token'); + $token = new PhutilOpaqueEnvelope($token); + + $viewer = $this->getViewer(); + + $ref = id(new DoorkeeperImportEngine()) + ->setViewer($viewer) + ->setRefs(array($ref)) + ->setThrowOnMissingLink(true) + ->setContextProperty('github.token', $token) + ->needLocalOnly($local) + ->executeOne(); + + if ($ref->getSyncFailed()) { + $xobj = null; + } else { + $xobj = $ref->getExternalObject(); + } + + if ($xobj) { + $this->externalObject = $xobj; + } + + return $xobj; + } + + private function getExternalObject(NuanceItem $item) { + if ($this->externalObject === null) { + $xobj = $this->reloadExternalObject($item, $local = true); + if ($xobj) { + $this->externalObject = $xobj; + } else { + $this->externalObject = false; + } + } + + if ($this->externalObject) { + return $this->externalObject; + } + + return null; + } + private function newRawEvent(NuanceItem $item) { $type = $item->getItemProperty('api.type'); $raw = $item->getItemProperty('api.raw', array()); @@ -147,6 +183,15 @@ final class NuanceGitHubEventItemType public function getItemActions(NuanceItem $item) { $actions = array(); + $xobj = $this->getExternalObject($item); + if ($xobj) { + $actions[] = $this->newItemAction($item, 'reload') + ->setName(pht('Reload from GitHub')) + ->setIcon('fa-refresh') + ->setWorkflow(true) + ->setRenderAsForm(true); + } + $actions[] = $this->newItemAction($item, 'sync') ->setName(pht('Import to Maniphest')) ->setIcon('fa-anchor') @@ -189,6 +234,7 @@ final class NuanceGitHubEventItemType ->appendForm($form) ->addCancelButton($item->getURI(), pht('Done')); case 'sync': + case 'reload': $item->issueCommand($viewer->getPHID(), $action); return id(new AphrontRedirectResponse())->setURI($item->getURI()); } @@ -238,9 +284,117 @@ final class NuanceGitHubEventItemType ->appendChild($property_list); } - protected function handleCommand(NuanceItem $item, $action) { + protected function handleCommand( + NuanceItem $item, + NuanceItemCommand $command) { + + $action = $command->getCommand(); + switch ($action) { + case 'sync': + return $this->syncItem($item, $command); + case 'reload': + $this->reloadExternalObject($item); + return true; + } + return null; } + private function syncItem( + NuanceItem $item, + NuanceItemCommand $command) { + + $xobj_phid = $item->getItemProperty('doorkeeper.xobj.phid'); + if (!$xobj_phid) { + throw new Exception( + pht( + 'Unable to sync: no external object PHID.')); + } + + // TODO: Write some kind of marker to prevent double-synchronization. + + $viewer = $this->getViewer(); + + $xobj = id(new DoorkeeperExternalObjectQuery()) + ->setViewer($viewer) + ->withPHIDs(array($xobj_phid)) + ->executeOne(); + if (!$xobj) { + throw new Exception( + pht( + 'Unable to sync: failed to load object "%s".', + $xobj_phid)); + } + + $nuance_phid = id(new PhabricatorNuanceApplication())->getPHID(); + + $xactions = array(); + + $task = id(new ManiphestTaskQuery()) + ->setViewer($viewer) + ->withBridgedObjectPHIDs(array($xobj_phid)) + ->executeOne(); + if (!$task) { + $task = ManiphestTask::initializeNewTask($viewer) + ->setAuthorPHID($nuance_phid) + ->setBridgedObjectPHID($xobj_phid); + + $title = $xobj->getProperty('task.title'); + if (!strlen($title)) { + $title = pht('Nuance Item %d Task', $item->getID()); + } + + $description = $xobj->getProperty('task.description'); + $created = $xobj->getProperty('task.created'); + $state = $xobj->getProperty('task.state'); + + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransaction::TYPE_TITLE) + ->setNewValue($title) + ->setDateCreated($created); + + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransaction::TYPE_DESCRIPTION) + ->setNewValue($description) + ->setDateCreated($created); + + $task->setDateCreated($created); + + // TODO: Synchronize state. + } + + $event = $this->newRawEvent($item); + $comment = $event->getComment(); + if (strlen($comment)) { + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new ManiphestTransactionComment()) + ->setContent($comment)); + } + + // TODO: Preserve the item's original source. + $source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_DAEMON, + array()); + + // TOOD: This should really be the external source. + $acting_phid = $nuance_phid; + + $editor = id(new ManiphestTransactionEditor()) + ->setActor($viewer) + ->setActingAsPHID($acting_phid) + ->setContentSource($source) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + $xactions = $editor->applyTransactions($task, $xactions); + + return array( + 'objectPHID' => $task->getPHID(), + 'xactionPHIDs' => mpull($xactions, 'getPHID'), + ); + } + } diff --git a/src/applications/nuance/item/NuanceItemType.php b/src/applications/nuance/item/NuanceItemType.php index 144d6e86f5..e8397b13f8 100644 --- a/src/applications/nuance/item/NuanceItemType.php +++ b/src/applications/nuance/item/NuanceItemType.php @@ -131,7 +131,9 @@ abstract class NuanceItemType ->applyTransactions($item, array($xaction)); } - protected function handleCommand(NuanceItem $item, $action) { + protected function handleCommand( + NuanceItem $item, + NuanceItemCommand $command) { return null; } From b1937962663d36b2be326a69553fa8ae55fd9be6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Mar 2016 08:37:55 -0700 Subject: [PATCH 23/31] Provide "Reproduction Steps" docs and separate "Version" doc Summary: I know this is ultimately pointless but feel better about pushing back on users when there is no possible way they could be acting in good faith. Test Plan: Read documents. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D15518 --- src/docs/contributor/bug_reports.diviner | 42 +-- .../contributor/reproduction_steps.diviner | 252 ++++++++++++++++++ src/docs/contributor/version.diviner | 57 ++++ 3 files changed, 316 insertions(+), 35 deletions(-) create mode 100644 src/docs/contributor/reproduction_steps.diviner create mode 100644 src/docs/contributor/version.diviner diff --git a/src/docs/contributor/bug_reports.diviner b/src/docs/contributor/bug_reports.diviner index 987fd1571f..91cfbce947 100644 --- a/src/docs/contributor/bug_reports.diviner +++ b/src/docs/contributor/bug_reports.diviner @@ -8,8 +8,9 @@ Include Reproduction Steps! IMPORTANT: When filing a bug report, you **MUST** include reproduction instructions. We can not help fix bugs we can not reproduce, and will not -accept reports which omit reproduction instructions. See below for details. +accept reports which omit reproduction instructions. +For help, see @{article:Providing Reproduction Steps}. Overview ======== @@ -69,12 +70,9 @@ To update Phabricator, use a script like the one described in @{article:Upgrading Phabricator}. **If you can not update** for some reason, please include the version of -Phabricator you are running when you file a report. You can find the version in -{nav Config > Versions} in the web UI. +Phabricator you are running when you file a report. -(The version is just the Git hash of your local HEAD, so you can also find it -by running `git show` in `phabricator/` and looking at the first line of -output.) +For help, see @{article:Providing Version Information}. Supported Issues @@ -86,6 +84,8 @@ support. **We can NOT help you with issues we can not reproduce.** It is critical that you explain how to reproduce the issue when filing a report. +For help, see @{article:Providing Reproduction Steps}. + **We do NOT support prototype applications.** If you're running into an issue with a prototype application, you're on your own. For more information about prototype applications, see @{article:User Guide: Prototype Applications}. @@ -144,39 +144,11 @@ reproduce the issue. What did you do? If you do it again, does it still break? Does it depend on a specific browser? Can you reproduce the issue on `secure.phabricator.com`? -Feel free to try to reproduce issues on the upstream install (which is kept near -HEAD), within reason -- it's okay to make a few test objects if you're having -trouble narrowing something down or want to check if updating might fix an -issue. - It is nearly impossible for us to resolve many issues if we can not reproduce them. We will not accept reports which do not contain the information required to reproduce problems. - -Unreproducible Problems -======================= - -Before we can fix a bug, we need to reproduce it. If we can't reproduce a -problem, we can't tell if we've fixed it and often won't be able to figure out -why it is occurring. - -Most problems reproduce easily, but some are more difficult to reproduce. We -will generally make a reasonable effort to reproduce problems, but sometimes -we will be unable to reproduce an issue. - -Many of these unreproducible issues turn out to be bizarre environmental -problems that are unique to one user's install, and figuring out what is wrong -takes a very long time with a lot of back and forth as we ask questions to -narrow down the cause of the problem. When we eventually figure it out and fix -it, few others benefit (in some cases, no one else). This sort of fishing -expedition is not a good use of anyone's time, and it's very hard for us to -prioritize solving these problems because they represent a huge effort for very -little benefit. - -We will make a reasonable effort to reproduce problems, but can not help with -issues which we can't reproduce. You can make sure we're able to help resolve -your issue by generating clear reproduction steps. +For help, see @{article:Providing Reproduction Steps}. Create a Task in Maniphest diff --git a/src/docs/contributor/reproduction_steps.diviner b/src/docs/contributor/reproduction_steps.diviner new file mode 100644 index 0000000000..4ce17b5aff --- /dev/null +++ b/src/docs/contributor/reproduction_steps.diviner @@ -0,0 +1,252 @@ +@title Providing Reproduction Steps +@group detail + +Describes how to provide reproduction steps. + +Overview +======== + +When you submit a bug report about Phabricator, you **MUST** include +reproduction steps. We can not help you with bugs we can not reproduce, and +will not accept reports which omit reproduction steps or have incomplete or +insufficient instructions. + +This document explains what we're looking for in good reproduction steps. +Briefly: + + - Reproduction steps must allow us to reproduce the problem locally on a + clean, up-to-date install of Phabricator. + - Reproduction should be as simple as possible. + +Good reproduction steps can take time to write out clearly, simplify, and +verify. As a reporter, we expect you to shoulder as much of this burden as you +can, to make make it easy for us to reproduce and resolve bugs. + +We do not have the resources to pursue reports with limited, inaccurate, or +incomplete reproduction steps, and will not accept them. These reports require +large amounts of our time and are often fruitless. + + +Example Reproduction Steps +========================== + +Here's an example of what good reproduction steps might look like: + +--- + +Reproduction steps: + + - Click "Create Event" in Calendar. + - Fill in the required fields with any text (name, description, etc). + - Set an invalid time for one of the dates, like the meaningless string + "Tea Time". This is not valid, so we're expecting an error when we + submit the form. + - Click "Create" to save the event. + +Expected result: + + - Form reloads with an error message about the specific mistake. + - All field values are retained. + +Actual result: + + - Form reloads with an error message about the specific mistake. + - Most values are discarded, so I have to re-type the name, description, etc. + +--- + +These steps are **complete** and **self-contained**: anyone can reproduce the +issue by following these steps. These steps are **clear** and **easy to +follow**. These steps are also simple and minimal: they don't include any +extra unnecessary steps. + +Finally, these steps explain what the reporter expected to happen, what they +observed, and how those behaviors differ. This isn't as important when the +observation is an obvious error like an exception, but can be important if a +behavior is merely odd or ambiguous. + + +Reliable Reproduction +===================== + +When you file a bug report, the first thing we do to fix it is to try to +reproduce the problem locally on an up-to-date install of Phabricator. We will +do this by following the steps you provide. If we can recreate the issue +locally, we can almost always resolve the problem (often very promptly). + +However, many reports do not have enough information, are missing important +steps, or rely on data (like commits, users, other projects, permission +settings, feed stories, etc) that we don't have access to. We either can't +follow these steps, or can't reproduce issues by following them. + +Reproduction steps must be complete and self-contained, and must allow +**anyone** to reproduce the issue on a new, empty install of Phabricator. If +the bug you're seeing depends on data or configuration which would not be +present on a new install, you need to include that information in your steps. + +For example, if you're seeing an issue which depends on a particular policy +setting or configuration setting, you need to include instructions for creating +the policy or adjusting the setting in your steps. + + +Getting Started +=============== + +To generate reproduction steps, first find a sequence of actions which +reproduce the issue you're seeing reliably. + +Next, write down everything you did as clearly as possible. Make sure each step +is self-contained: anyone should be able to follow your steps, without access +to private or proprietary data. + +Now, to verify that your steps provide a complete, self-contained way to +reproduce the issue, follow them yourself on a new, empty, up-to-date instance +of Phabricator. + +If you can't easily start an empty instance locally, you can launch an empty +instance on Phacility in about 60 seconds (see the "Resources" section for +details). + +If you can follow your steps and reproduce the issue on a clean instance, +we'll probably be able to follow them and reproduce the issue ourselves. + +If you can't follow your steps because they depend on information which is not +available on a clean instance (for example, a certain config setting), rewrite +them to include instrutions to create that information (for example, adjusting +the config to the problematic value). + +If you follow your instructions but the issue does not reproduce, the issue +might already be fixed. Make sure your install is up to date. + +If your install is up to date and the issue still doesn't reproduce on a clean +install, your reproduction steps are missing important information. You need to +figure out what key element differs between your install and the clean install. + +Once you have working reproduction steps, your steps may have details which +aren't actually necessary to reproduce the issue. You may be able to simplify +them by removing some steps or describing steps more narrowly. For help, see +"Simplifying Steps" below. + + +Resources +========= + +We provide some resources which can make it easier to start building steps, or +to simplify steps. + +**Phacility Test Instances**: You can launch a new, up-to-date instance of +Phabricator on Phacility in about a minute at . +These instances run `stable`. + +You can use these instances to make sure that issues haven't already been +fixed, that they reproduce on a clean install, or that your steps are really +complete, and that the root cause isn't custom code or local extensions. Using +a test instance will avoid disrupting other users on your install. + +**Test Repositories**: There are several test repositories on +`secure.phabriactor.com` which you can push commits to if you need to build +an example to demonstrate a problem. + +For example, if you're seeing an issue with a certain filename but the commit +where the problem occurs is in a proprietary internal repository, push a commit +that affects a file with a similar name to a test repository, then reproduce +against the test data. This will allow you to generate steps which anyone can +follow. + + +Simplifying Steps +================= + +If you aren't sure how to simplify reproduction steps, this section has some +advice. + +In general, you'll simplify reproduction steps by first finding a scenario +where the issue reproduces reliably (a "bad" case) and a second, similar +situation where it does not reproduce (a "good" case). Once you have a "bad" +case and a "good" case, you'll change the scenarios step-by-step to be more +similar to each other, until you have two scenarios which differ only a very +small amount. This remaining difference usually points clearly at the root +cause of the issue. + +For example, suppose you notice that you get an error if you commit a file +named `A Banana.doc`, but not if you commit a file named `readme.md`. In +this case, `A Banana.doc` is your "bad" case and `readme.md` is your "good" +case. + +There are several differences between these file names, and any of them might +be causing the problem. To narrow this down, you can try making the scenarios +more similar. For example, do these files work? + + - `A_Banana.doc` - Problem with spaces? + - `A Banana.md` - File extension issue? + - `A Ban.doc` - Path length issue? + - `a banana.doc` - Issue with letter case? + +Some of these probably work, while others might not. These could lead you to a +smaller case which reproduces the problem, which might be something like this: + + - Files like `a b`, which contain spaces, do not work. + - Files like `a.doc`, which have the `.doc` extension, do not work. + - Files like `AAAAAAAAAA`, which have more than 9 letters, do not work. + - Files like `A`, which have uppercase letters, do not work. + +With a simpler reproduction scenario, you can simplify your steps to be more +tailored and mimimal. This will help us pointpoint the issue more quickly and +be more certain that we've understood and resolved it. + +It is more important that steps be complete than that they be simple, and it's +acceptable to submit complex instructions if you have difficulty simplifying +them, so long as they are complete, self-contained, and accurately reproduce +the issue. + + +Things to Avoid +=============== + +These are common mistakes when providing reproduction instructions: + +**Insufficient Information**: The most common issue we see is instructions +which do not have enough information: they are missing critical details which +are necessary in order to follow them on a clean install. + +**Inaccurate Steps**: The second most common issue we see is instructions +which do not actually reproduce a problem when followed on a clean, up-to-date +install. Verify your steps by testing them. + +**Private Information**: Some users provide reports which hinge on the +particulars of private commits in proprietary repositories we can not access. +This is not useful, because we can not examine the underlying commit to figure +out why it is causing an issue. + +Instead, reproduce the issue in a public repository. There are several test +repositories available which you can push commits to in order to construct a +reproduction case. + +**Screenshots**: Screenshots can be helpful to explain a set of steps or show +what you're seeing, but they usually aren't sufficient on their own because +they don't contain all the information we need to reproduce them. + +For example, a screenshot may show a particular policy or object, but not have +enough information for us rebuild a similar object locally. + + +Alternatives +============ + +If you have an issue which you can't build reproduction steps for, or which +only reproduces in your environment, or which you don't want to narrow down +to a minimal reproduction case, we can't accept it as a bug report. These +issues are tremendously time consuming for us to pursue and rarely benefit +more than one install. + +If the issue is important but falls outside the scope of permissible bug +reports, we're happy to provide more tailored support at consulting rates. See +[[ https://secure.phabricator.com/w/consulting/ | Consulting ]] for details. + + +Next Steps +========== + +Continue by: + + - returning to @{article:Contributing Bug Reports}. diff --git a/src/docs/contributor/version.diviner b/src/docs/contributor/version.diviner new file mode 100644 index 0000000000..81b1c845ef --- /dev/null +++ b/src/docs/contributor/version.diviner @@ -0,0 +1,57 @@ +@title Providing Version Information +@group detail + +How to provide version information with reports made to the upstream. + +Overview +======== + +When you submit a bug report, we require that you include version information. + +Despite our insistence that users update before reporting issues, many reports +we receive describe issues which have already been resolved. Including version +information in your report allows us to quickly determine that you are out of +date and that updating will fix your issue. + +That said, your report must also include reproduction steps, and you should be +unable to generate valid reproduction steps for an issue which has already been +resolved because valid reproduction steps must also reproduce against a clean, +up-to-date install. See @{article:Providing Reproduction Steps} for details. + + +Phabricator Version +=================== + +To get Phabricator version information: + + - Go to the {nav Config} application. You can type "Config" into the global + search box, or navigate to `https://your.install.com/config/`. You must + be an administrator to access this application. + - Click {nav Versions} in the left menu. + - Copy and paste all of the information on the page into your report. + + +Arcanist Version +================ + +To get Arcanist version information: + + - Run `arc version`. + - Copy and paste all of the output into your report. + + +Other Versions +============== + +In general, we use `git` commit hashes as version identifiers, so you can +identify the version of something by running `git show` and copy/pasting the +hash from the output. This may be useful if you're encountering an issue which +prevents you from reaching the version reporting screen. + + +Next Steps +========== + +Continue by: + + - returning to @{article:Contributing Bug Reports}. From 7cfc87bbe65e7bf4dcbeb7143669adc662442869 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Mar 2016 05:55:11 -0700 Subject: [PATCH 24/31] Improve rendering of many GitHub event strings Summary: Ref T10538. This makes us render better human-readable descriptions of more GitHub event types. Test Plan: Ran unit tests. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10538 Differential Revision: https://secure.phabricator.com/D15516 --- .../nuance/github/NuanceGitHubRawEvent.php | 172 ++++++++++++++++++ .../NuanceGitHubRawEventTestCase.php | 1 + .../github/__tests__/issueevents/assigned.txt | 3 +- .../github/__tests__/issueevents/closed.txt | 3 +- .../__tests__/issueevents/demilestoned.txt | 3 +- .../github/__tests__/issueevents/labeled.txt | 3 +- .../github/__tests__/issueevents/locked.txt | 3 +- .../__tests__/issueevents/milestoned.txt | 3 +- .../github/__tests__/issueevents/renamed.txt | 3 +- .../github/__tests__/issueevents/reopened.txt | 3 +- .../__tests__/issueevents/unassigned.txt | 3 +- .../__tests__/issueevents/unlabeled.txt | 3 +- .../github/__tests__/issueevents/unlocked.txt | 3 +- .../repositoryevents/CreateEvent.tag.txt | 37 ++++ .../IssueCommentEvent.created.pull.txt | 3 +- .../IssueCommentEvent.created.txt | 3 +- .../repositoryevents/IssuesEvent.closed.txt | 3 +- .../repositoryevents/IssuesEvent.opened.txt | 3 +- .../repositoryevents/IssuesEvent.reopened.txt | 3 +- .../PullRequestEvent.opened.txt | 3 +- .../__tests__/repositoryevents/PushEvent.txt | 3 +- .../repositoryevents/WatchEvent.started.txt | 3 +- .../nuance/item/NuanceGitHubEventItemType.php | 54 +----- 23 files changed, 249 insertions(+), 72 deletions(-) create mode 100644 src/applications/nuance/github/__tests__/repositoryevents/CreateEvent.tag.txt diff --git a/src/applications/nuance/github/NuanceGitHubRawEvent.php b/src/applications/nuance/github/NuanceGitHubRawEvent.php index 2c0d62c7ca..1283fb43b7 100644 --- a/src/applications/nuance/github/NuanceGitHubRawEvent.php +++ b/src/applications/nuance/github/NuanceGitHubRawEvent.php @@ -125,6 +125,11 @@ final class NuanceGitHubRawEvent extends Phobject { } } else { switch ($this->getIssueRawKind()) { + case 'CreateEvent': + $ref = idxv($raw, array('payload', 'ref')); + + $repo = $this->getRepositoryFullRawName(); + return "https://github.com/{$repo}/commits/{$ref}"; case 'PushEvent': // These don't really have a URI since there may be multiple commits // involved and GitHub doesn't bundle the push as an object on its @@ -205,4 +210,171 @@ final class NuanceGitHubRawEvent extends Phobject { return idxv($raw, array('payload', 'issue', 'pull_request')); } + public function getEventFullTitle() { + switch ($this->type) { + case self::TYPE_ISSUE: + $title = $this->getRawIssueEventTitle(); + break; + case self::TYPE_REPOSITORY: + $title = $this->getRawRepositoryEventTitle(); + break; + default: + $title = pht('Unknown Event Type ("%s")', $this->type); + break; + } + + return pht( + 'GitHub %s %s (%s)', + $this->getRepositoryFullRawName(), + $this->getTargetObjectName(), + $title); + } + + private function getTargetObjectName() { + if ($this->isPullRequestEvent()) { + $number = $this->getRawIssueNumber(); + return pht('Pull Request #%d', $number); + } else if ($this->isIssueEvent()) { + $number = $this->getRawIssueNumber(); + return pht('Issue #%d', $number); + } else if ($this->type == self::TYPE_REPOSITORY) { + $raw = $this->raw; + + + $type = idx($raw, 'type'); + switch ($type) { + case 'CreateEvent': + $ref = idxv($raw, array('payload', 'ref')); + $ref_type = idxv($raw, array('payload', 'ref_type')); + + switch ($ref_type) { + case 'branch': + return pht('Branch %s', $ref); + case 'tag': + return pht('Tag %s', $ref); + default: + return pht('Ref %s', $ref); + } + break; + case 'PushEvent': + $ref = idxv($raw, array('payload', 'ref')); + if (preg_match('(^refs/heads/)', $ref)) { + return pht('Branch %s', substr($ref, strlen('refs/heads/'))); + } else { + return pht('Ref %s', $ref); + } + break; + case 'WatchEvent': + $actor = idxv($raw, array('actor', 'login')); + return pht('User %s', $actor); + } + + return pht('Unknown Object'); + } else { + return pht('Unknown Object'); + } + } + + private function getRawIssueEventTitle() { + $raw = $this->raw; + + $action = idxv($raw, array('event')); + switch ($action) { + case 'assigned': + $assignee = idxv($raw, array('assignee', 'login')); + $title = pht('Assigned: %s', $assignee); + break; + case 'closed': + $title = pht('Closed'); + break; + case 'demilestoned': + $milestone = idxv($raw, array('milestone', 'title')); + $title = pht('Removed Milestone: %s', $milestone); + break; + case 'labeled': + $label = idxv($raw, array('label', 'name')); + $title = pht('Added Label: %s', $label); + break; + case 'locked': + $title = pht('Locked'); + break; + case 'milestoned': + $milestone = idxv($raw, array('milestone', 'title')); + $title = pht('Added Milestone: %s', $milestone); + break; + case 'renamed': + $title = pht('Renamed'); + break; + case 'reopened': + $title = pht('Reopened'); + break; + case 'unassigned': + $assignee = idxv($raw, array('assignee', 'login')); + $title = pht('Unassigned: %s', $assignee); + break; + case 'unlabeled': + $label = idxv($raw, array('label', 'name')); + $title = pht('Removed Label: %s', $label); + break; + case 'unlocked': + $title = pht('Unlocked'); + break; + default: + $title = pht('"%s"', $action); + break; + } + + + return $title; + } + + private function getRawRepositoryEventTitle() { + $raw = $this->raw; + + $type = idx($raw, 'type'); + switch ($type) { + case 'CreateEvent': + return pht('Created'); + case 'PushEvent': + $head = idxv($raw, array('payload', 'head')); + $head = substr($head, 0, 12); + return pht('Pushed: %s', $head); + case 'IssuesEvent': + $action = idxv($raw, array('payload', 'action')); + switch ($action) { + case 'closed': + return pht('Closed'); + case 'opened': + return pht('Created'); + case 'reopened': + return pht('Reopened'); + default: + return pht('"%s"', $action); + } + break; + case 'IssueCommentEvent': + $action = idxv($raw, array('payload', 'action')); + switch ($action) { + case 'created': + return pht('Comment'); + default: + return pht('"%s"', $action); + } + break; + case 'PullRequestEvent': + $action = idxv($raw, array('payload', 'action')); + switch ($action) { + case 'opened': + return pht('Created'); + default: + return pht('"%s"', $action); + } + break; + case 'WatchEvent': + return pht('Watched'); + } + + return pht('"%s"', $type); + } + } diff --git a/src/applications/nuance/github/__tests__/NuanceGitHubRawEventTestCase.php b/src/applications/nuance/github/__tests__/NuanceGitHubRawEventTestCase.php index 5ce199cb8c..f5e2119141 100644 --- a/src/applications/nuance/github/__tests__/NuanceGitHubRawEventTestCase.php +++ b/src/applications/nuance/github/__tests__/NuanceGitHubRawEventTestCase.php @@ -50,6 +50,7 @@ final class NuanceGitHubRawEventTestCase 'pull.number' => $event->getPullRequestNumber(), 'id' => $event->getID(), 'uri' => $event->getURI(), + 'title.full' => $event->getEventFullTitle(), ); // Only verify the keys which are actually present in the test. This diff --git a/src/applications/nuance/github/__tests__/issueevents/assigned.txt b/src/applications/nuance/github/__tests__/issueevents/assigned.txt index 7c1c89435f..a126186b19 100644 --- a/src/applications/nuance/github/__tests__/issueevents/assigned.txt +++ b/src/applications/nuance/github/__tests__/issueevents/assigned.txt @@ -112,5 +112,6 @@ "is.pull": false, "issue.number": 1, "id": 583217900, - "uri": "https://github.com/epriestley/poems/issues/1#event-583217900" + "uri": "https://github.com/epriestley/poems/issues/1#event-583217900", + "title.full": "GitHub epriestley/poems Issue #1 (Assigned: epriestley)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/closed.txt b/src/applications/nuance/github/__tests__/issueevents/closed.txt index e13c9a1ffd..6428fc6bcb 100644 --- a/src/applications/nuance/github/__tests__/issueevents/closed.txt +++ b/src/applications/nuance/github/__tests__/issueevents/closed.txt @@ -74,5 +74,6 @@ "is.pull": false, "issue.number": 1, "id": 583218864, - "uri": "https://github.com/epriestley/poems/issues/1#event-583218864" + "uri": "https://github.com/epriestley/poems/issues/1#event-583218864", + "title.full": "GitHub epriestley/poems Issue #1 (Closed)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/demilestoned.txt b/src/applications/nuance/github/__tests__/issueevents/demilestoned.txt index 1c9bab5725..5c32ec6c6c 100644 --- a/src/applications/nuance/github/__tests__/issueevents/demilestoned.txt +++ b/src/applications/nuance/github/__tests__/issueevents/demilestoned.txt @@ -77,5 +77,6 @@ "is.pull": false, "issue.number": 1, "id": 583218613, - "uri": "https://github.com/epriestley/poems/issues/1#event-583218613" + "uri": "https://github.com/epriestley/poems/issues/1#event-583218613", + "title.full": "GitHub epriestley/poems Issue #1 (Removed Milestone: b)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/labeled.txt b/src/applications/nuance/github/__tests__/issueevents/labeled.txt index 92cd7cd4f0..bb88a13b9a 100644 --- a/src/applications/nuance/github/__tests__/issueevents/labeled.txt +++ b/src/applications/nuance/github/__tests__/issueevents/labeled.txt @@ -78,5 +78,6 @@ "is.pull": false, "issue.number": 1, "id": 583217784, - "uri": "https://github.com/epriestley/poems/issues/1#event-583217784" + "uri": "https://github.com/epriestley/poems/issues/1#event-583217784", + "title.full": "GitHub epriestley/poems Issue #1 (Added Label: bug)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/locked.txt b/src/applications/nuance/github/__tests__/issueevents/locked.txt index 536d95af8e..7eafd9e906 100644 --- a/src/applications/nuance/github/__tests__/issueevents/locked.txt +++ b/src/applications/nuance/github/__tests__/issueevents/locked.txt @@ -74,5 +74,6 @@ "is.pull": false, "issue.number": 1, "id": 583218006, - "uri": "https://github.com/epriestley/poems/issues/1#event-583218006" + "uri": "https://github.com/epriestley/poems/issues/1#event-583218006", + "title.full": "GitHub epriestley/poems Issue #1 (Locked)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/milestoned.txt b/src/applications/nuance/github/__tests__/issueevents/milestoned.txt index 748beddda9..3e5a6a4590 100644 --- a/src/applications/nuance/github/__tests__/issueevents/milestoned.txt +++ b/src/applications/nuance/github/__tests__/issueevents/milestoned.txt @@ -77,5 +77,6 @@ "is.pull": false, "issue.number": 1, "id": 583217866, - "uri": "https://github.com/epriestley/poems/issues/1#event-583217866" + "uri": "https://github.com/epriestley/poems/issues/1#event-583217866", + "title.full": "GitHub epriestley/poems Issue #1 (Added Milestone: b)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/renamed.txt b/src/applications/nuance/github/__tests__/issueevents/renamed.txt index e4a4614ebb..08a3c0c448 100644 --- a/src/applications/nuance/github/__tests__/issueevents/renamed.txt +++ b/src/applications/nuance/github/__tests__/issueevents/renamed.txt @@ -78,5 +78,6 @@ "is.pull": false, "issue.number": 1, "id": 583218162, - "uri": "https://github.com/epriestley/poems/issues/1#event-583218162" + "uri": "https://github.com/epriestley/poems/issues/1#event-583218162", + "title.full": "GitHub epriestley/poems Issue #1 (Renamed)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/reopened.txt b/src/applications/nuance/github/__tests__/issueevents/reopened.txt index baab332450..26d5b3ec75 100644 --- a/src/applications/nuance/github/__tests__/issueevents/reopened.txt +++ b/src/applications/nuance/github/__tests__/issueevents/reopened.txt @@ -74,5 +74,6 @@ "is.pull": false, "issue.number": 1, "id": 583218814, - "uri": "https://github.com/epriestley/poems/issues/1#event-583218814" + "uri": "https://github.com/epriestley/poems/issues/1#event-583218814", + "title.full": "GitHub epriestley/poems Issue #1 (Reopened)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/unassigned.txt b/src/applications/nuance/github/__tests__/issueevents/unassigned.txt index 43c610b576..086401afa7 100644 --- a/src/applications/nuance/github/__tests__/issueevents/unassigned.txt +++ b/src/applications/nuance/github/__tests__/issueevents/unassigned.txt @@ -112,5 +112,6 @@ "is.pull": false, "issue.number": 1, "id": 583218511, - "uri": "https://github.com/epriestley/poems/issues/1#event-583218511" + "uri": "https://github.com/epriestley/poems/issues/1#event-583218511", + "title.full": "GitHub epriestley/poems Issue #1 (Unassigned: epriestley)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/unlabeled.txt b/src/applications/nuance/github/__tests__/issueevents/unlabeled.txt index 8d40ba5702..dde464ab15 100644 --- a/src/applications/nuance/github/__tests__/issueevents/unlabeled.txt +++ b/src/applications/nuance/github/__tests__/issueevents/unlabeled.txt @@ -78,5 +78,6 @@ "is.pull": false, "issue.number": 1, "id": 583218703, - "uri": "https://github.com/epriestley/poems/issues/1#event-583218703" + "uri": "https://github.com/epriestley/poems/issues/1#event-583218703", + "title.full": "GitHub epriestley/poems Issue #1 (Removed Label: bug)" } diff --git a/src/applications/nuance/github/__tests__/issueevents/unlocked.txt b/src/applications/nuance/github/__tests__/issueevents/unlocked.txt index 080fbd79e8..580a6170bb 100644 --- a/src/applications/nuance/github/__tests__/issueevents/unlocked.txt +++ b/src/applications/nuance/github/__tests__/issueevents/unlocked.txt @@ -74,5 +74,6 @@ "is.pull": false, "issue.number": 1, "id": 583218062, - "uri": "https://github.com/epriestley/poems/issues/1#event-583218062" + "uri": "https://github.com/epriestley/poems/issues/1#event-583218062", + "title.full": "GitHub epriestley/poems Issue #1 (Unlocked)" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/CreateEvent.tag.txt b/src/applications/nuance/github/__tests__/repositoryevents/CreateEvent.tag.txt new file mode 100644 index 0000000000..f0a8a3a1ad --- /dev/null +++ b/src/applications/nuance/github/__tests__/repositoryevents/CreateEvent.tag.txt @@ -0,0 +1,37 @@ +{ + "id": "3784548642", + "type": "CreateEvent", + "actor": { + "id": 102631, + "login": "epriestley", + "gravatar_id": "", + "url": "https://api.github.com/users/epriestley", + "avatar_url": "https://avatars.githubusercontent.com/u/102631?" + }, + "repo": { + "id": 14627834, + "name": "epriestley/poems", + "url": "https://api.github.com/repos/epriestley/poems" + }, + "payload": { + "ref": "phabricator/diff/400", + "ref_type": "tag", + "master_branch": "master", + "description": "Poems (Mirror)", + "pusher_type": "user" + }, + "public": true, + "created_at": "2016-03-19T22:07:56Z" +} + +~~~~~ +{ + "repository.name.full": "epriestley/poems", + "is.issue": false, + "is.pull": false, + "issue.number": null, + "pull.number": null, + "id": 3784548642, + "uri": "https://github.com/epriestley/poems/commits/phabricator/diff/400", + "title.full": "GitHub epriestley/poems Tag phabricator/diff/400 (Created)" +} diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt index 7b3e7d3708..71abbceac4 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.pull.txt @@ -159,5 +159,6 @@ "issue.number": null, "pull.number": 2, "id": 3740938746, - "uri": "https://github.com/epriestley/poems/pull/2#issuecomment-194282800" + "uri": "https://github.com/epriestley/poems/pull/2#issuecomment-194282800", + "title.full": "GitHub epriestley/poems Pull Request #2 (Comment)" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt index acb40f0971..a1ca094045 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssueCommentEvent.created.txt @@ -96,5 +96,6 @@ "is.pull": false, "issue.number": 1, "id": 3733510485, - "uri": "https://github.com/epriestley/poems/issues/1#issuecomment-193528669" + "uri": "https://github.com/epriestley/poems/issues/1#issuecomment-193528669", + "title.full": "GitHub epriestley/poems Issue #1 (Comment)" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.closed.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.closed.txt index 78fc81be95..6e7d743303 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.closed.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.closed.txt @@ -68,5 +68,6 @@ "is.pull": false, "issue.number": 1, "id": 3740905151, - "uri": "https://github.com/epriestley/poems/issues/1#event-3740905151" + "uri": "https://github.com/epriestley/poems/issues/1#event-3740905151", + "title.full": "GitHub epriestley/poems Issue #1 (Closed)" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.opened.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.opened.txt index 7ba07a591b..0b42f723d5 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.opened.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.opened.txt @@ -68,5 +68,6 @@ "is.pull": false, "issue.number": 1, "id": 3733509737, - "uri": "https://github.com/epriestley/poems/issues/1#event-3733509737" + "uri": "https://github.com/epriestley/poems/issues/1#event-3733509737", + "title.full": "GitHub epriestley/poems Issue #1 (Created)" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.reopened.txt b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.reopened.txt index 797eb84078..3a0c5b7a20 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.reopened.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/IssuesEvent.reopened.txt @@ -68,5 +68,6 @@ "is.pull": false, "issue.number": 1, "id": 3740908680, - "uri": "https://github.com/epriestley/poems/issues/1#event-3740908680" + "uri": "https://github.com/epriestley/poems/issues/1#event-3740908680", + "title.full": "GitHub epriestley/poems Issue #1 (Reopened)" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/PullRequestEvent.opened.txt b/src/applications/nuance/github/__tests__/repositoryevents/PullRequestEvent.opened.txt index c2f892f090..f80649e724 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/PullRequestEvent.opened.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/PullRequestEvent.opened.txt @@ -332,5 +332,6 @@ "issue.number": null, "pull.number": 2, "id": 3740936638, - "uri": "https://github.com/epriestley/poems/pull/2#event-3740936638" + "uri": "https://github.com/epriestley/poems/pull/2#event-3740936638", + "title.full": "GitHub epriestley/poems Pull Request #2 (Created)" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/PushEvent.txt b/src/applications/nuance/github/__tests__/repositoryevents/PushEvent.txt index d7c1b5ecad..c6ecf1bdbc 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/PushEvent.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/PushEvent.txt @@ -43,5 +43,6 @@ "is.pull": false, "issue.number": null, "id": 3498724127, - "uri": "https://github.com/epriestley/poems/commits/c829132d37c4c1da80d319942a5a1e500632b52f" + "uri": "https://github.com/epriestley/poems/commits/c829132d37c4c1da80d319942a5a1e500632b52f", + "title.full": "GitHub epriestley/poems Branch master (Pushed: c829132d37c4)" } diff --git a/src/applications/nuance/github/__tests__/repositoryevents/WatchEvent.started.txt b/src/applications/nuance/github/__tests__/repositoryevents/WatchEvent.started.txt index c65a5ee771..c23678f448 100644 --- a/src/applications/nuance/github/__tests__/repositoryevents/WatchEvent.started.txt +++ b/src/applications/nuance/github/__tests__/repositoryevents/WatchEvent.started.txt @@ -27,5 +27,6 @@ "issue.number": null, "pull.number": null, "id": 3740950917, - "uri": null + "uri": null, + "title.full": "GitHub epriestley/poems User epriestley (Watched)" } diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php index 1c6d90bbf3..2bccfb76b4 100644 --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -16,59 +16,7 @@ final class NuanceGitHubEventItemType } public function getItemDisplayName(NuanceItem $item) { - $api_type = $item->getItemProperty('api.type'); - switch ($api_type) { - case 'issue': - return $this->getGitHubIssueAPIEventDisplayName($item); - case 'repository': - return $this->getGitHubRepositoryAPIEventDisplayName($item); - default: - return pht('GitHub Event (Unknown API Type "%s")', $api_type); - } - } - - private function getGitHubIssueAPIEventDisplayName(NuanceItem $item) { - $raw = $item->getItemProperty('api.raw', array()); - - $action = idxv($raw, array('event')); - $number = idxv($raw, array('issue', 'number')); - - return pht('GitHub Issue #%d (%s)', $number, $action); - } - - private function getGitHubRepositoryAPIEventDisplayName(NuanceItem $item) { - $raw = $item->getItemProperty('api.raw', array()); - - $repo = idxv($raw, array('repo', 'name'), pht('')); - - $type = idx($raw, 'type'); - switch ($type) { - case 'PushEvent': - $head = idxv($raw, array('payload', 'head')); - $head = substr($head, 0, 8); - $name = pht('Push %s', $head); - break; - case 'IssuesEvent': - $action = idxv($raw, array('payload', 'action')); - $number = idxv($raw, array('payload', 'issue', 'number')); - $name = pht('Issue #%d (%s)', $number, $action); - break; - case 'IssueCommentEvent': - $action = idxv($raw, array('payload', 'action')); - $number = idxv($raw, array('payload', 'issue', 'number')); - $name = pht('Issue #%d (Comment, %s)', $number, $action); - break; - case 'PullRequestEvent': - $action = idxv($raw, array('payload', 'action')); - $number = idxv($raw, array('payload', 'pull_request', 'number')); - $name = pht('Pull Request #%d (%s)', $number, $action); - break; - default: - $name = pht('Unknown Event ("%s")', $type); - break; - } - - return pht('GitHub %s %s', $repo, $name); + return $this->newRawEvent($item)->getEventFullTitle(); } public function canUpdateItems() { From 3493d9d5138e0a630624bced548090163ce9be8a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Mar 2016 09:09:05 -0700 Subject: [PATCH 25/31] Fix a typo Summary: Whoops. Test Plan: O.o Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D15519 --- src/docs/contributor/reproduction_steps.diviner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/docs/contributor/reproduction_steps.diviner b/src/docs/contributor/reproduction_steps.diviner index 4ce17b5aff..d5d58913b7 100644 --- a/src/docs/contributor/reproduction_steps.diviner +++ b/src/docs/contributor/reproduction_steps.diviner @@ -144,7 +144,7 @@ complete, and that the root cause isn't custom code or local extensions. Using a test instance will avoid disrupting other users on your install. **Test Repositories**: There are several test repositories on -`secure.phabriactor.com` which you can push commits to if you need to build +`secure.phabricator.com` which you can push commits to if you need to build an example to demonstrate a problem. For example, if you're seeing an issue with a certain filename but the commit From 4dc857e36d54aa52735af93592906752651aa5ed Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 25 Mar 2016 05:05:08 -0700 Subject: [PATCH 26/31] Fix an issue with incorrect split head detection in Mercurial after pushing a medley of varied changes Summary: Fixes T10665. See that task for discussion. Because `$head_map` is not properly re-initialized for each ref we check, pushes which affect multiple branches (say, "A" and "B") can have information bleed from the first branch check to the second branch. To trigger a problem behavior, you can push one commit which updates an existing branch, plus one commit which creates a new branch. If they process in the right order, the `$head_map` from the updated branch will bleed into the `$head_map` for the new branch and trigger an incorrect head split detection. Test Plan: - Pushed a set of changes which updated `branch-a` and created `branch-b`. - Before change: improper detection of split heads. - After change: clean push. - Pushed a set of changes which split the head of `branch-d`. - Correct detection of split heads. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10665 Differential Revision: https://secure.phabricator.com/D15522 --- .../diffusion/engine/DiffusionCommitHookEngine.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index 53dc417be4..857ae76da3 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -770,10 +770,10 @@ final class DiffusionCommitHookEngine extends Phobject { } $stray_heads = array(); + $head_map = array(); if ($old_heads && !$new_heads) { // This is a branch deletion with "--close-branch". - $head_map = array(); foreach ($old_heads as $old_head) { $head_map[$old_head] = array(self::EMPTY_HASH); } @@ -798,7 +798,6 @@ final class DiffusionCommitHookEngine extends Phobject { '{node}\1'); } - $head_map = array(); foreach (new FutureIterator($dfutures) as $future_head => $dfuture) { list($stdout) = $dfuture->resolvex(); $descendant_heads = array_filter(explode("\1", $stdout)); From 0856a36e97c51015b4f0687eddb538a5c934eca2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 25 Mar 2016 05:29:19 -0700 Subject: [PATCH 27/31] When an object has been imported from an external source, show a curtain panel Summary: Ref T10537. Show when an object is bridged to something external. Test Plan: {F1190099} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10537 Differential Revision: https://secure.phabricator.com/D15520 --- src/__phutil_library_map__.php | 2 ++ ...oorkeeperBridgedObjectCurtainExtension.php | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 src/applications/doorkeeper/engineextension/DoorkeeperBridgedObjectCurtainExtension.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3932c6a4c1..c4771045bb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -845,6 +845,7 @@ phutil_register_library_map(array( 'DoorkeeperBridgeGitHubIssue' => 'applications/doorkeeper/bridge/DoorkeeperBridgeGitHubIssue.php', 'DoorkeeperBridgeJIRA' => 'applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php', 'DoorkeeperBridgeJIRATestCase' => 'applications/doorkeeper/bridge/__tests__/DoorkeeperBridgeJIRATestCase.php', + 'DoorkeeperBridgedObjectCurtainExtension' => 'applications/doorkeeper/engineextension/DoorkeeperBridgedObjectCurtainExtension.php', 'DoorkeeperBridgedObjectInterface' => 'applications/doorkeeper/bridge/DoorkeeperBridgedObjectInterface.php', 'DoorkeeperDAO' => 'applications/doorkeeper/storage/DoorkeeperDAO.php', 'DoorkeeperExternalObject' => 'applications/doorkeeper/storage/DoorkeeperExternalObject.php', @@ -5001,6 +5002,7 @@ phutil_register_library_map(array( 'DoorkeeperBridgeGitHubIssue' => 'DoorkeeperBridgeGitHub', 'DoorkeeperBridgeJIRA' => 'DoorkeeperBridge', 'DoorkeeperBridgeJIRATestCase' => 'PhabricatorTestCase', + 'DoorkeeperBridgedObjectCurtainExtension' => 'PHUICurtainExtension', 'DoorkeeperDAO' => 'PhabricatorLiskDAO', 'DoorkeeperExternalObject' => array( 'DoorkeeperDAO', diff --git a/src/applications/doorkeeper/engineextension/DoorkeeperBridgedObjectCurtainExtension.php b/src/applications/doorkeeper/engineextension/DoorkeeperBridgedObjectCurtainExtension.php new file mode 100644 index 0000000000..ae59440973 --- /dev/null +++ b/src/applications/doorkeeper/engineextension/DoorkeeperBridgedObjectCurtainExtension.php @@ -0,0 +1,31 @@ +getBridgedObject(); + if (!$xobj) { + return null; + } + + $tag = id(new DoorkeeperTagView()) + ->setExternalObject($xobj); + + return $this->newPanel() + ->setHeaderText(pht('Imported From')) + ->setOrder(5000) + ->appendChild($tag); + } + +} From d76652b3317874403a09ed2ec3b7f1d46ead5848 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 25 Mar 2016 21:41:47 +0000 Subject: [PATCH 28/31] Update Harbormaster for two column Summary: Updates the Harbormaster UI to match the new two column everywhere else. Test Plan: Did best I could, tested builds, plans, steps, buildables. Unable to test lint/unit locally, I need to set that up. Kick the tires for me pls. :3 Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D15523 --- resources/celerity/map.php | 6 +- .../DifferentialRevisionViewController.php | 1 - .../HarbormasterBuildViewController.php | 133 +++++++++--------- .../HarbormasterBuildableViewController.php | 67 +++++---- .../HarbormasterLintMessagesController.php | 19 ++- .../HarbormasterPlanViewController.php | 91 +++++------- .../HarbormasterStepAddController.php | 44 +++--- .../HarbormasterStepEditController.php | 28 ++-- .../HarbormasterStepViewController.php | 51 +++---- .../HarbormasterUnitMessageListController.php | 19 ++- .../HarbormasterUnitMessageViewController.php | 31 ++-- .../view/HarbormasterUnitSummaryView.php | 13 +- webroot/rsrc/css/phui/phui-timeline-view.css | 2 +- 13 files changed, 261 insertions(+), 244 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index a89bebd270..b2125badda 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'befbf333', + 'core.pkg.css' => '9acdee84', 'core.pkg.js' => '7d8faf57', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '7ba78475', @@ -155,7 +155,7 @@ return array( 'rsrc/css/phui/phui-spacing.css' => '042804d6', 'rsrc/css/phui/phui-status.css' => '37309046', 'rsrc/css/phui/phui-tag-view.css' => '6bbd83e2', - 'rsrc/css/phui/phui-timeline-view.css' => 'a0173eba', + 'rsrc/css/phui/phui-timeline-view.css' => '6e342216', 'rsrc/css/phui/phui-two-column-view.css' => '9c43b599', 'rsrc/css/phui/workboards/phui-workboard-color.css' => 'ac6fe6a7', 'rsrc/css/phui/workboards/phui-workboard.css' => 'e6d89647', @@ -845,7 +845,7 @@ return array( 'phui-status-list-view-css' => '37309046', 'phui-tag-view-css' => '6bbd83e2', 'phui-theme-css' => '027ba77e', - 'phui-timeline-view-css' => 'a0173eba', + 'phui-timeline-view-css' => '6e342216', 'phui-two-column-view-css' => '9c43b599', 'phui-workboard-color-css' => 'ac6fe6a7', 'phui-workboard-view-css' => 'e6d89647', diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 75602af0a6..61fffd9b2a 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1166,7 +1166,6 @@ final class DifferentialRevisionViewController extends DifferentialController { ->setBuildable($diff->getBuildable()) ->setUnitMessages($diff->getUnitMessages()) ->setLimit(5) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setShowViewAll(true); } diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php index 25f3506c0b..7e22d05471 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -22,30 +22,33 @@ final class HarbormasterBuildViewController $title = pht('Build %d', $id); - $header = id(new PHUIHeaderView()) + $page_header = id(new PHUIHeaderView()) ->setHeader($title) ->setUser($viewer) - ->setPolicyObject($build); + ->setPolicyObject($build) + ->setHeaderIcon('fa-cubes'); if ($build->isRestarting()) { - $header->setStatus('fa-exclamation-triangle', 'red', pht('Restarting')); + $page_header->setStatus( + 'fa-exclamation-triangle', 'red', pht('Restarting')); } else if ($build->isPausing()) { - $header->setStatus('fa-exclamation-triangle', 'red', pht('Pausing')); + $page_header->setStatus( + 'fa-exclamation-triangle', 'red', pht('Pausing')); } else if ($build->isResuming()) { - $header->setStatus('fa-exclamation-triangle', 'red', pht('Resuming')); + $page_header->setStatus( + 'fa-exclamation-triangle', 'red', pht('Resuming')); } else if ($build->isAborting()) { - $header->setStatus('fa-exclamation-triangle', 'red', pht('Aborting')); + $page_header->setStatus( + 'fa-exclamation-triangle', 'red', pht('Aborting')); } - $box = id(new PHUIObjectBoxView()) - ->setHeader($header); - - $actions = $this->buildActionList($build); - $this->buildPropertyLists($box, $build, $actions); + $curtain = $this->buildCurtainView($build); + $properties = $this->buildPropertyList($build); $crumbs = $this->buildApplicationCrumbs(); $this->addBuildableCrumb($crumbs, $build->getBuildable()); $crumbs->addTextCrumb($title); + $crumbs->setBorder(true); if ($generation === null || $generation > $build->getBuildGeneration() || $generation < 0) { @@ -85,12 +88,14 @@ final class HarbormasterBuildViewController foreach ($build_targets as $build_target) { $header = id(new PHUIHeaderView()) ->setHeader($build_target->getName()) - ->setUser($viewer); + ->setUser($viewer) + ->setHeaderIcon('fa-bullseye'); $target_box = id(new PHUIObjectBoxView()) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setHeader($header); - $properties = new PHUIPropertyListView(); + $property_list = new PHUIPropertyListView(); $target_artifacts = idx($artifacts, $build_target->getPHID(), array()); @@ -107,13 +112,12 @@ final class HarbormasterBuildViewController if ($links) { $links = phutil_implode_html(phutil_tag('br'), $links); - $properties->addProperty( + $property_list->addProperty( pht('External Link'), $links); } $status_view = new PHUIStatusListView(); - $item = new PHUIStatusItemView(); $status = $build_target->getTargetStatus(); @@ -168,13 +172,13 @@ final class HarbormasterBuildViewController } } - $properties->addProperty( + $property_list->addProperty( pht('When'), phutil_implode_html(" \xC2\xB7 ", $when)); - $properties->addProperty(pht('Status'), $status_view); + $property_list->addProperty(pht('Status'), $status_view); - $target_box->addPropertyList($properties, pht('Overview')); + $target_box->addPropertyList($property_list, pht('Overview')); $step = $build_target->getBuildStep(); @@ -182,9 +186,9 @@ final class HarbormasterBuildViewController $description = $step->getDescription(); if ($description) { $description = new PHUIRemarkupView($viewer, $description); - $properties->addSectionHeader( + $property_list->addSectionHeader( pht('Description'), PHUIPropertyListView::ICON_SUMMARY); - $properties->addTextContent($description); + $property_list->addTextContent($description); } } else { $target_box->setFormErrors( @@ -196,35 +200,35 @@ final class HarbormasterBuildViewController } $details = $build_target->getDetails(); - $properties = new PHUIPropertyListView(); + $property_list = new PHUIPropertyListView(); foreach ($details as $key => $value) { - $properties->addProperty($key, $value); + $property_list->addProperty($key, $value); } - $target_box->addPropertyList($properties, pht('Configuration')); + $target_box->addPropertyList($property_list, pht('Configuration')); $variables = $build_target->getVariables(); - $properties = new PHUIPropertyListView(); - $properties->addRawContent($this->buildProperties($variables)); - $target_box->addPropertyList($properties, pht('Variables')); + $property_list = new PHUIPropertyListView(); + $property_list->addRawContent($this->buildProperties($variables)); + $target_box->addPropertyList($property_list, pht('Variables')); $artifacts_tab = $this->buildArtifacts($build_target, $target_artifacts); - $properties = new PHUIPropertyListView(); - $properties->addRawContent($artifacts_tab); - $target_box->addPropertyList($properties, pht('Artifacts')); + $property_list = new PHUIPropertyListView(); + $property_list->addRawContent($artifacts_tab); + $target_box->addPropertyList($property_list, pht('Artifacts')); $build_messages = idx($messages, $build_target->getPHID(), array()); - $properties = new PHUIPropertyListView(); - $properties->addRawContent($this->buildMessages($build_messages)); - $target_box->addPropertyList($properties, pht('Messages')); + $property_list = new PHUIPropertyListView(); + $property_list->addRawContent($this->buildMessages($build_messages)); + $target_box->addPropertyList($property_list, pht('Messages')); - $properties = new PHUIPropertyListView(); - $properties->addProperty( + $property_list = new PHUIPropertyListView(); + $property_list->addProperty( pht('Build Target ID'), $build_target->getID()); - $properties->addProperty( + $property_list->addProperty( pht('Build Target PHID'), $build_target->getPHID()); - $target_box->addPropertyList($properties, pht('Metadata')); + $target_box->addPropertyList($property_list, pht('Metadata')); $targets[] = $target_box; @@ -236,16 +240,20 @@ final class HarbormasterBuildViewController new HarbormasterBuildTransactionQuery()); $timeline->setShouldTerminate(true); - return $this->buildApplicationPage( - array( - $crumbs, - $box, + $view = id(new PHUITwoColumnView()) + ->setHeader($page_header) + ->setCurtain($curtain) + ->setMainColumn(array( + $properties, $targets, $timeline, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } private function buildArtifacts( @@ -342,6 +350,7 @@ final class HarbormasterBuildViewController $log_box = id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setForm($log_view); if ($is_empty) { @@ -432,14 +441,11 @@ final class HarbormasterBuildViewController )); } - private function buildActionList(HarbormasterBuild $build) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + private function buildCurtainView(HarbormasterBuild $build) { + $viewer = $this->getViewer(); $id = $build->getID(); - $list = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->setObject($build); + $curtain = $this->newCurtainView($build); $can_restart = $build->canRestartBuild() && @@ -465,7 +471,7 @@ final class HarbormasterBuildViewController $viewer, HarbormasterBuildCommand::COMMAND_ABORT); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Restart Build')) ->setIcon('fa-repeat') @@ -474,7 +480,7 @@ final class HarbormasterBuildViewController ->setWorkflow(true)); if ($build->canResumeBuild()) { - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Resume Build')) ->setIcon('fa-play') @@ -482,7 +488,7 @@ final class HarbormasterBuildViewController ->setDisabled(!$can_resume) ->setWorkflow(true)); } else { - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Pause Build')) ->setIcon('fa-pause') @@ -491,7 +497,7 @@ final class HarbormasterBuildViewController ->setWorkflow(true)); } - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Abort Build')) ->setIcon('fa-exclamation-triangle') @@ -499,21 +505,14 @@ final class HarbormasterBuildViewController ->setDisabled(!$can_abort) ->setWorkflow(true)); - return $list; + return $curtain; } - private function buildPropertyLists( - PHUIObjectBoxView $box, - HarbormasterBuild $build, - PhabricatorActionListView $actions) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + private function buildPropertyList(HarbormasterBuild $build) { + $viewer = $this->getViewer(); $properties = id(new PHUIPropertyListView()) - ->setUser($viewer) - ->setObject($build) - ->setActionList($actions); - $box->addPropertyList($properties); + ->setUser($viewer); $handles = id(new PhabricatorHandleQuery()) ->setViewer($viewer) @@ -538,6 +537,12 @@ final class HarbormasterBuildViewController $properties->addProperty( pht('Status'), $this->getStatus($build)); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('PROPERTIES')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($properties); + } private function getStatus(HarbormasterBuild $build) { diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php index 22d598431f..fe124367df 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php @@ -35,44 +35,44 @@ final class HarbormasterBuildableViewController $header = id(new PHUIHeaderView()) ->setHeader($title) ->setUser($viewer) - ->setPolicyObject($buildable); - - $box = id(new PHUIObjectBoxView()) - ->setHeader($header); + ->setPolicyObject($buildable) + ->setHeaderIcon('fa-recycle'); $timeline = $this->buildTransactionTimeline( $buildable, new HarbormasterBuildableTransactionQuery()); $timeline->setShouldTerminate(true); - $actions = $this->buildActionList($buildable); - $this->buildPropertyLists($box, $buildable, $actions); + $curtain = $this->buildCurtainView($buildable); + $properties = $this->buildPropertyList($buildable); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($buildable->getMonogram()); + $crumbs->setBorder(true); - return $this->buildApplicationPage( - array( - $crumbs, - $box, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $properties, $lint, $unit, $build_list, $timeline, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } - private function buildActionList(HarbormasterBuildable $buildable) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + private function buildCurtainView(HarbormasterBuildable $buildable) { + $viewer = $this->getViewer(); $id = $buildable->getID(); - $list = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->setObject($buildable); + $curtain = $this->newCurtainView($buildable); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, @@ -117,7 +117,7 @@ final class HarbormasterBuildableViewController $resume_uri = "buildable/{$id}/resume/"; $abort_uri = "buildable/{$id}/abort/"; - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setIcon('fa-repeat') ->setName(pht('Restart All Builds')) @@ -125,7 +125,7 @@ final class HarbormasterBuildableViewController ->setWorkflow(true) ->setDisabled(!$can_restart || !$can_edit)); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setIcon('fa-pause') ->setName(pht('Pause All Builds')) @@ -133,7 +133,7 @@ final class HarbormasterBuildableViewController ->setWorkflow(true) ->setDisabled(!$can_pause || !$can_edit)); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setIcon('fa-play') ->setName(pht('Resume All Builds')) @@ -141,7 +141,7 @@ final class HarbormasterBuildableViewController ->setWorkflow(true) ->setDisabled(!$can_resume || !$can_edit)); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setIcon('fa-exclamation-triangle') ->setName(pht('Abort All Builds')) @@ -149,21 +149,14 @@ final class HarbormasterBuildableViewController ->setWorkflow(true) ->setDisabled(!$can_abort || !$can_edit)); - return $list; + return $curtain; } - private function buildPropertyLists( - PHUIObjectBoxView $box, - HarbormasterBuildable $buildable, - PhabricatorActionListView $actions) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + private function buildPropertyList(HarbormasterBuildable $buildable) { + $viewer = $this->getViewer(); $properties = id(new PHUIPropertyListView()) - ->setUser($viewer) - ->setObject($buildable) - ->setActionList($actions); - $box->addPropertyList($properties); + ->setUser($viewer); $container_phid = $buildable->getContainerPHID(); $buildable_phid = $buildable->getBuildablePHID(); @@ -184,6 +177,10 @@ final class HarbormasterBuildableViewController ? pht('Manual Buildable') : pht('Automatic Buildable')); + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('PROPERTIES')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($properties); } private function buildBuildList(HarbormasterBuildable $buildable) { @@ -279,6 +276,7 @@ final class HarbormasterBuildableViewController $box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Builds')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($build_list); return $box; @@ -330,6 +328,7 @@ final class HarbormasterBuildableViewController $lint = id(new PHUIObjectBoxView()) ->setHeader($lint_header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($lint_table); } else { $lint = null; diff --git a/src/applications/harbormaster/controller/HarbormasterLintMessagesController.php b/src/applications/harbormaster/controller/HarbormasterLintMessagesController.php index befe0e450b..e7636dbbd8 100644 --- a/src/applications/harbormaster/controller/HarbormasterLintMessagesController.php +++ b/src/applications/harbormaster/controller/HarbormasterLintMessagesController.php @@ -45,20 +45,27 @@ final class HarbormasterLintMessagesController $crumbs = $this->buildApplicationCrumbs(); $this->addBuildableCrumb($crumbs, $buildable); $crumbs->addTextCrumb(pht('Lint')); + $crumbs->setBorder(true); $title = array( $buildable->getMonogram(), pht('Lint'), ); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader($title); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $lint, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } } diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php index 847eaa6328..28bd16456d 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -24,50 +24,50 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { $header = id(new PHUIHeaderView()) ->setHeader($plan->getName()) ->setUser($viewer) - ->setPolicyObject($plan); + ->setPolicyObject($plan) + ->setHeaderIcon('fa-ship'); - $box = id(new PHUIObjectBoxView()) - ->setHeader($header); - - $actions = $this->buildActionList($plan); - $this->buildPropertyLists($box, $plan, $actions); + $curtain = $this->buildCurtainView($plan); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('Plan %d', $id)); + $crumbs->setBorder(true); list($step_list, $has_any_conflicts, $would_deadlock) = $this->buildStepList($plan); + $error = null; if ($would_deadlock) { - $box->setFormErrors( - array( - pht( - 'This build plan will deadlock when executed, due to '. - 'circular dependencies present in the build plan. '. - 'Examine the step list and resolve the deadlock.'), - )); + $error = pht('This build plan will deadlock when executed, due to '. + 'circular dependencies present in the build plan. '. + 'Examine the step list and resolve the deadlock.'); } else if ($has_any_conflicts) { // A deadlocking build will also cause all the artifacts to be // invalid, so we just skip showing this message if that's the // case. - $box->setFormErrors( - array( - pht( - 'This build plan has conflicts in one or more build steps. '. - 'Examine the step list and resolve the listed errors.'), - )); + $error = pht('This build plan has conflicts in one or more build steps. '. + 'Examine the step list and resolve the listed errors.'); } - return $this->buildApplicationPage( - array( - $crumbs, - $box, + if ($error) { + $error = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->appendChild($error); + } + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $error, $step_list, $timeline, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); } private function buildStepList(HarbormasterBuildPlan $plan) { @@ -206,25 +206,24 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { $step_box = id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($step_list); return array($step_box, $has_any_conflicts, $is_deadlocking); } - private function buildActionList(HarbormasterBuildPlan $plan) { + private function buildCurtainView(HarbormasterBuildPlan $plan) { $viewer = $this->getViewer(); $id = $plan->getID(); - $list = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->setObject($plan); + $curtain = $this->newCurtainView($plan); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $plan, PhabricatorPolicyCapability::CAN_EDIT); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Plan')) ->setHref($this->getApplicationURI("plan/edit/{$id}/")) @@ -233,7 +232,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { ->setIcon('fa-pencil')); if ($plan->isDisabled()) { - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Enable Plan')) ->setHref($this->getApplicationURI("plan/disable/{$id}/")) @@ -241,7 +240,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { ->setDisabled(!$can_edit) ->setIcon('fa-check')); } else { - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Disable Plan')) ->setHref($this->getApplicationURI("plan/disable/{$id}/")) @@ -252,7 +251,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { $can_run = ($can_edit && $plan->canRunManually()); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Run Plan Manually')) ->setHref($this->getApplicationURI("plan/run/{$id}/")) @@ -260,26 +259,12 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { ->setDisabled(!$can_run) ->setIcon('fa-play-circle')); - return $list; - } - - private function buildPropertyLists( - PHUIObjectBoxView $box, - HarbormasterBuildPlan $plan, - PhabricatorActionListView $actions) { - $request = $this->getRequest(); - $viewer = $request->getUser(); - - $properties = id(new PHUIPropertyListView()) - ->setUser($viewer) - ->setObject($plan) - ->setActionList($actions); - $box->addPropertyList($properties); - - $properties->addProperty( - pht('Created'), - phabricator_datetime($plan->getDateCreated(), $viewer)); + $curtain->addPanel( + id(new PHUICurtainPanelView()) + ->setHeaderText(pht('Created')) + ->appendChild(phabricator_datetime($plan->getDateCreated(), $viewer))); + return $curtain; } private function buildArtifactList( diff --git a/src/applications/harbormaster/controller/HarbormasterStepAddController.php b/src/applications/harbormaster/controller/HarbormasterStepAddController.php index 06412e2323..94471d7f0d 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepAddController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepAddController.php @@ -40,15 +40,13 @@ final class HarbormasterStepAddController } $groups = mgroup($all, 'getBuildStepGroupKey'); - $lists = array(); + $boxes = array(); $enabled_groups = HarbormasterBuildStepGroup::getAllEnabledGroups(); foreach ($enabled_groups as $group) { $list = id(new PHUIObjectItemListView()) - ->setHeader($group->getGroupName()) ->setNoDataString( - pht( - 'This group has no available build steps.')); + pht('This group has no available build steps.')); $steps = idx($groups, $group->getGroupKey(), array()); @@ -76,28 +74,36 @@ final class HarbormasterStepAddController $list->addItem($item); } - $lists[] = $list; + $box = id(new PHUIObjectBoxView()) + ->setHeaderText($group->getGroupName()) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($list); + + $boxes[] = $box; } $crumbs = $this->buildApplicationCrumbs() ->addTextCrumb($plan_title, $cancel_uri) - ->addTextCrumb(pht('Add Build Step')); + ->addTextCrumb(pht('Add Build Step')) + ->setBorder(true); - $box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Add Build Step')) - ->appendChild($lists); + $title = array($plan_title, pht('Add Build Step')); - return $this->buildApplicationPage( - array( - $crumbs, - $box, - ), - array( - 'title' => array( - $plan_title, - pht('Add Build Step'), - ), + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Add Build Step')) + ->setHeaderIcon('fa-plus-square'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( + $boxes, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } } diff --git a/src/applications/harbormaster/controller/HarbormasterStepEditController.php b/src/applications/harbormaster/controller/HarbormasterStepEditController.php index 10f35e5fe4..8cd91bb5a4 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php @@ -184,14 +184,19 @@ final class HarbormasterStepEditController if ($is_new) { $submit = pht('Create Build Step'); - $header = pht('New Step: %s', $implementation->getName()); + $header = id(new PHUIHeaderView()) + ->setHeader(pht('New Step: %s', $implementation->getName())) + ->setHeaderIcon('fa-plus-square'); $crumbs->addTextCrumb(pht('Add Step')); } else { $submit = pht('Save Build Step'); - $header = pht('Edit Step: %s', $implementation->getName()); + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Edit Step: %s', $implementation->getName())) + ->setHeaderIcon('fa-pencil'); $crumbs->addTextCrumb(pht('Step %d', $step->getID()), $cancel_uri); $crumbs->addTextCrumb(pht('Edit Step')); } + $crumbs->setBorder(true); $form->appendChild( id(new AphrontFormSubmitControl()) @@ -199,8 +204,9 @@ final class HarbormasterStepEditController ->addCancelButton($cancel_uri)); $box = id(new PHUIObjectBoxView()) - ->setHeaderText($header) + ->setHeaderText(pht('Step')) ->setValidationException($validation_exception) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setForm($form); $variables = $this->renderBuildVariablesTable(); @@ -215,16 +221,19 @@ final class HarbormasterStepEditController $timeline->setShouldTerminate(true); } - return $this->buildApplicationPage( - array( - $crumbs, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $box, $variables, $timeline, - ), - array( - 'title' => $implementation->getName(), )); + + return $this->newPage() + ->setTitle($implementation->getName()) + ->setCrumbs($crumbs) + ->appendChild($view); + } private function renderBuildVariablesTable() { @@ -254,6 +263,7 @@ final class HarbormasterStepEditController return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Build Variables')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($form); } diff --git a/src/applications/harbormaster/controller/HarbormasterStepViewController.php b/src/applications/harbormaster/controller/HarbormasterStepViewController.php index faa4b62c7e..07be537fa9 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepViewController.php @@ -29,30 +29,33 @@ final class HarbormasterStepViewController $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('Plan %d', $plan_id), $plan_uri); $crumbs->addTextCrumb(pht('Step %d', $id)); + $crumbs->setBorder(true); - $box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Build Step %d: %s', $id, $step->getName())); + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Build Step %d: %s', $id, $step->getName())) + ->setHeaderIcon('fa-chevron-circle-right'); $properties = $this->buildPropertyList($step, $field_list); - $actions = $this->buildActionList($step); - $properties->setActionList($actions); - - $box->addPropertyList($properties); + $curtain = $this->buildCurtainView($step); $timeline = $this->buildTransactionTimeline( $step, new HarbormasterBuildStepTransactionQuery()); $timeline->setShouldTerminate(true); - return $this->buildApplicationPage( - array( - $crumbs, - $box, + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $properties, $timeline, - ), - array( - 'title' => pht('Step %d', $id), )); + + return $this->newPage() + ->setTitle(pht('Step %d', $id)) + ->setCrumbs($crumbs) + ->appendChild($view); + } private function buildPropertyList( @@ -61,8 +64,7 @@ final class HarbormasterStepViewController $viewer = $this->getViewer(); $view = id(new PHUIPropertyListView()) - ->setUser($viewer) - ->setObject($step); + ->setUser($viewer); try { $implementation = $step->getStepImplementation(); @@ -92,8 +94,6 @@ final class HarbormasterStepViewController $viewer, $view); - $view->invokeWillRenderEvent(); - $description = $step->getDescription(); if (strlen($description)) { $view->addSectionHeader( @@ -103,24 +103,25 @@ final class HarbormasterStepViewController new PHUIRemarkupView($viewer, $description)); } - return $view; + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('PROPERTIES')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($view); } - private function buildActionList(HarbormasterBuildStep $step) { + private function buildCurtainView(HarbormasterBuildStep $step) { $viewer = $this->getViewer(); $id = $step->getID(); - $list = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->setObject($step); + $curtain = $this->newCurtainView($step); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $step, PhabricatorPolicyCapability::CAN_EDIT); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Step')) ->setHref($this->getApplicationURI("step/edit/{$id}/")) @@ -128,7 +129,7 @@ final class HarbormasterStepViewController ->setDisabled(!$can_edit) ->setIcon('fa-pencil')); - $list->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Delete Step')) ->setHref($this->getApplicationURI("step/delete/{$id}/")) @@ -136,7 +137,7 @@ final class HarbormasterStepViewController ->setDisabled(!$can_edit) ->setIcon('fa-times')); - return $list; + return $curtain; } diff --git a/src/applications/harbormaster/controller/HarbormasterUnitMessageListController.php b/src/applications/harbormaster/controller/HarbormasterUnitMessageListController.php index 2e7f20a71e..05f06a4e99 100644 --- a/src/applications/harbormaster/controller/HarbormasterUnitMessageListController.php +++ b/src/applications/harbormaster/controller/HarbormasterUnitMessageListController.php @@ -41,20 +41,27 @@ final class HarbormasterUnitMessageListController $crumbs = $this->buildApplicationCrumbs(); $this->addBuildableCrumb($crumbs, $buildable); $crumbs->addTextCrumb(pht('Unit Tests')); + $crumbs->setBorder(true); $title = array( $buildable->getMonogram(), pht('Unit Tests'), ); - return $this->buildApplicationPage( - array( - $crumbs, + $header = id(new PHUIHeaderView()) + ->setHeader($buildable->getMonogram().' '.pht('Unit Tests')); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( $unit, - ), - array( - 'title' => $title, )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } } diff --git a/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php b/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php index 652c5a2e23..d344422dd5 100644 --- a/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php @@ -38,9 +38,7 @@ final class HarbormasterUnitMessageViewController ->setStatus($status_icon, $status_color, $status_label); $properties = $this->buildPropertyListView($message); - $actions = $this->buildActionView($message, $build); - - $properties->setActionList($actions); + $curtain = $this->buildCurtainView($message, $build); $unit = id(new PHUIObjectBoxView()) ->setHeader($header) @@ -54,22 +52,29 @@ final class HarbormasterUnitMessageViewController "/harbormaster/unit/{$buildable_id}/"); $crumbs->addTextCrumb(pht('Unit %d', $id)); + $crumbs->setBorder(true); $title = array( $display_name, $buildable->getMonogram(), ); + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $unit, + )); + return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) - ->appendChild($unit); + ->appendChild($view); } private function buildPropertyListView( HarbormasterBuildUnitMessage $message) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $view = id(new PHUIPropertyListView()) ->setUser($viewer); @@ -81,6 +86,7 @@ final class HarbormasterUnitMessageViewController $details = $message->getUnitMessageDetails(); if (strlen($details)) { // TODO: Use the log view here, once it gets cleaned up. + // Shenanigans below. $details = phutil_tag( 'div', array( @@ -100,23 +106,24 @@ final class HarbormasterUnitMessageViewController PHUIPropertyListView::ICON_TESTPLAN); $view->addTextContent($details); - return $view; + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('PROPERTIES')) + ->appendChild($view); } - private function buildActionView( + private function buildCurtainView( HarbormasterBuildUnitMessage $message, HarbormasterBuild $build) { $viewer = $this->getViewer(); - $view = id(new PhabricatorActionListView()) - ->setUser($viewer); + $curtain = $this->newCurtainView($build); - $view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('View Build')) ->setHref($build->getURI()) ->setIcon('fa-wrench')); - return $view; + return $curtain; } } diff --git a/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php b/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php index 49efdc3e25..28f79ee181 100644 --- a/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php +++ b/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php @@ -7,7 +7,6 @@ final class HarbormasterUnitSummaryView extends AphrontView { private $limit; private $excuse; private $showViewAll; - private $background; public function setBuildable(HarbormasterBuildable $buildable) { $this->buildable = $buildable; @@ -34,11 +33,6 @@ final class HarbormasterUnitSummaryView extends AphrontView { return $this; } - public function setBackground($background) { - $this->background = $background; - return $this; - } - public function render() { $messages = $this->messages; $buildable = $this->buildable; @@ -79,7 +73,8 @@ final class HarbormasterUnitSummaryView extends AphrontView { } $box = id(new PHUIObjectBoxView()) - ->setHeader($header); + ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY); $table = id(new HarbormasterUnitPropertyView()) ->setUnitMessages($messages); @@ -109,10 +104,6 @@ final class HarbormasterUnitSummaryView extends AphrontView { $box->setTable($table); - if ($this->background) { - $box->setBackground($this->background); - } - return $box; } diff --git a/webroot/rsrc/css/phui/phui-timeline-view.css b/webroot/rsrc/css/phui/phui-timeline-view.css index ed4f77aa61..9778f1a58c 100644 --- a/webroot/rsrc/css/phui/phui-timeline-view.css +++ b/webroot/rsrc/css/phui/phui-timeline-view.css @@ -46,7 +46,7 @@ width: 9px; height: 9px; border-radius: 2px; - margin-left: 74px; + margin-left: 76px; } .device-desktop .phui-timeline-wedge { From 5576785f9fcc93a92e5830ea6a72d1103606ec7c Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 25 Mar 2016 15:41:05 -0700 Subject: [PATCH 29/31] Clean up spacing on empty logs in Harbormaster Summary: Better spacing in new layout. Test Plan: Tested changes against `secure` Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D15524 --- resources/celerity/map.php | 4 ++-- .../controller/HarbormasterBuildViewController.php | 2 +- webroot/rsrc/css/application/harbormaster/harbormaster.css | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index b2125badda..c710675681 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -70,7 +70,7 @@ return array( 'rsrc/css/application/feed/feed.css' => 'ecd4ec57', 'rsrc/css/application/files/global-drag-and-drop.css' => '5c1b47c2', 'rsrc/css/application/flag/flag.css' => '5337623f', - 'rsrc/css/application/harbormaster/harbormaster.css' => '834879db', + 'rsrc/css/application/harbormaster/harbormaster.css' => 'f491c9f4', 'rsrc/css/application/herald/herald-test.css' => 'a52e323e', 'rsrc/css/application/herald/herald.css' => 'dc31f6e9', 'rsrc/css/application/maniphest/batch-editor.css' => 'b0f0b6d5', @@ -561,7 +561,7 @@ return array( 'font-fontawesome' => 'c43323c5', 'font-lato' => 'c7ccd872', 'global-drag-and-drop-css' => '5c1b47c2', - 'harbormaster-css' => '834879db', + 'harbormaster-css' => 'f491c9f4', 'herald-css' => 'dc31f6e9', 'herald-rule-editor' => 'd6a7e717', 'herald-test-css' => 'a52e323e', diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php index 7e22d05471..320e94d642 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -375,7 +375,7 @@ final class HarbormasterBuildViewController 'div', array( 'id' => $hide_id, - 'class' => 'harbormaster-empty-logs-are-hidden mlr mlt mll', + 'class' => 'harbormaster-empty-logs-are-hidden', ), array( pht( diff --git a/webroot/rsrc/css/application/harbormaster/harbormaster.css b/webroot/rsrc/css/application/harbormaster/harbormaster.css index a80c8b7418..4bfd2d6cbe 100644 --- a/webroot/rsrc/css/application/harbormaster/harbormaster.css +++ b/webroot/rsrc/css/application/harbormaster/harbormaster.css @@ -18,6 +18,8 @@ border: 1px solid {$yellow}; text-align: center; padding: 12px; + margin: 0 0 20px 0; + border-radius: 3px; color: {$darkgreytext}; } From 39a4d5b8c1bb36cb29d9e48c83c7f90be4836edc Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 25 Mar 2016 15:45:53 -0700 Subject: [PATCH 30/31] Fix unit test view detail fatal Summary: Fixes T10674. Test Plan: {F1190743} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10674 Differential Revision: https://secure.phabricator.com/D15525 --- .../controller/HarbormasterUnitMessageViewController.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php b/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php index d344422dd5..d1bac8f874 100644 --- a/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php @@ -106,9 +106,7 @@ final class HarbormasterUnitMessageViewController PHUIPropertyListView::ICON_TESTPLAN); $view->addTextContent($details); - return id(new PHUIObjectBoxView()) - ->setHeaderText(pht('PROPERTIES')) - ->appendChild($view); + return $view; } private function buildCurtainView( From d784d9c0440c7a6d758fb7b5e71f622940889efc Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 25 Mar 2016 15:54:43 -0700 Subject: [PATCH 31/31] Set blue background (unless it looks terrible) Summary: See D15525. Test Plan: {F1190753} Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D15526 --- .../controller/HarbormasterUnitMessageViewController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php b/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php index d1bac8f874..631c332938 100644 --- a/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php @@ -41,7 +41,8 @@ final class HarbormasterUnitMessageViewController $curtain = $this->buildCurtainView($message, $build); $unit = id(new PHUIObjectBoxView()) - ->setHeader($header) + ->setHeaderText(pht('TEST RESULT')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); $crumbs = $this->buildApplicationCrumbs();