From a3d50118e117a15aada78ee2a8c9d7002c650cf8 Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Sat, 5 Jul 2014 01:56:02 +1000 Subject: [PATCH] Allow users to specify names of build steps Summary: Ref T1049. This provides a user-configurable name field on build steps, which allows users to uniquely identify their steps. The intention is that this field will be used in D9806 to better identify the dependencies (rather than showing an unhelpful PHID). Test Plan: Set the name of some build steps, saw it appear in the correct places. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Maniphest Tasks: T1049 Differential Revision: https://secure.phabricator.com/D9816 --- .../20140704.harbormasterstep.1.sql | 2 ++ .../20140704.harbormasterstep.2.sql | 2 ++ .../HarbormasterBuildViewController.php | 2 +- .../HarbormasterBuildableViewController.php | 7 +------ .../HarbormasterPlanViewController.php | 2 +- .../HarbormasterStepEditController.php | 19 ++++++++++++++++++- .../editor/HarbormasterBuildStepEditor.php | 11 +++++++++++ .../storage/build/HarbormasterBuildTarget.php | 14 ++++++++++++++ .../configuration/HarbormasterBuildStep.php | 9 +++++++++ .../HarbormasterBuildStepTransaction.php | 1 + 10 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 resources/sql/autopatches/20140704.harbormasterstep.1.sql create mode 100644 resources/sql/autopatches/20140704.harbormasterstep.2.sql diff --git a/resources/sql/autopatches/20140704.harbormasterstep.1.sql b/resources/sql/autopatches/20140704.harbormasterstep.1.sql new file mode 100644 index 0000000000..318765a024 --- /dev/null +++ b/resources/sql/autopatches/20140704.harbormasterstep.1.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildstep + ADD name VARCHAR(255) COLLATE utf8_bin; diff --git a/resources/sql/autopatches/20140704.harbormasterstep.2.sql b/resources/sql/autopatches/20140704.harbormasterstep.2.sql new file mode 100644 index 0000000000..517d29baac --- /dev/null +++ b/resources/sql/autopatches/20140704.harbormasterstep.2.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildtarget + ADD name VARCHAR(255) COLLATE utf8_bin; diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php index 6ae9639999..58419976d8 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -72,7 +72,7 @@ final class HarbormasterBuildViewController ->setHeader(pht( 'Build Target %d (%s)', $build_target->getID(), - $build_target->getImplementation()->getName())) + $build_target->getName())) ->setUser($viewer); $properties = new PHUIPropertyListView(); diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php index 8585b832c3..e8a41f13f6 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php @@ -282,12 +282,7 @@ final class HarbormasterBuildableViewController break; } - try { - $impl = $target->getImplementation(); - $name = $impl->getName(); - } catch (Exception $ex) { - $name = $target->getClassName(); - } + $name = $target->getName(); $target_list->addItem( id(new PHUIStatusItemView()) diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php index 8bfe298fdd..45dad6f344 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -129,7 +129,7 @@ final class HarbormasterPlanViewController } $item = id(new PHUIObjectItemView()) ->setObjectName('Step '.$i++) - ->setHeader($implementation->getName()); + ->setHeader($step->getName()); $item->addAttribute($implementation->getDescription()); diff --git a/src/applications/harbormaster/controller/HarbormasterStepEditController.php b/src/applications/harbormaster/controller/HarbormasterStepEditController.php index ca43de75e5..09da4df9c9 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php @@ -64,9 +64,15 @@ final class HarbormasterStepEditController ->setViewer($viewer) ->readFieldsFromStorage($step); + $e_name = true; + $v_name = $step->getName(); + $errors = array(); $validation_exception = null; if ($request->isFormPost()) { + $e_name = null; + $v_name = $request->getStr('name'); + $xactions = $field_list->buildFieldTransactionsFromRequest( new HarbormasterBuildStepTransaction(), $request); @@ -76,6 +82,11 @@ final class HarbormasterStepEditController ->setContinueOnNoEffect(true) ->setContentSourceFromRequest($request); + $name_xaction = id(new HarbormasterBuildStepTransaction()) + ->setTransactionType(HarbormasterBuildStepTransaction::TYPE_NAME) + ->setNewValue($v_name); + array_unshift($xactions, $name_xaction); + if ($is_new) { // This is okay, but a little iffy. We should move it inside the editor // if we create plans elsewhere. @@ -99,7 +110,13 @@ final class HarbormasterStepEditController } $form = id(new AphrontFormView()) - ->setUser($viewer); + ->setUser($viewer) + ->appendChild( + id(new AphrontFormTextControl()) + ->setName('name') + ->setLabel(pht('Name')) + ->setError($e_name) + ->setValue($v_name)); $field_list->appendFieldsToForm($form); diff --git a/src/applications/harbormaster/editor/HarbormasterBuildStepEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildStepEditor.php index 8c5ff83bbe..cc7124f5da 100644 --- a/src/applications/harbormaster/editor/HarbormasterBuildStepEditor.php +++ b/src/applications/harbormaster/editor/HarbormasterBuildStepEditor.php @@ -7,6 +7,7 @@ final class HarbormasterBuildStepEditor $types = parent::getTransactionTypes(); $types[] = HarbormasterBuildStepTransaction::TYPE_CREATE; + $types[] = HarbormasterBuildStepTransaction::TYPE_NAME; return $types; } @@ -18,6 +19,11 @@ final class HarbormasterBuildStepEditor switch ($xaction->getTransactionType()) { case HarbormasterBuildStepTransaction::TYPE_CREATE: return null; + case HarbormasterBuildStepTransaction::TYPE_NAME: + if ($this->getIsNewObject()) { + return null; + } + return $object->getName(); } return parent::getCustomTransactionOldValue($object, $xaction); @@ -30,6 +36,8 @@ final class HarbormasterBuildStepEditor switch ($xaction->getTransactionType()) { case HarbormasterBuildStepTransaction::TYPE_CREATE: return true; + case HarbormasterBuildStepTransaction::TYPE_NAME: + return $xaction->getNewValue(); } return parent::getCustomTransactionNewValue($object, $xaction); @@ -42,6 +50,8 @@ final class HarbormasterBuildStepEditor switch ($xaction->getTransactionType()) { case HarbormasterBuildStepTransaction::TYPE_CREATE: return; + case HarbormasterBuildStepTransaction::TYPE_NAME: + return $object->setName($xaction->getNewValue()); } return parent::applyCustomInternalTransaction($object, $xaction); @@ -53,6 +63,7 @@ final class HarbormasterBuildStepEditor switch ($xaction->getTransactionType()) { case HarbormasterBuildStepTransaction::TYPE_CREATE: + case HarbormasterBuildStepTransaction::TYPE_NAME: return; } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php index c545bcaf03..0c994701e1 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php @@ -3,6 +3,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO implements PhabricatorPolicyInterface { + protected $name; protected $buildPHID; protected $buildStepPHID; protected $className; @@ -25,6 +26,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO HarbormasterBuildStep $build_step, array $variables) { return id(new HarbormasterBuildTarget()) + ->setName($build_step->getName()) ->setBuildPHID($build->getPHID()) ->setBuildStepPHID($build_step->getPHID()) ->setClassName($build_step->getClassName()) @@ -99,6 +101,18 @@ final class HarbormasterBuildTarget extends HarbormasterDAO return $this->implementation; } + public function getName() { + if (strlen($this->name)) { + return $this->name; + } + + try { + return $this->getImplementation()->getName(); + } catch (Exception $e) { + return $this->getClassName(); + } + } + private function getBuildTargetVariables() { return array( 'target.phid' => $this->getPHID(), diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php index 37b9377c40..c412e09840 100644 --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php @@ -5,6 +5,7 @@ final class HarbormasterBuildStep extends HarbormasterDAO PhabricatorPolicyInterface, PhabricatorCustomFieldInterface { + protected $name; protected $buildPlanPHID; protected $className; protected $details = array(); @@ -50,6 +51,14 @@ final class HarbormasterBuildStep extends HarbormasterDAO return $this; } + public function getName() { + if (strlen($this->name)) { + return $this->name; + } + + return $this->getStepImplementation()->getName(); + } + public function getStepImplementation() { if ($this->implementation === null) { $obj = HarbormasterBuildStepImplementation::requireImplementation( diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStepTransaction.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStepTransaction.php index ca04a9818b..f1e75d1fb3 100644 --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStepTransaction.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStepTransaction.php @@ -4,6 +4,7 @@ final class HarbormasterBuildStepTransaction extends PhabricatorApplicationTransaction { const TYPE_CREATE = 'harbormaster:step:create'; + const TYPE_NAME = 'harbormaster:step:name'; public function getApplicationName() { return 'harbormaster';