Convert remaining Pholio image transactions to modular transaction framework

Summary: Also cleans up now-dead code relating to old transactions

Test Plan: Created lots of mocks, replaced their images, added/removed images, changed the sequence, verified expected DB xaction rows, Mock updates, and correct rendering of timeline.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D17892

src/__phutil_library_map__.php

@@ -4376,9 +4376,12 @@ phutil_register_library_map(array(
     '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 @@ phutil_register_library_map(array(
       'PhabricatorPolicyInterface',
     ),
     'PholioImageDescriptionTransaction' => 'PholioImageTransactionType',
+    'PholioImageFileTransaction' => 'PholioImageTransactionType',
     'PholioImageNameTransaction' => 'PholioImageTransactionType',
     'PholioImagePHIDType' => 'PhabricatorPHIDType',
     'PholioImageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PholioImageReplaceTransaction' => 'PholioImageTransactionType',
+    'PholioImageSequenceTransaction' => 'PholioImageTransactionType',
     'PholioImageTransactionType' => 'PholioTransactionType',
     'PholioImageUploadController' => 'PholioController',
     'PholioInlineController' => 'PholioController',

src/applications/pholio/controller/PholioMockEditController.php

@@ -151,7 +151,7 @@ final class PholioMockEditController extends PholioController {
             ->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 @@ final class PholioMockEditController extends PholioController {
             ->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 @@ final class PholioMockEditController extends PholioController {
                 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 @@ final class PholioMockEditController extends PholioController {
         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)));
         }

src/applications/pholio/editor/PholioMockEditor.php

@@ -17,7 +17,8 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
     $this->newImages = $new_images;
     return $this;
   }
-  private function getNewImages() {
+
+  public function getNewImages() {
     return $this->newImages;
   }
 
@@ -31,89 +32,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
 
     $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 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
 
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
-        case PholioTransaction::TYPE_IMAGE_FILE:
+        case PholioImageFileTransaction::TRANSACTIONTYPE:
-        case PholioTransaction::TYPE_IMAGE_REPLACE:
+        case PholioImageReplaceTransaction::TRANSACTIONTYPE:
           return true;
           break;
       }
@@ -148,7 +69,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
     $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 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
             }
           }
           break;
-        case PholioTransaction::TYPE_IMAGE_REPLACE:
+        case PholioImageReplaceTransaction::TRANSACTIONTYPE:
           $image = $xaction->getNewValue();
           $image->save();
           $new_images[] = $image;
@@ -170,67 +91,6 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
     $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 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
     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) {

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 @@ final class PholioTransaction extends PhabricatorModularTransaction {
     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 @@ final class PholioTransaction extends PhabricatorModularTransaction {
       case PholioMockDescriptionTransaction::TRANSACTIONTYPE:
       case PholioImageNameTransaction::TRANSACTIONTYPE:
       case PholioImageDescriptionTransaction::TRANSACTIONTYPE:
-      case self::TYPE_IMAGE_SEQUENCE:
+      case PholioImageSequenceTransaction::TRANSACTIONTYPE:
-      case self::TYPE_IMAGE_FILE:
+      case PholioImageFileTransaction::TRANSACTIONTYPE:
-      case self::TYPE_IMAGE_REPLACE:
+      case PholioImageReplaceTransaction::TRANSACTIONTYPE:
         $tags[] = self::MAILTAG_UPDATED;
         break;
       default:
@@ -137,45 +86,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
           $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 @@ final class PholioTransaction extends PhabricatorModularTransaction {
           $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();
-  }
-
 }

src/applications/pholio/xaction/PholioImageFileTransaction.php

@@ -0,0 +1,120 @@
+<?php
+
+final class PholioImageFileTransaction
+  extends PholioImageTransactionType {
+
+  const TRANSACTIONTYPE = 'image-file';
+
+  public function generateOldValue($object) {
+    $images = $object->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);
+  }
+
+}

src/applications/pholio/xaction/PholioImageReplaceTransaction.php

@@ -0,0 +1,68 @@
+<?php
+
+final class PholioImageReplaceTransaction
+  extends PholioImageTransactionType {
+
+  const TRANSACTIONTYPE = 'image-replace';
+
+  public function generateOldValue($object) {
+    $new_image = $this->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);
+  }
+
+}

src/applications/pholio/xaction/PholioImageSequenceTransaction.php

@@ -0,0 +1,60 @@
+<?php
+
+final class PholioImageSequenceTransaction
+  extends PholioImageTransactionType {
+
+  const TRANSACTIONTYPE = 'image-sequence';
+
+  public function generateOldValue($object) {
+    $sequence = null;
+    $phid = null;
+    $image = $this->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;
+    }
+  }
+
+}