diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1334,12 +1334,15 @@ 'HarbormasterBuildPlanEditEngine' => 'applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php', 'HarbormasterBuildPlanEditor' => 'applications/harbormaster/editor/HarbormasterBuildPlanEditor.php', 'HarbormasterBuildPlanNameNgrams' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlanNameNgrams.php', + 'HarbormasterBuildPlanNameTransaction' => 'applications/harbormaster/xaction/plan/HarbormasterBuildPlanNameTransaction.php', 'HarbormasterBuildPlanPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPlanPHIDType.php', 'HarbormasterBuildPlanQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanQuery.php', 'HarbormasterBuildPlanSearchAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildPlanSearchAPIMethod.php', 'HarbormasterBuildPlanSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildPlanSearchEngine.php', + 'HarbormasterBuildPlanStatusTransaction' => 'applications/harbormaster/xaction/plan/HarbormasterBuildPlanStatusTransaction.php', 'HarbormasterBuildPlanTransaction' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php', 'HarbormasterBuildPlanTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanTransactionQuery.php', + 'HarbormasterBuildPlanTransactionType' => 'applications/harbormaster/xaction/plan/HarbormasterBuildPlanTransactionType.php', 'HarbormasterBuildQuery' => 'applications/harbormaster/query/HarbormasterBuildQuery.php', 'HarbormasterBuildRequest' => 'applications/harbormaster/engine/HarbormasterBuildRequest.php', 'HarbormasterBuildSearchConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildSearchConduitAPIMethod.php', @@ -6943,12 +6946,15 @@ 'HarbormasterBuildPlanEditEngine' => 'PhabricatorEditEngine', 'HarbormasterBuildPlanEditor' => 'PhabricatorApplicationTransactionEditor', 'HarbormasterBuildPlanNameNgrams' => 'PhabricatorSearchNgrams', + 'HarbormasterBuildPlanNameTransaction' => 'HarbormasterBuildPlanTransactionType', 'HarbormasterBuildPlanPHIDType' => 'PhabricatorPHIDType', 'HarbormasterBuildPlanQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildPlanSearchAPIMethod' => 'PhabricatorSearchEngineAPIMethod', 'HarbormasterBuildPlanSearchEngine' => 'PhabricatorApplicationSearchEngine', - 'HarbormasterBuildPlanTransaction' => 'PhabricatorApplicationTransaction', + 'HarbormasterBuildPlanStatusTransaction' => 'HarbormasterBuildPlanTransactionType', + 'HarbormasterBuildPlanTransaction' => 'PhabricatorModularTransaction', 'HarbormasterBuildPlanTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'HarbormasterBuildPlanTransactionType' => 'PhabricatorModularTransactionType', 'HarbormasterBuildQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildRequest' => 'Phobject', 'HarbormasterBuildSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', diff --git a/src/applications/harbormaster/controller/HarbormasterPlanDisableController.php b/src/applications/harbormaster/controller/HarbormasterPlanDisableController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanDisableController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanDisableController.php @@ -19,11 +19,11 @@ return new Aphront404Response(); } - $plan_uri = $this->getApplicationURI('plan/'.$plan->getID().'/'); + $plan_uri = $plan->getURI(); if ($request->isFormPost()) { - $type_status = HarbormasterBuildPlanTransaction::TYPE_STATUS; + $type_status = HarbormasterBuildPlanStatusTransaction::TRANSACTIONTYPE; $v_status = $plan->isDisabled() ? HarbormasterBuildPlan::STATUS_ACTIVE diff --git a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php --- a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php +++ b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php @@ -82,7 +82,8 @@ ->setKey('name') ->setLabel(pht('Name')) ->setIsRequired(true) - ->setTransactionType(HarbormasterBuildPlanTransaction::TYPE_NAME) + ->setTransactionType( + HarbormasterBuildPlanNameTransaction::TRANSACTIONTYPE) ->setDescription(pht('The build plan name.')) ->setConduitDescription(pht('Rename the plan.')) ->setConduitTypeDescription(pht('New plan name.')) diff --git a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php --- a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php +++ b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php @@ -11,100 +11,23 @@ return pht('Harbormaster Build Plans'); } + public function getCreateObjectTitle($author, $object) { + return pht('%s created this build plan.', $author); + } + + public function getCreateObjectTitleForFeed($author, $object) { + return pht('%s created %s.', $author, $object); + } + protected function supportsSearch() { return true; } public function getTransactionTypes() { $types = parent::getTransactionTypes(); - $types[] = HarbormasterBuildPlanTransaction::TYPE_NAME; - $types[] = HarbormasterBuildPlanTransaction::TYPE_STATUS; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; return $types; } - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { - case HarbormasterBuildPlanTransaction::TYPE_NAME: - if ($this->getIsNewObject()) { - return null; - } - return $object->getName(); - case HarbormasterBuildPlanTransaction::TYPE_STATUS: - return $object->getPlanStatus(); - } - - return parent::getCustomTransactionOldValue($object, $xaction); - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { - case HarbormasterBuildPlanTransaction::TYPE_NAME: - return $xaction->getNewValue(); - case HarbormasterBuildPlanTransaction::TYPE_STATUS: - return $xaction->getNewValue(); - } - return parent::getCustomTransactionNewValue($object, $xaction); - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { - case HarbormasterBuildPlanTransaction::TYPE_NAME: - $object->setName($xaction->getNewValue()); - return; - case HarbormasterBuildPlanTransaction::TYPE_STATUS: - $object->setPlanStatus($xaction->getNewValue()); - return; - } - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { - case HarbormasterBuildPlanTransaction::TYPE_NAME: - case HarbormasterBuildPlanTransaction::TYPE_STATUS: - return; - } - return parent::applyCustomExternalTransaction($object, $xaction); - } - - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - - switch ($type) { - case HarbormasterBuildPlanTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getName(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('You must choose a name for your build plan.'), - last($xactions)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - break; - } - - return $errors; - } - - } diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php @@ -84,6 +84,16 @@ return ($this->getPlanStatus() == self::STATUS_DISABLED); } + public function getURI() { + return urisprintf( + '/harbormaster/plan/%s/', + $this->getID()); + } + + public function getObjectName() { + return pht('Build Plan %d', $this->getID()); + } + /* -( Autoplans )---------------------------------------------------------- */ diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php @@ -1,10 +1,7 @@ getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_NAME: - if ($old === null) { - return 'fa-plus'; - } - break; - } - - return parent::getIcon(); - } - - public function getColor() { - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_NAME: - if ($old === null) { - return 'green'; - } - break; - } - - return parent::getIcon(); - } - - public function getTitle() { - $old = $this->getOldValue(); - $new = $this->getNewValue(); - $author_handle = $this->renderHandleLink($this->getAuthorPHID()); - - switch ($this->getTransactionType()) { - case self::TYPE_NAME: - if ($old === null) { - return pht( - '%s created this build plan.', - $author_handle); - } else { - return pht( - '%s renamed this build plan from "%s" to "%s".', - $author_handle, - $old, - $new); - } - case self::TYPE_STATUS: - if ($new == HarbormasterBuildPlan::STATUS_DISABLED) { - return pht( - '%s disabled this build plan.', - $author_handle); - } else { - return pht( - '%s enabled this build plan.', - $author_handle); - } - } - - return parent::getTitle(); + public function getBaseTransactionClass() { + return 'HarbormasterBuildPlanTransactionType'; } } diff --git a/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanNameTransaction.php b/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanNameTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanNameTransaction.php @@ -0,0 +1,46 @@ +getName(); + } + + public function applyInternalEffects($object, $value) { + $object->setName($value); + } + + public function getTitle() { + return pht( + '%s renamed this build plan from "%s" to "%s".', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + if ($this->isEmptyTextTransaction($object->getName(), $xactions)) { + $errors[] = $this->newRequiredError( + pht('You must choose a name for your build plan.')); + } + + return $errors; + } + + public function getTransactionTypeForConduit($xaction) { + return 'name'; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array( + 'old' => $xaction->getOldValue(), + 'new' => $xaction->getNewValue(), + ); + } + +} diff --git a/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanStatusTransaction.php b/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanStatusTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanStatusTransaction.php @@ -0,0 +1,67 @@ +getPlanStatus(); + } + + public function applyInternalEffects($object, $value) { + $object->setPlanStatus($value); + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new === HarbormasterBuildPlan::STATUS_DISABLED) { + return pht( + '%s disabled this build plan.', + $this->renderAuthor()); + } else { + return pht( + '%s enabled this build plan.', + $this->renderAuthor()); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $options = array( + HarbormasterBuildPlan::STATUS_DISABLED, + HarbormasterBuildPlan::STATUS_ACTIVE, + ); + $options = array_fuse($options); + + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + if (!isset($options[$new])) { + $errors[] = $this->newInvalidError( + pht( + 'Status "%s" is not a valid build plan status. Valid '. + 'statuses are: %s.', + $new, + implode(', ', $options))); + continue; + } + + } + + return $errors; + } + + public function getTransactionTypeForConduit($xaction) { + return 'status'; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array( + 'old' => $xaction->getOldValue(), + 'new' => $xaction->getNewValue(), + ); + } + +} diff --git a/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanTransactionType.php b/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanTransactionType.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/xaction/plan/HarbormasterBuildPlanTransactionType.php @@ -0,0 +1,4 @@ +