From 74d86f09023ca23126160c3fe8b5115bd9108c1c Mon Sep 17 00:00:00 2001 From: Austin McKinley Date: Fri, 12 May 2017 16:00:55 -0700 Subject: [PATCH] Migrate Pholio image description and mock status to modular transactions Summary: Also removes more now-dead code from `PholioTransaction`. Test Plan: opened and closed a bunch of mocks, edited a bunch of image descriptions Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D17868 --- src/__phutil_library_map__.php | 4 + .../PholioMockArchiveController.php | 2 +- .../controller/PholioMockEditController.php | 2 +- .../pholio/editor/PholioMockEditor.php | 35 --------- .../pholio/storage/PholioTransaction.php | 73 +----------------- .../PholioImageDescriptionTransaction.php | 76 +++++++++++++++++++ .../xaction/PholioImageNameTransaction.php | 10 --- .../xaction/PholioMockStatusTransaction.php | 66 ++++++++++++++++ 8 files changed, 151 insertions(+), 117 deletions(-) create mode 100644 src/applications/pholio/xaction/PholioImageDescriptionTransaction.php create mode 100644 src/applications/pholio/xaction/PholioMockStatusTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7a7ca114d1..611b2c118c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4360,6 +4360,7 @@ phutil_register_library_map(array( 'PholioDefaultEditCapability' => 'applications/pholio/capability/PholioDefaultEditCapability.php', 'PholioDefaultViewCapability' => 'applications/pholio/capability/PholioDefaultViewCapability.php', 'PholioImage' => 'applications/pholio/storage/PholioImage.php', + 'PholioImageDescriptionTransaction' => 'applications/pholio/xaction/PholioImageDescriptionTransaction.php', 'PholioImageNameTransaction' => 'applications/pholio/xaction/PholioImageNameTransaction.php', 'PholioImagePHIDType' => 'applications/pholio/phid/PholioImagePHIDType.php', 'PholioImageQuery' => 'applications/pholio/query/PholioImageQuery.php', @@ -4391,6 +4392,7 @@ phutil_register_library_map(array( 'PholioMockRelationship' => 'applications/pholio/relationships/PholioMockRelationship.php', 'PholioMockRelationshipSource' => 'applications/search/relationship/PholioMockRelationshipSource.php', 'PholioMockSearchEngine' => 'applications/pholio/query/PholioMockSearchEngine.php', + 'PholioMockStatusTransaction' => 'applications/pholio/xaction/PholioMockStatusTransaction.php', 'PholioMockThumbGridView' => 'applications/pholio/view/PholioMockThumbGridView.php', 'PholioMockTransactionType' => 'applications/pholio/xaction/PholioMockTransactionType.php', 'PholioMockViewController' => 'applications/pholio/controller/PholioMockViewController.php', @@ -9905,6 +9907,7 @@ phutil_register_library_map(array( 'PhabricatorMarkupInterface', 'PhabricatorPolicyInterface', ), + 'PholioImageDescriptionTransaction' => 'PholioImageTransactionType', 'PholioImageNameTransaction' => 'PholioImageTransactionType', 'PholioImagePHIDType' => 'PhabricatorPHIDType', 'PholioImageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', @@ -9949,6 +9952,7 @@ phutil_register_library_map(array( 'PholioMockRelationship' => 'PhabricatorObjectRelationship', 'PholioMockRelationshipSource' => 'PhabricatorObjectRelationshipSource', 'PholioMockSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PholioMockStatusTransaction' => 'PholioMockTransactionType', 'PholioMockThumbGridView' => 'AphrontView', 'PholioMockTransactionType' => 'PholioTransactionType', 'PholioMockViewController' => 'PholioController', diff --git a/src/applications/pholio/controller/PholioMockArchiveController.php b/src/applications/pholio/controller/PholioMockArchiveController.php index be8066ea7b..feca94610b 100644 --- a/src/applications/pholio/controller/PholioMockArchiveController.php +++ b/src/applications/pholio/controller/PholioMockArchiveController.php @@ -32,7 +32,7 @@ final class PholioMockArchiveController $xactions = array(); $xactions[] = id(new PholioTransaction()) - ->setTransactionType(PholioTransaction::TYPE_STATUS) + ->setTransactionType(PholioMockStatusTransaction::TRANSACTIONTYPE) ->setNewValue($new_status); id(new PholioMockEditor()) diff --git a/src/applications/pholio/controller/PholioMockEditController.php b/src/applications/pholio/controller/PholioMockEditController.php index 42038cc2d3..e97af5da27 100644 --- a/src/applications/pholio/controller/PholioMockEditController.php +++ b/src/applications/pholio/controller/PholioMockEditController.php @@ -173,7 +173,7 @@ final class PholioMockEditController extends PholioController { array($existing_image->getPHID() => $title)); $xactions[] = id(new PholioTransaction()) ->setTransactionType( - PholioTransaction::TYPE_IMAGE_DESCRIPTION) + PholioImageDescriptionTransaction::TRANSACTIONTYPE) ->setNewValue( array($existing_image->getPHID() => $description)); $xactions[] = id(new PholioTransaction()) diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php index 7910830b92..6f5bf1df2b 100644 --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -29,11 +29,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; - $types[] = PholioTransaction::TYPE_STATUS; $types[] = PholioTransaction::TYPE_INLINE; $types[] = PholioTransaction::TYPE_IMAGE_FILE; - $types[] = PholioTransaction::TYPE_IMAGE_DESCRIPTION; $types[] = PholioTransaction::TYPE_IMAGE_REPLACE; $types[] = PholioTransaction::TYPE_IMAGE_SEQUENCE; @@ -45,20 +43,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_STATUS: - return $object->getStatus(); case PholioTransaction::TYPE_IMAGE_FILE: $images = $object->getImages(); return mpull($images, 'getPHID'); - case PholioTransaction::TYPE_IMAGE_DESCRIPTION: - $description = null; - $phid = null; - $image = $this->getImageForXaction($object, $xaction); - if ($image) { - $description = $image->getDescription(); - $phid = $image->getPHID(); - } - return array($phid => $description); case PholioTransaction::TYPE_IMAGE_REPLACE: $raw = $xaction->getNewValue(); return $raw->getReplacesImagePHID(); @@ -79,8 +66,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_STATUS: - case PholioTransaction::TYPE_IMAGE_DESCRIPTION: case PholioTransaction::TYPE_IMAGE_SEQUENCE: return $xaction->getNewValue(); case PholioTransaction::TYPE_IMAGE_REPLACE: @@ -185,17 +170,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $this->setNewImages($new_images); } - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_STATUS: - $object->setStatus($xaction->getNewValue()); - break; - } - } - private function getImageForXaction( PholioMock $mock, PhabricatorApplicationTransaction $xaction) { @@ -242,12 +216,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { } $object->attachImages($images); break; - case PholioTransaction::TYPE_IMAGE_DESCRIPTION: - $image = $this->getImageForXaction($object, $xaction); - $value = (string)head($xaction->getNewValue()); - $image->setDescription($value); - $image->save(); - break; case PholioTransaction::TYPE_IMAGE_SEQUENCE: $image = $this->getImageForXaction($object, $xaction); $value = (int)head($xaction->getNewValue()); @@ -276,8 +244,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $type = $u->getTransactionType(); switch ($type) { - case PholioTransaction::TYPE_STATUS: - return $v; case PholioTransaction::TYPE_IMAGE_REPLACE: $u_img = $u->getNewValue(); $v_img = $v->getNewValue(); @@ -287,7 +253,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { break; case PholioTransaction::TYPE_IMAGE_FILE: return $this->mergePHIDOrEdgeTransactions($u, $v); - case PholioTransaction::TYPE_IMAGE_DESCRIPTION: case PholioTransaction::TYPE_IMAGE_SEQUENCE: $raw_new_value_u = $u->getNewValue(); $raw_new_value_v = $v->getNewValue(); diff --git a/src/applications/pholio/storage/PholioTransaction.php b/src/applications/pholio/storage/PholioTransaction.php index 85688b84d2..84a833b29f 100644 --- a/src/applications/pholio/storage/PholioTransaction.php +++ b/src/applications/pholio/storage/PholioTransaction.php @@ -2,12 +2,8 @@ final class PholioTransaction extends PhabricatorModularTransaction { - // Edits to the high level mock - const TYPE_STATUS = 'status'; - // Edits to images within the mock const TYPE_IMAGE_FILE = 'image-file'; - const TYPE_IMAGE_DESCRIPTION = 'image-description'; const TYPE_IMAGE_REPLACE = 'image-replace'; const TYPE_IMAGE_SEQUENCE = 'image-sequence'; @@ -54,7 +50,7 @@ final class PholioTransaction extends PhabricatorModularTransaction { $phids[] = $new; $phids[] = $old; break; - case self::TYPE_IMAGE_DESCRIPTION: + case PholioImageDescriptionTransaction::TRANSACTIONTYPE: case PholioImageNameTransaction::TRANSACTIONTYPE: case self::TYPE_IMAGE_SEQUENCE: $phids[] = key($new); @@ -68,8 +64,6 @@ final class PholioTransaction extends PhabricatorModularTransaction { $old = $this->getOldValue(); switch ($this->getTransactionType()) { - case self::TYPE_IMAGE_DESCRIPTION: - return ($old === array(null => null)); // this is boring / silly to surface; changing sequence is NBD case self::TYPE_IMAGE_SEQUENCE: return true; @@ -86,13 +80,6 @@ final class PholioTransaction extends PhabricatorModularTransaction { switch ($this->getTransactionType()) { case self::TYPE_INLINE: return 'fa-comment'; - case self::TYPE_STATUS: - if ($new == PholioMock::STATUS_CLOSED) { - return 'fa-ban'; - } else { - return 'fa-check'; - } - case self::TYPE_IMAGE_DESCRIPTION: case self::TYPE_IMAGE_SEQUENCE: return 'fa-pencil'; case self::TYPE_IMAGE_FILE: @@ -110,13 +97,13 @@ final class PholioTransaction extends PhabricatorModularTransaction { case PhabricatorTransactions::TYPE_COMMENT: $tags[] = self::MAILTAG_COMMENT; break; - case self::TYPE_STATUS: + case PholioMockStatusTransaction::TRANSACTIONTYPE: $tags[] = self::MAILTAG_STATUS; break; case PholioMockNameTransaction::TRANSACTIONTYPE: case PholioMockDescriptionTransaction::TRANSACTIONTYPE: case PholioImageNameTransaction::TRANSACTIONTYPE: - case self::TYPE_IMAGE_DESCRIPTION: + case PholioImageDescriptionTransaction::TRANSACTIONTYPE: case self::TYPE_IMAGE_SEQUENCE: case self::TYPE_IMAGE_FILE: case self::TYPE_IMAGE_REPLACE: @@ -137,17 +124,6 @@ final class PholioTransaction extends PhabricatorModularTransaction { $type = $this->getTransactionType(); switch ($type) { - case self::TYPE_STATUS: - if ($new == PholioMock::STATUS_CLOSED) { - return pht( - '%s closed this mock.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s opened this mock.', - $this->renderHandleLink($author_phid)); - } - break; case self::TYPE_INLINE: $count = 1; foreach ($this->getTransactionGroup() as $xaction) { @@ -194,12 +170,6 @@ final class PholioTransaction extends PhabricatorModularTransaction { $this->renderHandleList($rem)); } break; - case self::TYPE_IMAGE_DESCRIPTION: - return pht( - '%s updated an image\'s (%s) description.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink(key($new))); - break; case self::TYPE_IMAGE_SEQUENCE: return pht( '%s updated an image\'s (%s) sequence.', @@ -220,19 +190,6 @@ final class PholioTransaction extends PhabricatorModularTransaction { $type = $this->getTransactionType(); switch ($type) { - case self::TYPE_STATUS: - if ($new == PholioMock::STATUS_CLOSED) { - return pht( - '%s closed a mock %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - } else { - return pht( - '%s opened a mock %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - } - break; case self::TYPE_INLINE: return pht( '%s added an inline comment to %s.', @@ -246,12 +203,6 @@ final class PholioTransaction extends PhabricatorModularTransaction { $this->renderHandleLink($author_phid), $this->renderHandleLink($object_phid)); break; - case self::TYPE_IMAGE_DESCRIPTION: - return pht( - '%s updated image descriptions of %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - break; case self::TYPE_IMAGE_SEQUENCE: return pht( '%s updated image sequence of %s.', @@ -268,12 +219,6 @@ final class PholioTransaction extends PhabricatorModularTransaction { $new = $this->getNewValue(); switch ($this->getTransactionType()) { - case self::TYPE_STATUS: - if ($new == PholioMock::STATUS_CLOSED) { - return PhabricatorTransactions::COLOR_INDIGO; - } else { - return PhabricatorTransactions::COLOR_GREEN; - } case self::TYPE_IMAGE_REPLACE: return PhabricatorTransactions::COLOR_YELLOW; case self::TYPE_IMAGE_FILE: @@ -291,16 +236,4 @@ final class PholioTransaction extends PhabricatorModularTransaction { return parent::getColor(); } - public function getNoEffectDescription() { - switch ($this->getTransactionType()) { - case self::TYPE_IMAGE_NAME: - return pht('The image title was not updated.'); - case self::TYPE_IMAGE_DESCRIPTION: - return pht('The image description was not updated.'); - case self::TYPE_IMAGE_SEQUENCE: - return pht('The image sequence was not updated.'); - } - - return parent::getNoEffectDescription(); - } } diff --git a/src/applications/pholio/xaction/PholioImageDescriptionTransaction.php b/src/applications/pholio/xaction/PholioImageDescriptionTransaction.php new file mode 100644 index 0000000000..32fd8da285 --- /dev/null +++ b/src/applications/pholio/xaction/PholioImageDescriptionTransaction.php @@ -0,0 +1,76 @@ +getImageForXaction($object); + if ($image) { + $description = $image->getDescription(); + $phid = $image->getPHID(); + } + return array($phid => $description); + } + + public function applyInternalEffects($object, $value) { + $image = $this->getImageForXaction($object); + $value = (string)head($this->getNewValue()); + $image->setDescription($value); + $image->save(); + } + + public function getTitle() { + $new = $this->getNewValue(); + + return pht( + '%s updated an image\'s (%s) description.', + $this->renderAuthor(), + $this->renderHandle(head_key($new))); + } + + public function getTitleForFeed() { + return pht( + '%s updated image descriptions of %s.', + $this->renderAuthor(), + $this->renderObject()); + } + + public function mergeTransactions( + $object, + PhabricatorApplicationTransaction $u, + PhabricatorApplicationTransaction $v) { + + $raw_new_value_u = $u->getNewValue(); + $raw_new_value_v = $v->getNewValue(); + $phid_u = head_key($raw_new_value_u); + $phid_v = head_key($raw_new_value_v); + if ($phid_u == $phid_v) { + return $v; + } + + return null; + } + + public function shouldHide() { + $old = $this->getOldValue(); + return ($old === array(null => null)); + } + + public function hasChangeDetailView() { + return true; + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + + return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setViewer($viewer) + ->setOldText(head($this->getOldValue())) + ->setNewText(head($this->getNewValue())); + } + +} diff --git a/src/applications/pholio/xaction/PholioImageNameTransaction.php b/src/applications/pholio/xaction/PholioImageNameTransaction.php index b888b1244b..33b01903ee 100644 --- a/src/applications/pholio/xaction/PholioImageNameTransaction.php +++ b/src/applications/pholio/xaction/PholioImageNameTransaction.php @@ -42,16 +42,6 @@ final class PholioImageNameTransaction $this->renderObject()); } - public function getIcon() { - $new = $this->getNewValue(); - - if ($new == PholioMock::STATUS_CLOSED) { - return 'fa-ban'; - } else { - return 'fa-check'; - } - } - public function mergeTransactions( $object, PhabricatorApplicationTransaction $u, diff --git a/src/applications/pholio/xaction/PholioMockStatusTransaction.php b/src/applications/pholio/xaction/PholioMockStatusTransaction.php new file mode 100644 index 0000000000..ccacca2b5e --- /dev/null +++ b/src/applications/pholio/xaction/PholioMockStatusTransaction.php @@ -0,0 +1,66 @@ +getStatus(); + } + + public function applyInternalEffects($object, $value) { + $object->setStatus($value); + } + + public function getTitle() { + $new = $this->getNewValue(); + + if ($new == PholioMock::STATUS_CLOSED) { + return pht( + '%s closed this mock.', + $this->renderAuthor()); + } else { + return pht( + '%s opened this mock.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + $new = $this->getNewValue(); + + if ($new == PholioMock::STATUS_CLOSED) { + return pht( + '%s closed mock %s.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s opened mock %s.', + $this->renderAuthor(), + $this->renderObject()); + } + } + + public function getIcon() { + $new = $this->getNewValue(); + + if ($new == PholioMock::STATUS_CLOSED) { + return 'fa-ban'; + } else { + return 'fa-check'; + } + } + + public function getColor() { + $new = $this->getNewValue(); + + if ($new == PholioMock::STATUS_CLOSED) { + return PhabricatorTransactions::COLOR_INDIGO; + } else { + return PhabricatorTransactions::COLOR_GREEN; + } + } + +}