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 @@ -4056,6 +4056,8 @@ 'PhabricatorProjectColumnEditController' => 'applications/project/controller/PhabricatorProjectColumnEditController.php', 'PhabricatorProjectColumnHeader' => 'applications/project/order/PhabricatorProjectColumnHeader.php', 'PhabricatorProjectColumnHideController' => 'applications/project/controller/PhabricatorProjectColumnHideController.php', + 'PhabricatorProjectColumnLimitTransaction' => 'applications/project/xaction/column/PhabricatorProjectColumnLimitTransaction.php', + 'PhabricatorProjectColumnNameTransaction' => 'applications/project/xaction/column/PhabricatorProjectColumnNameTransaction.php', 'PhabricatorProjectColumnNaturalOrder' => 'applications/project/order/PhabricatorProjectColumnNaturalOrder.php', 'PhabricatorProjectColumnOrder' => 'applications/project/order/PhabricatorProjectColumnOrder.php', 'PhabricatorProjectColumnOwnerOrder' => 'applications/project/order/PhabricatorProjectColumnOwnerOrder.php', @@ -4067,10 +4069,12 @@ 'PhabricatorProjectColumnQuery' => 'applications/project/query/PhabricatorProjectColumnQuery.php', 'PhabricatorProjectColumnSearchEngine' => 'applications/project/query/PhabricatorProjectColumnSearchEngine.php', 'PhabricatorProjectColumnStatusOrder' => 'applications/project/order/PhabricatorProjectColumnStatusOrder.php', + 'PhabricatorProjectColumnStatusTransaction' => 'applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php', 'PhabricatorProjectColumnTitleOrder' => 'applications/project/order/PhabricatorProjectColumnTitleOrder.php', 'PhabricatorProjectColumnTransaction' => 'applications/project/storage/PhabricatorProjectColumnTransaction.php', 'PhabricatorProjectColumnTransactionEditor' => 'applications/project/editor/PhabricatorProjectColumnTransactionEditor.php', 'PhabricatorProjectColumnTransactionQuery' => 'applications/project/query/PhabricatorProjectColumnTransactionQuery.php', + 'PhabricatorProjectColumnTransactionType' => 'applications/project/xaction/column/PhabricatorProjectColumnTransactionType.php', 'PhabricatorProjectConfigOptions' => 'applications/project/config/PhabricatorProjectConfigOptions.php', 'PhabricatorProjectConfiguredCustomField' => 'applications/project/customfield/PhabricatorProjectConfiguredCustomField.php', 'PhabricatorProjectController' => 'applications/project/controller/PhabricatorProjectController.php', @@ -10159,6 +10163,8 @@ 'PhabricatorProjectColumnEditController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectColumnHeader' => 'Phobject', 'PhabricatorProjectColumnHideController' => 'PhabricatorProjectBoardController', + 'PhabricatorProjectColumnLimitTransaction' => 'PhabricatorProjectColumnTransactionType', + 'PhabricatorProjectColumnNameTransaction' => 'PhabricatorProjectColumnTransactionType', 'PhabricatorProjectColumnNaturalOrder' => 'PhabricatorProjectColumnOrder', 'PhabricatorProjectColumnOrder' => 'Phobject', 'PhabricatorProjectColumnOwnerOrder' => 'PhabricatorProjectColumnOrder', @@ -10173,10 +10179,12 @@ 'PhabricatorProjectColumnQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorProjectColumnSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorProjectColumnStatusOrder' => 'PhabricatorProjectColumnOrder', + 'PhabricatorProjectColumnStatusTransaction' => 'PhabricatorProjectColumnTransactionType', 'PhabricatorProjectColumnTitleOrder' => 'PhabricatorProjectColumnOrder', - 'PhabricatorProjectColumnTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorProjectColumnTransaction' => 'PhabricatorModularTransaction', 'PhabricatorProjectColumnTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorProjectColumnTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorProjectColumnTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorProjectConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorProjectConfiguredCustomField' => array( 'PhabricatorProjectStandardCustomField', diff --git a/src/applications/project/controller/PhabricatorProjectColumnEditController.php b/src/applications/project/controller/PhabricatorProjectColumnEditController.php --- a/src/applications/project/controller/PhabricatorProjectColumnEditController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnEditController.php @@ -76,8 +76,8 @@ $xactions = array(); - $type_name = PhabricatorProjectColumnTransaction::TYPE_NAME; - $type_limit = PhabricatorProjectColumnTransaction::TYPE_LIMIT; + $type_name = PhabricatorProjectColumnNameTransaction::TRANSACTIONTYPE; + $type_limit = PhabricatorProjectColumnLimitTransaction::TRANSACTIONTYPE; if (!$column->getProxy()) { $xactions[] = id(new PhabricatorProjectColumnTransaction()) @@ -93,7 +93,6 @@ $editor = id(new PhabricatorProjectColumnTransactionEditor()) ->setActor($viewer) ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true) ->setContentSourceFromRequest($request) ->applyTransactions($column, $xactions); return id(new AphrontRedirectResponse())->setURI($view_uri); diff --git a/src/applications/project/controller/PhabricatorProjectColumnHideController.php b/src/applications/project/controller/PhabricatorProjectColumnHideController.php --- a/src/applications/project/controller/PhabricatorProjectColumnHideController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnHideController.php @@ -82,7 +82,9 @@ $new_status = PhabricatorProjectColumn::STATUS_HIDDEN; } - $type_status = PhabricatorProjectColumnTransaction::TYPE_STATUS; + $type_status = + PhabricatorProjectColumnStatusTransaction::TRANSACTIONTYPE; + $xactions = array( id(new PhabricatorProjectColumnTransaction()) ->setTransactionType($type_status) diff --git a/src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php --- a/src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php @@ -11,130 +11,12 @@ return pht('Workboard Columns'); } - public function getTransactionTypes() { - $types = parent::getTransactionTypes(); - - $types[] = PhabricatorProjectColumnTransaction::TYPE_NAME; - $types[] = PhabricatorProjectColumnTransaction::TYPE_STATUS; - $types[] = PhabricatorProjectColumnTransaction::TYPE_LIMIT; - - return $types; - } - - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorProjectColumnTransaction::TYPE_NAME: - return $object->getName(); - case PhabricatorProjectColumnTransaction::TYPE_STATUS: - return $object->getStatus(); - case PhabricatorProjectColumnTransaction::TYPE_LIMIT: - return $object->getPointLimit(); - - } - - return parent::getCustomTransactionOldValue($object, $xaction); + public function getCreateObjectTitle($author, $object) { + return pht('%s created this column.', $author); } - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorProjectColumnTransaction::TYPE_NAME: - case PhabricatorProjectColumnTransaction::TYPE_STATUS: - return $xaction->getNewValue(); - case PhabricatorProjectColumnTransaction::TYPE_LIMIT: - $value = $xaction->getNewValue(); - if (strlen($value)) { - return (int)$xaction->getNewValue(); - } else { - return null; - } - } - - return parent::getCustomTransactionNewValue($object, $xaction); - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorProjectColumnTransaction::TYPE_NAME: - $object->setName($xaction->getNewValue()); - return; - case PhabricatorProjectColumnTransaction::TYPE_STATUS: - $object->setStatus($xaction->getNewValue()); - return; - case PhabricatorProjectColumnTransaction::TYPE_LIMIT: - $object->setPointLimit($xaction->getNewValue()); - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorProjectColumnTransaction::TYPE_NAME: - case PhabricatorProjectColumnTransaction::TYPE_STATUS: - case PhabricatorProjectColumnTransaction::TYPE_LIMIT: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - - switch ($type) { - case PhabricatorProjectColumnTransaction::TYPE_LIMIT: - foreach ($xactions as $xaction) { - $value = $xaction->getNewValue(); - if (strlen($value) && !preg_match('/^\d+\z/', $value)) { - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht( - 'Column point limit must either be empty or a nonnegative '. - 'integer.'), - $xaction); - } - } - break; - case PhabricatorProjectColumnTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getName(), - $xactions); - - // The default "Backlog" column is allowed to be unnamed, which - // means we use the default name. - - if ($missing && !$object->isDefaultColumn()) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('Column name is required.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - break; - } - - return $errors; + public function getCreateObjectTitleForFeed($author, $object) { + return pht('%s created %s.', $author, $object); } } diff --git a/src/applications/project/storage/PhabricatorProjectColumnTransaction.php b/src/applications/project/storage/PhabricatorProjectColumnTransaction.php --- a/src/applications/project/storage/PhabricatorProjectColumnTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectColumnTransaction.php @@ -1,11 +1,7 @@ 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 column.', - $author_handle); - } else { - if (!strlen($old)) { - return pht( - '%s named this column "%s".', - $author_handle, - $new); - } else if (strlen($new)) { - return pht( - '%s renamed this column from "%s" to "%s".', - $author_handle, - $old, - $new); - } else { - return pht( - '%s removed the custom name of this column.', - $author_handle); - } - } - case self::TYPE_LIMIT: - if (!$old) { - return pht( - '%s set the point limit for this column to %s.', - $author_handle, - $new); - } else if (!$new) { - return pht( - '%s removed the point limit for this column.', - $author_handle); - } else { - return pht( - '%s changed point limit for this column from %s to %s.', - $author_handle, - $old, - $new); - } - - case self::TYPE_STATUS: - switch ($new) { - case PhabricatorProjectColumn::STATUS_ACTIVE: - return pht( - '%s marked this column visible.', - $author_handle); - case PhabricatorProjectColumn::STATUS_HIDDEN: - return pht( - '%s marked this column hidden.', - $author_handle); - } - break; - } - - return parent::getTitle(); + public function getBaseTransactionClass() { + return 'PhabricatorProjectColumnTransactionType'; } } diff --git a/src/applications/project/xaction/column/PhabricatorProjectColumnLimitTransaction.php b/src/applications/project/xaction/column/PhabricatorProjectColumnLimitTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/project/xaction/column/PhabricatorProjectColumnLimitTransaction.php @@ -0,0 +1,63 @@ +getPointLimit(); + } + + public function generateNewValue($object, $value) { + if (strlen($value)) { + return (int)$value; + } else { + return null; + } + } + + public function applyInternalEffects($object, $value) { + $object->setPointLimit($value); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + if (!$old) { + return pht( + '%s set the point limit for this column to %s.', + $this->renderAuthor(), + $this->renderNewValue()); + } else if (!$new) { + return pht( + '%s removed the point limit for this column.', + $this->renderAuthor()); + } else { + return pht( + '%s changed the point limit for this column from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + foreach ($xactions as $xaction) { + $value = $xaction->getNewValue(); + if (strlen($value) && !preg_match('/^\d+\z/', $value)) { + $errors[] = $this->newInvalidError( + pht( + 'Column point limit must either be empty or a nonnegative '. + 'integer.'), + $xaction); + } + } + + return $errors; + } + +} diff --git a/src/applications/project/xaction/column/PhabricatorProjectColumnNameTransaction.php b/src/applications/project/xaction/column/PhabricatorProjectColumnNameTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/project/xaction/column/PhabricatorProjectColumnNameTransaction.php @@ -0,0 +1,66 @@ +getName(); + } + + public function applyInternalEffects($object, $value) { + $object->setName($value); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + if (!strlen($old)) { + return pht( + '%s named this column %s.', + $this->renderAuthor(), + $this->renderNewValue()); + } else if (strlen($new)) { + return pht( + '%s renamed this column from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } else { + return pht( + '%s removed the custom name of this column.', + $this->renderAuthor()); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + if ($this->isEmptyTextTransaction($object->getName(), $xactions)) { + // The default "Backlog" column is allowed to be unnamed, which + // means we use the default name. + if (!$object->isDefaultColumn()) { + $errors[] = $this->newRequiredError( + pht('Columns must have a name.')); + } + } + + $max_length = $object->getColumnMaximumByteLength('name'); + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + $new_length = strlen($new_value); + if ($new_length > $max_length) { + $errors[] = $this->newInvalidError( + pht( + 'Column names must not be longer than %s characters.', + new PhutilNumber($max_length)), + $xaction); + } + } + + return $errors; + } + +} diff --git a/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php b/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php @@ -0,0 +1,55 @@ +getStatus(); + } + + public function applyInternalEffects($object, $value) { + $object->setStatus($value); + } + + public function getTitle() { + $new = $this->getNewValue(); + + switch ($new) { + case PhabricatorProjectColumn::STATUS_ACTIVE: + return pht( + '%s unhid this column.', + $this->renderAuthor()); + case PhabricatorProjectColumn::STATUS_HIDDEN: + return pht( + '%s hid this column.', + $this->renderAuthor()); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $map = array( + PhabricatorProjectColumn::STATUS_ACTIVE, + PhabricatorProjectColumn::STATUS_HIDDEN, + ); + $map = array_fuse($map); + + foreach ($xactions as $xaction) { + $value = $xaction->getNewValue(); + if (!isset($map[$value])) { + $errors[] = $this->newInvalidError( + pht( + 'Column status "%s" is unrecognized, valid statuses are: %s.', + $value, + implode(', ', array_keys($map))), + $xaction); + } + } + + return $errors; + } + +} diff --git a/src/applications/project/xaction/column/PhabricatorProjectColumnTransactionType.php b/src/applications/project/xaction/column/PhabricatorProjectColumnTransactionType.php new file mode 100644 --- /dev/null +++ b/src/applications/project/xaction/column/PhabricatorProjectColumnTransactionType.php @@ -0,0 +1,4 @@ +