From 65b875d29d50a28d9a4cdf98606e789df1d66d9a Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Mon, 12 Aug 2013 13:09:07 -0700 Subject: [PATCH] Pholio - back end for image re-ordering Summary: companion diff to D6729. This is the back-end stuff, plus calls the JS in D6729 for when images are removed, un-removed, uploaded, or replaced. Fixes T3640. Test Plan: messed around with images. hit save - new order! temporarily showed these stories and got text about re-ordering stuff. Reviewers: epriestley, chad Reviewed By: epriestley CC: Korvin, aran Maniphest Tasks: T3640 Differential Revision: https://secure.phabricator.com/D6731 --- .../constants/PholioTransactionType.php | 1 + .../controller/PholioMockEditController.php | 11 +++++-- .../pholio/editor/PholioMockEditor.php | 26 +++++++++++++--- .../pholio/query/PholioMockQuery.php | 3 +- .../pholio/storage/PholioTransaction.php | 31 +++++++++++++++++++ .../pholio/behavior-pholio-mock-edit.js | 4 +++ 6 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/applications/pholio/constants/PholioTransactionType.php b/src/applications/pholio/constants/PholioTransactionType.php index 42713a0b12..e106628636 100644 --- a/src/applications/pholio/constants/PholioTransactionType.php +++ b/src/applications/pholio/constants/PholioTransactionType.php @@ -14,6 +14,7 @@ final class PholioTransactionType extends PholioConstants { const TYPE_IMAGE_NAME= 'image-name'; const TYPE_IMAGE_DESCRIPTION = 'image-description'; 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'; diff --git a/src/applications/pholio/controller/PholioMockEditController.php b/src/applications/pholio/controller/PholioMockEditController.php index 6d5c570d1a..e5b6ce464a 100644 --- a/src/applications/pholio/controller/PholioMockEditController.php +++ b/src/applications/pholio/controller/PholioMockEditController.php @@ -110,7 +110,8 @@ final class PholioMockEditController extends PholioController { ->setNewValue($value); } - $sequence = 0; + $order = $request->getStrList('imageOrder'); + $sequence_map = array_flip($order); $replaces = $request->getArr('replaces'); $replaces_map = array_flip($replaces); @@ -133,6 +134,7 @@ final class PholioMockEditController extends PholioController { $title = (string)$request->getStr('title_'.$file_phid); $description = (string)$request->getStr('description_'.$file_phid); + $sequence = $sequence_map[$file_phid]; if ($replaces_image_phid) { $replace_image = id(new PholioImage()) @@ -165,9 +167,12 @@ final class PholioMockEditController extends PholioController { PholioTransactionType::TYPE_IMAGE_DESCRIPTION) ->setNewValue( array($existing_image->getPHID() => $description)); - $existing_image->setSequence($sequence); + $xactions[] = id(new PholioTransaction()) + ->setTransactionType( + PholioTransactionType::TYPE_IMAGE_SEQUENCE) + ->setNewValue( + array($existing_image->getPHID() => $sequence)); } - $sequence++; } foreach ($mock_images as $file_phid => $mock_image) { if (!isset($files[$file_phid]) && !isset($replaces[$file_phid])) { diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php index 6da81252db..a7e66ed8e3 100644 --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -31,6 +31,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $types[] = PholioTransactionType::TYPE_IMAGE_NAME; $types[] = PholioTransactionType::TYPE_IMAGE_DESCRIPTION; $types[] = PholioTransactionType::TYPE_IMAGE_REPLACE; + $types[] = PholioTransactionType::TYPE_IMAGE_SEQUENCE; return $types; } @@ -55,7 +56,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $name = $image->getName(); $phid = $image->getPHID(); } - return array ($phid => $name); + return array($phid => $name); case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: $description = null; $phid = null; @@ -68,6 +69,15 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { case PholioTransactionType::TYPE_IMAGE_REPLACE: $raw = $xaction->getNewValue(); return $raw->getReplacesImagePHID(); + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: + $sequence = null; + $phid = null; + $image = $this->getImageForXaction($object, $xaction); + if ($image) { + $sequence = $image->getSequence(); + $phid = $image->getPHID(); + } + return array($phid => $sequence); } } @@ -80,6 +90,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { case PholioTransactionType::TYPE_DESCRIPTION: case PholioTransactionType::TYPE_IMAGE_NAME: case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: return $xaction->getNewValue(); case PholioTransactionType::TYPE_IMAGE_REPLACE: $raw = $xaction->getNewValue(); @@ -214,9 +225,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { } $object->attachImages($images); break; - case PholioTransactionType::TYPE_IMAGE_NAME: - $image = $this->getImageForXaction($object, $xaction); - $value = (string) head($xaction->getNewValue()); + case PholioTransactionType::TYPE_IMAGE_NAME: + $image = $this->getImageForXaction($object, $xaction); + $value = (string) head($xaction->getNewValue()); $image->setName($value); $image->save(); break; @@ -226,6 +237,12 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $image->setDescription($value); $image->save(); break; + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: + $image = $this->getImageForXaction($object, $xaction); + $value = (int) head($xaction->getNewValue()); + $image->setSequence($value); + $image->save(); + break; } } @@ -257,6 +274,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { return $this->mergePHIDOrEdgeTransactions($u, $v); case PholioTransactionType::TYPE_IMAGE_NAME: case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: $raw_new_value_u = $u->getNewValue(); $raw_new_value_v = $v->getNewValue(); $phid_u = key($raw_new_value_u); diff --git a/src/applications/pholio/query/PholioMockQuery.php b/src/applications/pholio/query/PholioMockQuery.php index 825a38442b..1f37935d28 100644 --- a/src/applications/pholio/query/PholioMockQuery.php +++ b/src/applications/pholio/query/PholioMockQuery.php @@ -124,7 +124,8 @@ final class PholioMockQuery foreach ($mocks as $mock) { $mock_images = idx($image_groups, $mock->getID(), array()); $mock->attachAllImages($mock_images); - $mock->attachImages(mfilter($mock_images, 'getIsObsolete', true)); + $active_images = mfilter($mock_images, 'getIsObsolete', true); + $mock->attachImages(msort($active_images, 'getSequence')); } } diff --git a/src/applications/pholio/storage/PholioTransaction.php b/src/applications/pholio/storage/PholioTransaction.php index a928852cd6..6f9f7f8cce 100644 --- a/src/applications/pholio/storage/PholioTransaction.php +++ b/src/applications/pholio/storage/PholioTransaction.php @@ -42,6 +42,7 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { break; case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: case PholioTransactionType::TYPE_IMAGE_NAME: + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: $phids[] = key($new); break; } @@ -58,6 +59,9 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { case PholioTransactionType::TYPE_IMAGE_NAME: case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: return ($old === array(null => null)); + // this is boring / silly to surface; changing sequence is NBD + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: + return true; } return parent::shouldHide(); @@ -71,6 +75,7 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { case PholioTransactionType::TYPE_DESCRIPTION: case PholioTransactionType::TYPE_IMAGE_NAME: case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: return 'edit'; case PholioTransactionType::TYPE_IMAGE_FILE: case PholioTransactionType::TYPE_IMAGE_REPLACE: @@ -168,6 +173,12 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { $this->renderHandleLink($author_phid), $this->renderHandleLink(key($new))); break; + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: + return pht( + '%s updated an image\'s (%s) sequence.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink(key($new))); + break; } return parent::getTitle(); @@ -228,6 +239,12 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { $this->renderHandleLink($author_phid), $this->renderHandleLink($object_phid)); break; + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: + return pht( + '%s updated image sequence of %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($object_phid)); + break; } return parent::getTitleForFeed(); @@ -282,6 +299,7 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { case PholioTransactionType::TYPE_DESCRIPTION: case PholioTransactionType::TYPE_IMAGE_NAME: case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: return PhabricatorTransactions::COLOR_BLUE; case PholioTransactionType::TYPE_IMAGE_REPLACE: return PhabricatorTransactions::COLOR_YELLOW; @@ -299,4 +317,17 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { return parent::getColor(); } + + public function getNoEffectDescription() { + switch ($this->getTransactionType()) { + case PholioTransactionType::TYPE_IMAGE_NAME: + return pht('The image title was not updated.'); + case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: + return pht('The image description was not updated.'); + case PholioTransactionType::TYPE_IMAGE_SEQUENCE: + return pht('The image sequence was not updated.'); + } + + return parent::getNoEffectDescription(); + } } diff --git a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-edit.js b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-edit.js index 8e18c8f28e..8b10f2dedf 100644 --- a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-edit.js +++ b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-edit.js @@ -35,9 +35,11 @@ JX.behavior('pholio-mock-edit', function(config) { JX.DOM.listen(undo, 'click', 'pholio-drop-undo', function(e) { e.kill(); JX.DOM.replace(undo, node); + synchronize_order(); }); JX.DOM.replace(node, undo); + synchronize_order(); }); @@ -116,6 +118,7 @@ JX.behavior('pholio-mock-edit', function(config) { build_update_control(new_node); JX.DOM.replace(node, new_node); + synchronize_order(); }) .start(); }); @@ -154,6 +157,7 @@ JX.behavior('pholio-mock-edit', function(config) { JX.DOM.replace(node, new_node); JX.DOM.alterClass(node, 'pholio-replacing', false); + synchronize_order(); }) .start(); });