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 @@ -4376,9 +4376,12 @@ 'PholioDefaultViewCapability' => 'applications/pholio/capability/PholioDefaultViewCapability.php', 'PholioImage' => 'applications/pholio/storage/PholioImage.php', 'PholioImageDescriptionTransaction' => 'applications/pholio/xaction/PholioImageDescriptionTransaction.php', + 'PholioImageFileTransaction' => 'applications/pholio/xaction/PholioImageFileTransaction.php', 'PholioImageNameTransaction' => 'applications/pholio/xaction/PholioImageNameTransaction.php', 'PholioImagePHIDType' => 'applications/pholio/phid/PholioImagePHIDType.php', 'PholioImageQuery' => 'applications/pholio/query/PholioImageQuery.php', + 'PholioImageReplaceTransaction' => 'applications/pholio/xaction/PholioImageReplaceTransaction.php', + 'PholioImageSequenceTransaction' => 'applications/pholio/xaction/PholioImageSequenceTransaction.php', 'PholioImageTransactionType' => 'applications/pholio/xaction/PholioImageTransactionType.php', 'PholioImageUploadController' => 'applications/pholio/controller/PholioImageUploadController.php', 'PholioInlineController' => 'applications/pholio/controller/PholioInlineController.php', @@ -9938,9 +9941,12 @@ 'PhabricatorPolicyInterface', ), 'PholioImageDescriptionTransaction' => 'PholioImageTransactionType', + 'PholioImageFileTransaction' => 'PholioImageTransactionType', 'PholioImageNameTransaction' => 'PholioImageTransactionType', 'PholioImagePHIDType' => 'PhabricatorPHIDType', 'PholioImageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PholioImageReplaceTransaction' => 'PholioImageTransactionType', + 'PholioImageSequenceTransaction' => 'PholioImageTransactionType', 'PholioImageTransactionType' => 'PholioTransactionType', 'PholioImageUploadController' => 'PholioController', 'PholioInlineController' => 'PholioController', 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 @@ -151,7 +151,7 @@ ->setSequence($sequence); $xactions[] = id(new PholioTransaction()) ->setTransactionType( - PholioTransaction::TYPE_IMAGE_REPLACE) + PholioImageReplaceTransaction::TRANSACTIONTYPE) ->setNewValue($replace_image); $posted_mock_images[] = $replace_image; } else if (!$existing_image) { // this is an add @@ -162,7 +162,7 @@ ->setDescription($description) ->setSequence($sequence); $xactions[] = id(new PholioTransaction()) - ->setTransactionType(PholioTransaction::TYPE_IMAGE_FILE) + ->setTransactionType(PholioImageFileTransaction::TRANSACTIONTYPE) ->setNewValue( array('+' => array($add_image))); $posted_mock_images[] = $add_image; @@ -178,7 +178,7 @@ array($existing_image->getPHID() => $description)); $xactions[] = id(new PholioTransaction()) ->setTransactionType( - PholioTransaction::TYPE_IMAGE_SEQUENCE) + PholioImageSequenceTransaction::TRANSACTIONTYPE) ->setNewValue( array($existing_image->getPHID() => $sequence)); @@ -189,7 +189,7 @@ if (!isset($files[$file_phid]) && !isset($replaces[$file_phid])) { // this is an outright delete $xactions[] = id(new PholioTransaction()) - ->setTransactionType(PholioTransaction::TYPE_IMAGE_FILE) + ->setTransactionType(PholioImageFileTransaction::TRANSACTIONTYPE) ->setNewValue( array('-' => array($mock_image))); } 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 @@ -17,7 +17,8 @@ $this->newImages = $new_images; return $this; } - private function getNewImages() { + + public function getNewImages() { return $this->newImages; } @@ -31,89 +32,9 @@ $types[] = PholioTransaction::TYPE_INLINE; - $types[] = PholioTransaction::TYPE_IMAGE_FILE; - $types[] = PholioTransaction::TYPE_IMAGE_REPLACE; - $types[] = PholioTransaction::TYPE_IMAGE_SEQUENCE; - return $types; } - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_IMAGE_FILE: - $images = $object->getImages(); - return mpull($images, 'getPHID'); - case PholioTransaction::TYPE_IMAGE_REPLACE: - $raw = $xaction->getNewValue(); - return $raw->getReplacesImagePHID(); - case PholioTransaction::TYPE_IMAGE_SEQUENCE: - $sequence = null; - $phid = null; - $image = $this->getImageForXaction($object, $xaction); - if ($image) { - $sequence = $image->getSequence(); - $phid = $image->getPHID(); - } - return array($phid => $sequence); - } - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_IMAGE_SEQUENCE: - return $xaction->getNewValue(); - case PholioTransaction::TYPE_IMAGE_REPLACE: - $raw = $xaction->getNewValue(); - return $raw->getPHID(); - case PholioTransaction::TYPE_IMAGE_FILE: - $raw_new_value = $xaction->getNewValue(); - $new_value = array(); - foreach ($raw_new_value as $key => $images) { - $new_value[$key] = mpull($images, 'getPHID'); - } - $xaction->setNewValue($new_value); - return $this->getPHIDTransactionNewValue($xaction); - } - } - - protected function extractFilePHIDsFromCustomTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - $images = $this->getNewImages(); - $images = mpull($images, null, 'getPHID'); - - switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_IMAGE_FILE: - $file_phids = array(); - foreach ($xaction->getNewValue() as $image_phid) { - $image = idx($images, $image_phid); - if (!$image) { - continue; - } - $file_phids[] = $image->getFilePHID(); - } - return $file_phids; - case PholioTransaction::TYPE_IMAGE_REPLACE: - $image_phid = $xaction->getNewValue(); - $image = idx($images, $image_phid); - - if ($image) { - return array($image->getFilePHID()); - } - break; - } - - return parent::extractFilePHIDsFromCustomTransaction($object, $xaction); - } - - protected function transactionHasEffect( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { @@ -132,8 +53,8 @@ foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_IMAGE_FILE: - case PholioTransaction::TYPE_IMAGE_REPLACE: + case PholioImageFileTransaction::TRANSACTIONTYPE: + case PholioImageReplaceTransaction::TRANSACTIONTYPE: return true; break; } @@ -148,7 +69,7 @@ $new_images = array(); foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_IMAGE_FILE: + case PholioImageFileTransaction::TRANSACTIONTYPE: $new_value = $xaction->getNewValue(); foreach ($new_value as $key => $txn_images) { if ($key != '+') { @@ -160,7 +81,7 @@ } } break; - case PholioTransaction::TYPE_IMAGE_REPLACE: + case PholioImageReplaceTransaction::TRANSACTIONTYPE: $image = $xaction->getNewValue(); $image->save(); $new_images[] = $image; @@ -170,67 +91,6 @@ $this->setNewImages($new_images); } - private function getImageForXaction( - PholioMock $mock, - PhabricatorApplicationTransaction $xaction) { - $raw_new_value = $xaction->getNewValue(); - $image_phid = key($raw_new_value); - $images = $mock->getImages(); - foreach ($images as $image) { - if ($image->getPHID() == $image_phid) { - return $image; - } - } - return null; - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - return; - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PholioTransaction::TYPE_IMAGE_FILE: - $old_map = array_fuse($xaction->getOldValue()); - $new_map = array_fuse($xaction->getNewValue()); - - $obsolete_map = array_diff_key($old_map, $new_map); - $images = $object->getImages(); - foreach ($images as $seq => $image) { - if (isset($obsolete_map[$image->getPHID()])) { - $image->setIsObsolete(1); - $image->save(); - unset($images[$seq]); - } - } - $object->attachImages($images); - break; - case PholioTransaction::TYPE_IMAGE_REPLACE: - $old = $xaction->getOldValue(); - $images = $object->getImages(); - foreach ($images as $seq => $image) { - if ($image->getPHID() == $old) { - $image->setIsObsolete(1); - $image->save(); - unset($images[$seq]); - } - } - $object->attachImages($images); - break; - case PholioTransaction::TYPE_IMAGE_SEQUENCE: - $image = $this->getImageForXaction($object, $xaction); - $value = (int)head($xaction->getNewValue()); - $image->setSequence($value); - $image->save(); - break; - } - } - protected function applyFinalEffects( PhabricatorLiskDAO $object, array $xactions) { @@ -244,35 +104,6 @@ return $xactions; } - protected function mergeTransactions( - PhabricatorApplicationTransaction $u, - PhabricatorApplicationTransaction $v) { - - $type = $u->getTransactionType(); - switch ($type) { - case PholioTransaction::TYPE_IMAGE_REPLACE: - $u_img = $u->getNewValue(); - $v_img = $v->getNewValue(); - if ($u_img->getReplacesImagePHID() == $v_img->getReplacesImagePHID()) { - return $v; - } - break; - case PholioTransaction::TYPE_IMAGE_FILE: - return $this->mergePHIDOrEdgeTransactions($u, $v); - case PholioTransaction::TYPE_IMAGE_SEQUENCE: - $raw_new_value_u = $u->getNewValue(); - $raw_new_value_v = $v->getNewValue(); - $phid_u = key($raw_new_value_u); - $phid_v = key($raw_new_value_v); - if ($phid_u == $phid_v) { - return $v; - } - break; - } - - return parent::mergeTransactions($u, $v); - } - protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { 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,11 +2,6 @@ final class PholioTransaction extends PhabricatorModularTransaction { - // Edits to images within the mock - const TYPE_IMAGE_FILE = 'image-file'; - const TYPE_IMAGE_REPLACE = 'image-replace'; - const TYPE_IMAGE_SEQUENCE = 'image-sequence'; - // Your witty commentary at the mock : image : x,y level const TYPE_INLINE = 'inline'; @@ -35,56 +30,10 @@ return new PholioTransactionView(); } - public function getRequiredHandlePHIDs() { - $phids = parent::getRequiredHandlePHIDs(); - $phids[] = $this->getObjectPHID(); - - $new = $this->getNewValue(); - $old = $this->getOldValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_IMAGE_FILE: - $phids = array_merge($phids, $new, $old); - break; - case self::TYPE_IMAGE_REPLACE: - $phids[] = $new; - $phids[] = $old; - break; - case PholioImageDescriptionTransaction::TRANSACTIONTYPE: - case PholioImageNameTransaction::TRANSACTIONTYPE: - case self::TYPE_IMAGE_SEQUENCE: - $phids[] = key($new); - break; - } - - return $phids; - } - - public function shouldHide() { - $old = $this->getOldValue(); - - switch ($this->getTransactionType()) { - // this is boring / silly to surface; changing sequence is NBD - case self::TYPE_IMAGE_SEQUENCE: - return true; - } - - return parent::shouldHide(); - } - public function getIcon() { - - $new = $this->getNewValue(); - $old = $this->getOldValue(); - switch ($this->getTransactionType()) { case self::TYPE_INLINE: return 'fa-comment'; - case self::TYPE_IMAGE_SEQUENCE: - return 'fa-pencil'; - case self::TYPE_IMAGE_FILE: - case self::TYPE_IMAGE_REPLACE: - return 'fa-picture-o'; } return parent::getIcon(); @@ -104,9 +53,9 @@ case PholioMockDescriptionTransaction::TRANSACTIONTYPE: case PholioImageNameTransaction::TRANSACTIONTYPE: case PholioImageDescriptionTransaction::TRANSACTIONTYPE: - case self::TYPE_IMAGE_SEQUENCE: - case self::TYPE_IMAGE_FILE: - case self::TYPE_IMAGE_REPLACE: + case PholioImageSequenceTransaction::TRANSACTIONTYPE: + case PholioImageFileTransaction::TRANSACTIONTYPE: + case PholioImageReplaceTransaction::TRANSACTIONTYPE: $tags[] = self::MAILTAG_UPDATED; break; default: @@ -137,45 +86,6 @@ $this->renderHandleLink($author_phid), $count); break; - case self::TYPE_IMAGE_REPLACE: - return pht( - '%s replaced %s with %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($old), - $this->renderHandleLink($new)); - break; - case self::TYPE_IMAGE_FILE: - $add = array_diff($new, $old); - $rem = array_diff($old, $new); - - if ($add && $rem) { - return pht( - '%s edited image(s), added %d: %s; removed %d: %s.', - $this->renderHandleLink($author_phid), - count($add), - $this->renderHandleList($add), - count($rem), - $this->renderHandleList($rem)); - } else if ($add) { - return pht( - '%s added %d image(s): %s.', - $this->renderHandleLink($author_phid), - count($add), - $this->renderHandleList($add)); - } else { - return pht( - '%s removed %d image(s): %s.', - $this->renderHandleLink($author_phid), - count($rem), - $this->renderHandleList($rem)); - } - break; - case self::TYPE_IMAGE_SEQUENCE: - return pht( - '%s updated an image\'s (%s) sequence.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink(key($new))); - break; } return parent::getTitle(); @@ -196,44 +106,9 @@ $this->renderHandleLink($author_phid), $this->renderHandleLink($object_phid)); break; - case self::TYPE_IMAGE_REPLACE: - case self::TYPE_IMAGE_FILE: - return pht( - '%s updated images of %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - break; - case self::TYPE_IMAGE_SEQUENCE: - return pht( - '%s updated image sequence of %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid)); - break; } return parent::getTitleForFeed(); } - public function getColor() { - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_IMAGE_REPLACE: - return PhabricatorTransactions::COLOR_YELLOW; - case self::TYPE_IMAGE_FILE: - $add = array_diff($new, $old); - $rem = array_diff($old, $new); - if ($add && $rem) { - return PhabricatorTransactions::COLOR_YELLOW; - } else if ($add) { - return PhabricatorTransactions::COLOR_GREEN; - } else { - return PhabricatorTransactions::COLOR_RED; - } - } - - return parent::getColor(); - } - } diff --git a/src/applications/pholio/xaction/PholioImageFileTransaction.php b/src/applications/pholio/xaction/PholioImageFileTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/pholio/xaction/PholioImageFileTransaction.php @@ -0,0 +1,120 @@ +getImages(); + return array_values(mpull($images, 'getPHID')); + } + + public function generateNewValue($object, $value) { + $new_value = array(); + foreach ($value as $key => $images) { + $new_value[$key] = mpull($images, 'getPHID'); + } + $old = array_fuse($this->getOldValue()); + return $this->getEditor()->getPHIDList($old, $new_value); + } + + public function applyInternalEffects($object, $value) { + $old_map = array_fuse($this->getOldValue()); + $new_map = array_fuse($this->getNewValue()); + + $obsolete_map = array_diff_key($old_map, $new_map); + $images = $object->getImages(); + foreach ($images as $seq => $image) { + if (isset($obsolete_map[$image->getPHID()])) { + $image->setIsObsolete(1); + $image->save(); + unset($images[$seq]); + } + } + $object->attachImages($images); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $add = array_diff($new, $old); + $rem = array_diff($old, $new); + + if ($add && $rem) { + return pht( + '%s edited image(s), added %d: %s; removed %d: %s.', + $this->renderAuthor(), + count($add), + $this->renderHandleList($add), + count($rem), + $this->renderHandleList($rem)); + } else if ($add) { + return pht( + '%s added %d image(s): %s.', + $this->renderAuthor(), + count($add), + $this->renderHandleList($add)); + } else { + return pht( + '%s removed %d image(s): %s.', + $this->renderAuthor(), + count($rem), + $this->renderHandleList($rem)); + } + } + + public function getTitleForFeed() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + return pht( + '%s updated images of %s.', + $this->renderAuthor(), + $this->renderObject()); + } + + public function getIcon() { + return 'fa-picture-o'; + } + + public function getColor() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $add = array_diff($new, $old); + $rem = array_diff($old, $new); + if ($add && $rem) { + return PhabricatorTransactions::COLOR_YELLOW; + } else if ($add) { + return PhabricatorTransactions::COLOR_GREEN; + } else { + return PhabricatorTransactions::COLOR_RED; + } + } + + public function extractFilePHIDs($object, $value) { + $images = $this->getEditor()->getNewImages(); + $images = mpull($images, null, 'getPHID'); + + + $file_phids = array(); + foreach ($value as $image_phid) { + $image = idx($images, $image_phid); + if (!$image) { + continue; + } + $file_phids[] = $image->getFilePHID(); + } + return $file_phids; + } + + public function mergeTransactions( + $object, + PhabricatorApplicationTransaction $u, + PhabricatorApplicationTransaction $v) { + return $this->getEditor()->mergePHIDOrEdgeTransactions($u, $v); + } + +} diff --git a/src/applications/pholio/xaction/PholioImageReplaceTransaction.php b/src/applications/pholio/xaction/PholioImageReplaceTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/pholio/xaction/PholioImageReplaceTransaction.php @@ -0,0 +1,68 @@ +getNewValue(); + return $new_image->getReplacesImagePHID(); + } + + public function generateNewValue($object, $value) { + return $value->getPHID(); + } + + public function applyInternalEffects($object, $value) { + $old = $this->getOldValue(); + $images = $object->getImages(); + foreach ($images as $seq => $image) { + if ($image->getPHID() == $old) { + $image->setIsObsolete(1); + $image->save(); + unset($images[$seq]); + } + } + $object->attachImages($images); + } + + public function getTitle() { + return pht( + '%s replaced %s with %s.', + $this->renderAuthor(), + $this->renderOldHandle(), + $this->renderNewHandle()); + } + + public function getTitleForFeed() { + return pht( + '%s updated images of %s.', + $this->renderAuthor(), + $this->renderObject()); + } + + public function getIcon() { + return 'fa-picture-o'; + } + + public function getColor() { + return PhabricatorTransactions::COLOR_YELLOW; + } + + public function mergeTransactions( + $object, + PhabricatorApplicationTransaction $u, + PhabricatorApplicationTransaction $v) { + $u_img = $u->getNewValue(); + $v_img = $v->getNewValue(); + if ($u_img->getReplacesImagePHID() == $v_img->getReplacesImagePHID()) { + return $v; + } + } + + public function extractFilePHIDs($object, $value) { + return array($value); + } + +} diff --git a/src/applications/pholio/xaction/PholioImageSequenceTransaction.php b/src/applications/pholio/xaction/PholioImageSequenceTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/pholio/xaction/PholioImageSequenceTransaction.php @@ -0,0 +1,60 @@ +getImageForXaction($object); + if ($image) { + $sequence = $image->getSequence(); + $phid = $image->getPHID(); + } + return array($phid => $sequence); + } + + public function applyInternalEffects($object, $value) { + $image = $this->getImageForXaction($object); + $value = (int)head($this->getNewValue()); + $image->setSequence($value); + $image->save(); + } + + public function getTitle() { + $new = $this->getNewValue(); + + return pht( + '%s updated an image\'s (%s) sequence.', + $this->renderAuthor(), + $this->renderHandleLink(key($new))); + } + + public function getTitleForFeed() { + return pht( + '%s updated image sequence of %s.', + $this->renderAuthor(), + $this->renderObject()); + } + + public function shouldHide() { + // this is boring / silly to surface; changing sequence is NBD + return true; + } + + public function mergeTransactions( + $object, + PhabricatorApplicationTransaction $u, + PhabricatorApplicationTransaction $v) { + $raw_new_value_u = $u->getNewValue(); + $raw_new_value_v = $v->getNewValue(); + $phid_u = key($raw_new_value_u); + $phid_v = key($raw_new_value_v); + if ($phid_u == $phid_v) { + return $v; + } + } + +}