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 @@ -4360,6 +4360,7 @@ '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 @@ '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 @@ 'PhabricatorMarkupInterface', 'PhabricatorPolicyInterface', ), + 'PholioImageDescriptionTransaction' => 'PholioImageTransactionType', 'PholioImageNameTransaction' => 'PholioImageTransactionType', 'PholioImagePHIDType' => 'PhabricatorPHIDType', 'PholioImageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', @@ -9949,6 +9952,7 @@ '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 --- a/src/applications/pholio/controller/PholioMockArchiveController.php +++ b/src/applications/pholio/controller/PholioMockArchiveController.php @@ -32,7 +32,7 @@ $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 --- a/src/applications/pholio/controller/PholioMockEditController.php +++ b/src/applications/pholio/controller/PholioMockEditController.php @@ -173,7 +173,7 @@ 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 --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -29,11 +29,9 @@ $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 @@ 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 @@ 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 @@ $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 @@ } $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 @@ $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 @@ 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 --- 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 @@ $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 @@ $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 @@ 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 @@ 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 @@ $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 @@ $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 @@ $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 @@ $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 @@ $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 @@ 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/PholioImageNameTransaction.php b/src/applications/pholio/xaction/PholioImageDescriptionTransaction.php copy from src/applications/pholio/xaction/PholioImageNameTransaction.php copy to src/applications/pholio/xaction/PholioImageDescriptionTransaction.php --- a/src/applications/pholio/xaction/PholioImageNameTransaction.php +++ b/src/applications/pholio/xaction/PholioImageDescriptionTransaction.php @@ -1,57 +1,44 @@ getImageForXaction($object); if ($image) { - $name = $image->getName(); + $description = $image->getDescription(); $phid = $image->getPHID(); } - return array($phid => $name); + return array($phid => $description); } public function applyInternalEffects($object, $value) { $image = $this->getImageForXaction($object); $value = (string)head($this->getNewValue()); - $image->setName($value); + $image->setDescription($value); $image->save(); } public function getTitle() { - $old = $this->getOldValue(); $new = $this->getNewValue(); return pht( - '%s renamed an image (%s) from %s to %s.', + '%s updated an image\'s (%s) description.', $this->renderAuthor(), - $this->renderHandle(key($new)), - $this->renderValue($old), - $this->renderValue($new)); + $this->renderHandle(head_key($new))); } public function getTitleForFeed() { return pht( - '%s updated the image names of %s.', + '%s updated image descriptions of %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 mergeTransactions( $object, PhabricatorApplicationTransaction $u, @@ -73,23 +60,17 @@ return ($old === array(null => null)); } - public function validateTransactions($object, array $xactions) { - $errors = array(); + public function hasChangeDetailView() { + return true; + } - $max_length = $object->getColumnMaximumByteLength('name'); - foreach ($xactions as $xaction) { - $new_value = head(array_values($xaction->getNewValue())); - $new_length = strlen($new_value); - if ($new_length > $max_length) { - $errors[] = $this->newInvalidError( - pht( - 'Mock image names must not be longer than %s character(s).', - new PhutilNumber($max_length))); - } - } + public function newChangeDetailView() { + $viewer = $this->getViewer(); - return $errors; + 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 --- a/src/applications/pholio/xaction/PholioImageNameTransaction.php +++ b/src/applications/pholio/xaction/PholioImageNameTransaction.php @@ -42,16 +42,6 @@ $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 --- /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; + } + } + +}