From 9d56a3d86ec7336c619779b63000a6e9f8eedf25 Mon Sep 17 00:00:00 2001 From: Austin McKinley Date: Wed, 12 Apr 2017 18:19:31 -0700 Subject: [PATCH] Reimplement Countdown transactions using Modular Transaction framework Test Plan: owls Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D17671 --- src/__phutil_library_map__.php | 10 +- .../editor/PhabricatorCountdownEditEngine.php | 9 +- .../editor/PhabricatorCountdownEditor.php | 124 ------------------ .../PhabricatorCountdownTransaction.php | 87 +----------- ...ricatorCountdownDescriptionTransaction.php | 55 ++++++++ .../PhabricatorCountdownEpochTransaction.php | 58 ++++++++ .../PhabricatorCountdownTitleTransaction.php | 54 ++++++++ .../PhabricatorCountdownTransactionType.php | 4 + 8 files changed, 192 insertions(+), 209 deletions(-) create mode 100644 src/applications/countdown/xaction/PhabricatorCountdownDescriptionTransaction.php create mode 100644 src/applications/countdown/xaction/PhabricatorCountdownEpochTransaction.php create mode 100644 src/applications/countdown/xaction/PhabricatorCountdownTitleTransaction.php create mode 100644 src/applications/countdown/xaction/PhabricatorCountdownTransactionType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 237a4ee392..4a92977988 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2413,9 +2413,11 @@ phutil_register_library_map(array( 'PhabricatorCountdownDAO' => 'applications/countdown/storage/PhabricatorCountdownDAO.php', 'PhabricatorCountdownDefaultEditCapability' => 'applications/countdown/capability/PhabricatorCountdownDefaultEditCapability.php', 'PhabricatorCountdownDefaultViewCapability' => 'applications/countdown/capability/PhabricatorCountdownDefaultViewCapability.php', + 'PhabricatorCountdownDescriptionTransaction' => 'applications/countdown/xaction/PhabricatorCountdownDescriptionTransaction.php', 'PhabricatorCountdownEditController' => 'applications/countdown/controller/PhabricatorCountdownEditController.php', 'PhabricatorCountdownEditEngine' => 'applications/countdown/editor/PhabricatorCountdownEditEngine.php', 'PhabricatorCountdownEditor' => 'applications/countdown/editor/PhabricatorCountdownEditor.php', + 'PhabricatorCountdownEpochTransaction' => 'applications/countdown/xaction/PhabricatorCountdownEpochTransaction.php', 'PhabricatorCountdownListController' => 'applications/countdown/controller/PhabricatorCountdownListController.php', 'PhabricatorCountdownMailReceiver' => 'applications/countdown/mail/PhabricatorCountdownMailReceiver.php', 'PhabricatorCountdownQuery' => 'applications/countdown/query/PhabricatorCountdownQuery.php', @@ -2423,9 +2425,11 @@ phutil_register_library_map(array( 'PhabricatorCountdownReplyHandler' => 'applications/countdown/mail/PhabricatorCountdownReplyHandler.php', 'PhabricatorCountdownSchemaSpec' => 'applications/countdown/storage/PhabricatorCountdownSchemaSpec.php', 'PhabricatorCountdownSearchEngine' => 'applications/countdown/query/PhabricatorCountdownSearchEngine.php', + 'PhabricatorCountdownTitleTransaction' => 'applications/countdown/xaction/PhabricatorCountdownTitleTransaction.php', 'PhabricatorCountdownTransaction' => 'applications/countdown/storage/PhabricatorCountdownTransaction.php', 'PhabricatorCountdownTransactionComment' => 'applications/countdown/storage/PhabricatorCountdownTransactionComment.php', 'PhabricatorCountdownTransactionQuery' => 'applications/countdown/query/PhabricatorCountdownTransactionQuery.php', + 'PhabricatorCountdownTransactionType' => 'applications/countdown/xaction/PhabricatorCountdownTransactionType.php', 'PhabricatorCountdownView' => 'applications/countdown/view/PhabricatorCountdownView.php', 'PhabricatorCountdownViewController' => 'applications/countdown/controller/PhabricatorCountdownViewController.php', 'PhabricatorCursorPagedPolicyAwareQuery' => 'infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php', @@ -7516,9 +7520,11 @@ phutil_register_library_map(array( 'PhabricatorCountdownDAO' => 'PhabricatorLiskDAO', 'PhabricatorCountdownDefaultEditCapability' => 'PhabricatorPolicyCapability', 'PhabricatorCountdownDefaultViewCapability' => 'PhabricatorPolicyCapability', + 'PhabricatorCountdownDescriptionTransaction' => 'PhabricatorCountdownTransactionType', 'PhabricatorCountdownEditController' => 'PhabricatorCountdownController', 'PhabricatorCountdownEditEngine' => 'PhabricatorEditEngine', 'PhabricatorCountdownEditor' => 'PhabricatorApplicationTransactionEditor', + 'PhabricatorCountdownEpochTransaction' => 'PhabricatorCountdownTransactionType', 'PhabricatorCountdownListController' => 'PhabricatorCountdownController', 'PhabricatorCountdownMailReceiver' => 'PhabricatorObjectMailReceiver', 'PhabricatorCountdownQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', @@ -7526,9 +7532,11 @@ phutil_register_library_map(array( 'PhabricatorCountdownReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', 'PhabricatorCountdownSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorCountdownSearchEngine' => 'PhabricatorApplicationSearchEngine', - 'PhabricatorCountdownTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorCountdownTitleTransaction' => 'PhabricatorCountdownTransactionType', + 'PhabricatorCountdownTransaction' => 'PhabricatorModularTransaction', 'PhabricatorCountdownTransactionComment' => 'PhabricatorApplicationTransactionComment', 'PhabricatorCountdownTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorCountdownTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorCountdownView' => 'AphrontView', 'PhabricatorCountdownViewController' => 'PhabricatorCountdownController', 'PhabricatorCursorPagedPolicyAwareQuery' => 'PhabricatorPolicyAwareQuery', diff --git a/src/applications/countdown/editor/PhabricatorCountdownEditEngine.php b/src/applications/countdown/editor/PhabricatorCountdownEditEngine.php index c1d5f6753a..844cd6d9df 100644 --- a/src/applications/countdown/editor/PhabricatorCountdownEditEngine.php +++ b/src/applications/countdown/editor/PhabricatorCountdownEditEngine.php @@ -81,7 +81,8 @@ final class PhabricatorCountdownEditEngine ->setKey('name') ->setLabel(pht('Name')) ->setIsRequired(true) - ->setTransactionType(PhabricatorCountdownTransaction::TYPE_TITLE) + ->setTransactionType( + PhabricatorCountdownTitleTransaction::TRANSACTIONTYPE) ->setDescription(pht('The countdown name.')) ->setConduitDescription(pht('Rename the countdown.')) ->setConduitTypeDescription(pht('New countdown name.')) @@ -89,7 +90,8 @@ final class PhabricatorCountdownEditEngine id(new PhabricatorEpochEditField()) ->setKey('epoch') ->setLabel(pht('End Date')) - ->setTransactionType(PhabricatorCountdownTransaction::TYPE_EPOCH) + ->setTransactionType( + PhabricatorCountdownEpochTransaction::TRANSACTIONTYPE) ->setDescription(pht('Date when the countdown ends.')) ->setConduitDescription(pht('Change the end date of the countdown.')) ->setConduitTypeDescription(pht('New countdown end date.')) @@ -97,7 +99,8 @@ final class PhabricatorCountdownEditEngine id(new PhabricatorRemarkupEditField()) ->setKey('description') ->setLabel(pht('Description')) - ->setTransactionType(PhabricatorCountdownTransaction::TYPE_DESCRIPTION) + ->setTransactionType( + PhabricatorCountdownDescriptionTransaction::TRANSACTIONTYPE) ->setDescription(pht('Description of the countdown.')) ->setConduitDescription(pht('Change the countdown description.')) ->setConduitTypeDescription(pht('New description.')) diff --git a/src/applications/countdown/editor/PhabricatorCountdownEditor.php b/src/applications/countdown/editor/PhabricatorCountdownEditor.php index 37f23f6bd6..322b2ee2c3 100644 --- a/src/applications/countdown/editor/PhabricatorCountdownEditor.php +++ b/src/applications/countdown/editor/PhabricatorCountdownEditor.php @@ -14,10 +14,6 @@ final class PhabricatorCountdownEditor public function getTransactionTypes() { $types = parent::getTransactionTypes(); - $types[] = PhabricatorCountdownTransaction::TYPE_TITLE; - $types[] = PhabricatorCountdownTransaction::TYPE_EPOCH; - $types[] = PhabricatorCountdownTransaction::TYPE_DESCRIPTION; - $types[] = PhabricatorTransactions::TYPE_EDGE; $types[] = PhabricatorTransactions::TYPE_SPACE; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; @@ -27,126 +23,6 @@ final class PhabricatorCountdownEditor return $types; } - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { - case PhabricatorCountdownTransaction::TYPE_TITLE: - return $object->getTitle(); - case PhabricatorCountdownTransaction::TYPE_DESCRIPTION: - return $object->getDescription(); - case PhabricatorCountdownTransaction::TYPE_EPOCH: - return $object->getEpoch(); - } - - return parent::getCustomTransactionOldValue($object, $xaction); - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorCountdownTransaction::TYPE_TITLE: - return $xaction->getNewValue(); - case PhabricatorCountdownTransaction::TYPE_DESCRIPTION: - return $xaction->getNewValue(); - case PhabricatorCountdownTransaction::TYPE_EPOCH: - return $xaction->getNewValue()->getEpoch(); - } - - return parent::getCustomTransactionNewValue($object, $xaction); - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - $type = $xaction->getTransactionType(); - switch ($type) { - case PhabricatorCountdownTransaction::TYPE_TITLE: - $object->setTitle($xaction->getNewValue()); - return; - case PhabricatorCountdownTransaction::TYPE_DESCRIPTION: - $object->setDescription($xaction->getNewValue()); - return; - case PhabricatorCountdownTransaction::TYPE_EPOCH: - $object->setEpoch($xaction->getNewValue()); - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - $type = $xaction->getTransactionType(); - switch ($type) { - case PhabricatorCountdownTransaction::TYPE_TITLE: - return; - case PhabricatorCountdownTransaction::TYPE_DESCRIPTION: - return; - case PhabricatorCountdownTransaction::TYPE_EPOCH: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - - switch ($type) { - case PhabricatorCountdownTransaction::TYPE_TITLE: - $missing = $this->validateIsEmptyTextField( - $object->getTitle(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('You must give the countdown a name.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - break; - case PhabricatorCountdownTransaction::TYPE_EPOCH: - if (!$object->getEpoch() && !$xactions) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('You must give the countdown an end date.'), - null); - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - - foreach ($xactions as $xaction) { - $value = $xaction->getNewValue(); - if (!$value->isValid()) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht('You must give the countdown a valid end date.'), - $xaction); - $errors[] = $error; - } - } - break; - } - - return $errors; - } - protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { diff --git a/src/applications/countdown/storage/PhabricatorCountdownTransaction.php b/src/applications/countdown/storage/PhabricatorCountdownTransaction.php index 247466ffe1..78e0a54ced 100644 --- a/src/applications/countdown/storage/PhabricatorCountdownTransaction.php +++ b/src/applications/countdown/storage/PhabricatorCountdownTransaction.php @@ -1,11 +1,7 @@ getAuthorPHID(); - $object_phid = $this->getObjectPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $type = $this->getTransactionType(); - switch ($type) { - case self::TYPE_TITLE: - return pht( - '%s renamed this countdown from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $old, - $new); - case self::TYPE_DESCRIPTION: - return pht( - '%s edited the description of this countdown.', - $this->renderHandleLink($author_phid)); - case self::TYPE_EPOCH: - return pht( - '%s updated this countdown to end on %s.', - $this->renderHandleLink($author_phid), - phabricator_datetime($new, $this->getViewer())); - } - - return parent::getTitle(); - } - - public function getTitleForFeed() { - $author_phid = $this->getAuthorPHID(); - $object_phid = $this->getObjectPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $type = $this->getTransactionType(); - switch ($type) { - case self::TYPE_TITLE: - return pht( - '%s renamed %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - case self::TYPE_DESCRIPTION: - return pht( - '%s edited the description of %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - case self::TYPE_EPOCH: - return pht( - '%s edited the end date of %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - } - - return parent::getTitleForFeed(); + public function getBaseTransactionClass() { + return 'PhabricatorCountdownTransactionType'; } public function getMailTags() { @@ -88,9 +30,9 @@ final class PhabricatorCountdownTransaction case PhabricatorTransactions::TYPE_COMMENT: $tags[] = self::MAILTAG_COMMENT; break; - case self::TYPE_TITLE: - case self::TYPE_EPOCH: - case self::TYPE_DESCRIPTION: + case PhabricatorCountdownTitleTransaction::TRANSACTIONTYPE: + case PhabricatorCountdownEpochTransaction::TRANSACTIONTYPE: + case PhabricatorCountdownDescriptionTransaction::TRANSACTIONTYPE: $tags[] = self::MAILTAG_DETAILS; break; default: @@ -100,21 +42,4 @@ final class PhabricatorCountdownTransaction return $tags; } - - public function hasChangeDetails() { - switch ($this->getTransactionType()) { - case self::TYPE_DESCRIPTION: - return ($this->getOldValue() !== null); - } - - return parent::hasChangeDetails(); - } - - public function renderChangeDetails(PhabricatorUser $viewer) { - return $this->renderTextCorpusChangeDetails( - $viewer, - $this->getOldValue(), - $this->getNewValue()); - } - } diff --git a/src/applications/countdown/xaction/PhabricatorCountdownDescriptionTransaction.php b/src/applications/countdown/xaction/PhabricatorCountdownDescriptionTransaction.php new file mode 100644 index 0000000000..a742ac2125 --- /dev/null +++ b/src/applications/countdown/xaction/PhabricatorCountdownDescriptionTransaction.php @@ -0,0 +1,55 @@ +getDescription(); + } + + public function applyInternalEffects($object, $value) { + $object->setDescription($value); + } + + public function getTitle() { + return pht( + '%s updated the countdown description.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s updated the countdown description for %s.', + $this->renderAuthor(), + $this->renderObject()); + } + + public function hasChangeDetailView() { + return true; + } + + public function getMailDiffSectionHeader() { + return pht('CHANGES TO COUNTDOWN DESCRIPTION'); + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + + return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setViewer($viewer) + ->setOldText($this->getOldValue()) + ->setNewText($this->getNewValue()); + } + + public function newRemarkupChanges() { + $changes = array(); + + $changes[] = $this->newRemarkupChange() + ->setOldValue($this->getOldValue()) + ->setNewValue($this->getNewValue()); + + return $changes; + } +} diff --git a/src/applications/countdown/xaction/PhabricatorCountdownEpochTransaction.php b/src/applications/countdown/xaction/PhabricatorCountdownEpochTransaction.php new file mode 100644 index 0000000000..905e0771d8 --- /dev/null +++ b/src/applications/countdown/xaction/PhabricatorCountdownEpochTransaction.php @@ -0,0 +1,58 @@ +getEpoch(); + } + + public function generateNewValue($object, $value) { + return $value->newPhutilDateTime() + ->newAbsoluteDateTime() + ->getEpoch(); + } + + public function applyInternalEffects($object, $value) { + $object->setEpoch($value); + } + + public function getTitle() { + return pht( + '%s updated the countdown end from %s to %s.', + $this->renderAuthor(), + $this->renderOldDate(), + $this->renderNewDate()); + } + + public function getTitleForFeed() { + return pht( + '%s updated the countdown end for %s from %s to %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderOldDate(), + $this->renderNewDate()); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + if (!$object->getEpoch() && !$xactions) { + $errors[] = $this->newRequiredError( + pht('You must give the countdown an end date.')); + return $errors; + } + + foreach ($xactions as $xaction) { + $value = $xaction->getNewValue(); + if (!$value->isValid()) { + $errors[] = $this->newInvalidError( + pht('You must give the countdown an end date.')); + } + } + + return $errors; + } +} diff --git a/src/applications/countdown/xaction/PhabricatorCountdownTitleTransaction.php b/src/applications/countdown/xaction/PhabricatorCountdownTitleTransaction.php new file mode 100644 index 0000000000..dca96c10c1 --- /dev/null +++ b/src/applications/countdown/xaction/PhabricatorCountdownTitleTransaction.php @@ -0,0 +1,54 @@ +getTitle(); + } + + public function applyInternalEffects($object, $value) { + $object->setTitle($value); + } + + public function getTitle() { + return pht( + '%s updated the title for this countdown from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function getTitleForFeed() { + return pht( + '%s updated the title for this countdown from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + if ($this->isEmptyTextTransaction($object->getTitle(), $xactions)) { + $errors[] = $this->newRequiredError(pht('Countdowns must have a title.')); + } + + $max_length = $object->getColumnMaximumByteLength('title'); + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + $new_length = strlen($new_value); + if ($new_length > $max_length) { + $errors[] = $this->newInvalidError( + pht( + 'Countdown titles must not be longer than %s character(s).', + new PhutilNumber($max_length))); + } + } + + return $errors; + } + +} diff --git a/src/applications/countdown/xaction/PhabricatorCountdownTransactionType.php b/src/applications/countdown/xaction/PhabricatorCountdownTransactionType.php new file mode 100644 index 0000000000..be0a8c6f78 --- /dev/null +++ b/src/applications/countdown/xaction/PhabricatorCountdownTransactionType.php @@ -0,0 +1,4 @@ +