Pholio - add concept of replacing images and primitive history view

Summary:
Now you can actually replace an image! Ref T3572. This ended up needing a wee bit of infrastructure to work...

 - add replace image transaction to pholio
 - add replacesImagePHID to PholioImage
 - tweaks to editor to properly update images with respect to replacement
   - add edges to track replacement
 - expose getNodes on graph query infrastructure to query the entire graph of who replaced who
 - move pholio image to new phid infrastructure

Still TODO - the history view should get chopped out a bit from the current view - no more inline comments / generally less functionality plus maybe a tweak or two to make this more sensical.

Test Plan: replaced images and played with history controller a little. works okay.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3572

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

resources/sql/patches/20130722.pholioreplace.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_pholio.pholio_image
+  ADD COLUMN replacesImagePHID VARCHAR(64) NULL COLLATE utf8_bin;

src/__phutil_library_map__.php

@@ -1743,6 +1743,8 @@ phutil_register_library_map(array(
     'PholioController' => 'applications/pholio/controller/PholioController.php',
     'PholioDAO' => 'applications/pholio/storage/PholioDAO.php',
     'PholioImage' => 'applications/pholio/storage/PholioImage.php',
+    'PholioImageHistoryController' => 'applications/pholio/controller/PholioImageHistoryController.php',
+    'PholioImageQuery' => 'applications/pholio/query/PholioImageQuery.php',
     'PholioImageUploadController' => 'applications/pholio/controller/PholioImageUploadController.php',
     'PholioInlineCommentEditView' => 'applications/pholio/view/PholioInlineCommentEditView.php',
     'PholioInlineCommentSaveView' => 'applications/pholio/view/PholioInlineCommentSaveView.php',
@@ -1764,6 +1766,7 @@ phutil_register_library_map(array(
     'PholioMockQuery' => 'applications/pholio/query/PholioMockQuery.php',
     'PholioMockSearchEngine' => 'applications/pholio/query/PholioMockSearchEngine.php',
     'PholioMockViewController' => 'applications/pholio/controller/PholioMockViewController.php',
+    'PholioPHIDTypeImage' => 'applications/pholio/phid/PholioPHIDTypeImage.php',
     'PholioPHIDTypeMock' => 'applications/pholio/phid/PholioPHIDTypeMock.php',
     'PholioRemarkupRule' => 'applications/pholio/remarkup/PholioRemarkupRule.php',
     'PholioReplyHandler' => 'applications/pholio/mail/PholioReplyHandler.php',
@@ -3795,7 +3798,10 @@ phutil_register_library_map(array(
     array(
       0 => 'PholioDAO',
       1 => 'PhabricatorMarkupInterface',
+      2 => 'PhabricatorPolicyInterface',
     ),
+    'PholioImageHistoryController' => 'PholioController',
+    'PholioImageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PholioImageUploadController' => 'PholioController',
     'PholioInlineCommentEditView' => 'AphrontView',
     'PholioInlineCommentSaveView' => 'AphrontView',
@@ -3829,6 +3835,7 @@ phutil_register_library_map(array(
     'PholioMockQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PholioMockSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'PholioMockViewController' => 'PholioController',
+    'PholioPHIDTypeImage' => 'PhabricatorPHIDType',
     'PholioPHIDTypeMock' => 'PhabricatorPHIDType',
     'PholioRemarkupRule' => 'PhabricatorRemarkupRuleObject',
     'PholioReplyHandler' => 'PhabricatorMailReplyHandler',

src/applications/phid/PhabricatorObjectHandle.php

@@ -109,7 +109,6 @@ final class PhabricatorObjectHandle
 
     static $map = array(
       PhabricatorPHIDConstants::PHID_TYPE_USER => 'User',
-      PhabricatorPHIDConstants::PHID_TYPE_PIMG => 'Pholio Image',
       PhabricatorPHIDConstants::PHID_TYPE_BLOG => 'Blog',
       PhabricatorPHIDConstants::PHID_TYPE_POST => 'Post',
       PhabricatorPHIDConstants::PHID_TYPE_LEGD => 'Legalpad Document',

src/applications/phid/PhabricatorPHIDConstants.php

@@ -17,7 +17,6 @@ final class PhabricatorPHIDConstants {
   const PHID_TYPE_TOBJ    = 'TOBJ';
   const PHID_TYPE_BLOG    = 'BLOG';
   const PHID_TYPE_ANSW    = 'ANSW';
-  const PHID_TYPE_PIMG    = 'PIMG';
   const PHID_TYPE_CONP    = 'CONP';
   const PHID_TYPE_ACNT    = 'ACNT';
   const PHID_TYPE_PDCT    = 'PDCT';

src/applications/phid/handle/PhabricatorObjectHandleData.php

@@ -68,11 +68,6 @@ final class PhabricatorObjectHandleData {
           $phids);
         return mpull($projects, null, 'getPHID');
 
-      case PhabricatorPHIDConstants::PHID_TYPE_PIMG:
-        $images = id(new PholioImage())
-          ->loadAllWhere('phid IN (%Ls)', $phids);
-        return mpull($images, null, 'getPHID');
-
       case PhabricatorPHIDConstants::PHID_TYPE_XACT:
         $subtypes = array();
         foreach ($phids as $phid) {
@@ -301,25 +296,6 @@ final class PhabricatorObjectHandleData {
           }
           break;
 
-        case PhabricatorPHIDConstants::PHID_TYPE_PIMG:
-          foreach ($phids as $phid) {
-            $handle = new PhabricatorObjectHandle();
-            $handle->setPHID($phid);
-            $handle->setType($type);
-            if (empty($objects[$phid])) {
-              $handle->setName('Unknown Image');
-            } else {
-              $image = $objects[$phid];
-              $handle->setName($image->getName());
-              $handle->setFullName($image->getName());
-              $handle->setURI(
-                '/M'.$image->getMockID().'/'.$image->getID().'/');
-              $handle->setComplete(true);
-            }
-            $handles[$phid] = $handle;
-          }
-          break;
-
         case PhabricatorPHIDConstants::PHID_TYPE_LEGD:
           foreach ($phids as $phid) {
             $handle = new PhabricatorObjectHandle();

src/applications/pholio/application/PhabricatorApplicationPholio.php

@@ -58,6 +58,7 @@ final class PhabricatorApplicationPholio extends PhabricatorApplication {
         ),
         'image/' => array(
           'upload/' => 'PholioImageUploadController',
+          'history/(?P<id>\d+)/' => 'PholioImageHistoryController',
         ),
       ),
     );

src/applications/pholio/constants/PholioTransactionType.php

@@ -13,6 +13,7 @@ final class PholioTransactionType extends PholioConstants {
   const TYPE_IMAGE_FILE = 'image-file';
   const TYPE_IMAGE_NAME= 'image-name';
   const TYPE_IMAGE_DESCRIPTION = 'image-description';
+  const TYPE_IMAGE_REPLACE = 'image-replace';
 
   /* your witty commentary at the mock : image : x,y level */
   const TYPE_INLINE  = 'inline';

src/applications/pholio/controller/PholioImageHistoryController.php

@@ -0,0 +1,96 @@
+<?php
+
+/**
+ * @group pholio
+ */
+final class PholioImageHistoryController extends PholioController {
+
+  private $imageID;
+
+  public function willProcessRequest(array $data) {
+    $this->imageID = $data['id'];
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $image = id(new PholioImageQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->imageID))
+      ->executeOne();
+
+    if (!$image) {
+      return new Aphront404Response();
+    }
+
+    // note while we have a mock object, its missing images we need to show
+    // the history of what's happened here.
+    // fetch the real deal
+    //
+    $mock = id(new PholioMockQuery())
+      ->setViewer($user)
+      ->needImages(true)
+      ->withIDs(array($image->getMockID()))
+      ->executeOne();
+
+    $phids = array($mock->getAuthorPHID());
+    $this->loadHandles($phids);
+
+    $engine = id(new PhabricatorMarkupEngine())
+      ->setViewer($user);
+    $engine->addObject($mock, PholioMock::MARKUP_FIELD_DESCRIPTION);
+    $engine->process();
+
+
+    $images = $mock->getImageHistorySet($this->imageID);
+    // TODO - this is a hack until we specialize the view object
+    $mock->attachImages($images);
+    $latest_image = last($images);
+
+    $title = pht(
+      'Image history for "%s" from the mock "%s."',
+      $latest_image->getName(),
+      $mock->getName());
+
+    $header = id(new PhabricatorHeaderView())
+      ->setHeader($title);
+
+    require_celerity_resource('pholio-css');
+    require_celerity_resource('pholio-inline-comments-css');
+
+    $comment_form_id = celerity_generate_unique_node_id();
+    $output = id(new PholioMockImagesView())
+      ->setRequestURI($request->getRequestURI())
+      ->setCommentFormID($comment_form_id)
+      ->setUser($user)
+      ->setMock($mock)
+      ->setImageID($this->imageID);
+
+    $crumbs = $this->buildApplicationCrumbs();
+    $crumbs
+      ->addCrumb(
+        id(new PhabricatorCrumbView())
+        ->setName('M'.$mock->getID())
+        ->setHref('/M'.$mock->getID()))
+      ->addCrumb(
+        id(new PhabricatorCrumbView())
+        ->setName('Image History')
+        ->setHref($request->getRequestURI()));
+
+    $content = array(
+      $crumbs,
+      $header,
+      $output->render(),
+    );
+
+    return $this->buildApplicationPage(
+      $content,
+      array(
+        'title' => 'M'.$mock->getID().' '.$title,
+        'device' => true,
+        'pageObjects' => array($mock->getPHID()),
+      ));
+  }
+
+}

src/applications/pholio/controller/PholioMockEditController.php

@@ -111,12 +111,41 @@ final class PholioMockEditController extends PholioController {
         }
 
         $sequence = 0;
+        $replaces = $request->getArr('replaces');
+        $replaces_map = array_flip($replaces);
+
+        /**
+         * Foreach file posted, check to see whether we are replacing an image,
+         * adding an image, or simply updating image metadata. Create
+         * transactions for these cases as appropos.
+         */
         foreach ($files as $file_phid => $file) {
-          $mock_image = idx($mock_images, $file_phid);
+          $replaces_image_phid = null;
+          if (isset($replaces_map[$file_phid])) {
+            $old_file_phid = $replaces_map[$file_phid];
+            $old_image = idx($mock_images, $old_file_phid);
+            if ($old_image) {
+              $replaces_image_phid = $old_image->getPHID();
+            }
+          }
+
+          $existing_image = idx($mock_images, $file_phid);
+
           $title = (string)$request->getStr('title_'.$file_phid);
           $description = (string)$request->getStr('description_'.$file_phid);
-          if (!$mock_image) {
+
-            // this is an add
+          if ($replaces_image_phid) {
+            $replace_image = id(new PholioImage())
+              ->setReplacesImagePHID($replaces_image_phid)
+              ->setFilePhid($file_phid)
+              ->setName(strlen($title) ? $title : $file->getName())
+              ->setDescription($description)
+              ->setSequence($sequence);
+            $xactions[] = id(new PholioTransaction())
+              ->setTransactionType(
+                PholioTransactionType::TYPE_IMAGE_REPLACE)
+              ->setNewValue($replace_image);
+         } else if (!$existing_image) { // this is an add
             $add_image = id(new PholioImage())
               ->setFilePhid($file_phid)
               ->setName(strlen($title) ? $title : $file->getName())
@@ -127,22 +156,22 @@ final class PholioMockEditController extends PholioController {
               ->setNewValue(
                 array('+' => array($add_image)));
           } else {
-            // update (maybe)
             $xactions[] = id(new PholioTransaction())
               ->setTransactionType(PholioTransactionType::TYPE_IMAGE_NAME)
               ->setNewValue(
-                array($mock_image->getPHID() => $title));
+                array($existing_image->getPHID() => $title));
             $xactions[] = id(new PholioTransaction())
               ->setTransactionType(
                 PholioTransactionType::TYPE_IMAGE_DESCRIPTION)
-              ->setNewValue(array($mock_image->getPHID() => $description));
+                ->setNewValue(
-            $mock_image->setSequence($sequence);
+                  array($existing_image->getPHID() => $description));
+            $existing_image->setSequence($sequence);
           }
           $sequence++;
         }
         foreach ($mock_images as $file_phid => $mock_image) {
-          if (!isset($files[$file_phid])) {
+          if (!isset($files[$file_phid]) && !isset($replaces[$file_phid])) {
-            // this is a delete
+            // this is an outright delete
             $xactions[] = id(new PholioTransaction())
               ->setTransactionType(PholioTransactionType::TYPE_IMAGE_FILE)
               ->setNewValue(

src/applications/pholio/controller/PholioMockViewController.php

@@ -76,6 +76,8 @@ final class PholioMockViewController extends PholioController {
     require_celerity_resource('pholio-css');
     require_celerity_resource('pholio-inline-comments-css');
 
+    $image_status = $this->getImageStatus($mock, $this->imageID);
+
     $comment_form_id = celerity_generate_unique_node_id();
     $output = id(new PholioMockImagesView())
       ->setRequestURI($request->getRequestURI())
@@ -100,6 +102,7 @@ final class PholioMockViewController extends PholioController {
 
     $content = array(
       $crumbs,
+      $image_status,
       $header,
       $actions,
       $properties,
@@ -117,6 +120,43 @@ final class PholioMockViewController extends PholioController {
       ));
   }
 
+  private function getImageStatus(PholioMock $mock, $image_id) {
+    $status = null;
+    $images = $mock->getImages();
+    foreach ($images as $image) {
+      if ($image->getID() == $image_id) {
+        return $status;
+      }
+    }
+
+    $images = $mock->getAllImages();
+    $images = mpull($images, null, 'getID');
+    $image = idx($images, $image_id);
+
+    if ($image) {
+      $history = $mock->getImageUpdateSet($image_id);
+      $latest_image = last($history);
+      $href = $this->getApplicationURI(
+        'image/history/'.$latest_image->getID().'/');
+      $status = id(new AphrontErrorView())
+        ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
+        ->setTitle(pht('The requested image is obsolete.'))
+        ->appendChild(phutil_tag(
+          'p',
+          array(),
+          array(
+            pht('You are viewing this mock with the latest image set.'),
+            ' ',
+            phutil_tag(
+              'a',
+              array('href' => $href),
+              pht(
+                'Click here to see the history of the now obsolete image.')))));
+    }
+
+    return $status;
+  }
+
   private function buildActionView(PholioMock $mock) {
     $user = $this->getRequest()->getUser();
 

src/applications/pholio/editor/PholioMockEditor.php

@@ -30,6 +30,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
     $types[] = PholioTransactionType::TYPE_IMAGE_FILE;
     $types[] = PholioTransactionType::TYPE_IMAGE_NAME;
     $types[] = PholioTransactionType::TYPE_IMAGE_DESCRIPTION;
+    $types[] = PholioTransactionType::TYPE_IMAGE_REPLACE;
 
     return $types;
   }
@@ -64,6 +65,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
           $phid = $image->getPHID();
         }
         return array($phid => $description);
+      case PholioTransactionType::TYPE_IMAGE_REPLACE:
+        $raw = $xaction->getNewValue();
+        return $raw->getReplacesImagePHID();
     }
   }
 
@@ -77,6 +81,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
       case PholioTransactionType::TYPE_IMAGE_NAME:
       case PholioTransactionType::TYPE_IMAGE_DESCRIPTION:
         return $xaction->getNewValue();
+      case PholioTransactionType::TYPE_IMAGE_REPLACE:
+        $raw = $xaction->getNewValue();
+        return $raw->getPHID();
       case PholioTransactionType::TYPE_IMAGE_FILE:
         $raw_new_value = $xaction->getNewValue();
         $new_value = array();
@@ -107,6 +114,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
         case PholioTransactionType::TYPE_IMAGE_FILE:
+        case PholioTransactionType::TYPE_IMAGE_REPLACE:
           return true;
           break;
       }
@@ -133,6 +141,11 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
             }
           }
           break;
+        case PholioTransactionType::TYPE_IMAGE_REPLACE:
+          $image = $xaction->getNewValue();
+          $image->save();
+          $new_images[] = $image;
+          break;
       }
     }
     $this->setNewImages($new_images);
@@ -189,6 +202,18 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
         }
         $object->attachImages($images);
         break;
+      case PholioTransactionType::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 PholioTransactionType::TYPE_IMAGE_NAME:
       $image = $this->getImageForXaction($object, $xaction);
       $value = (string) head($xaction->getNewValue());
@@ -224,6 +249,10 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
       case PholioTransactionType::TYPE_NAME:
       case PholioTransactionType::TYPE_DESCRIPTION:
         return $v;
+      case PholioTransactionType::TYPE_IMAGE_REPLACE:
+        if ($u->getNewValue() == $v->getOldValue()) {
+          return $v;
+        }
       case PholioTransactionType::TYPE_IMAGE_FILE:
         return $this->mergePHIDOrEdgeTransactions($u, $v);
       case PholioTransactionType::TYPE_IMAGE_NAME:

src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php

@@ -42,6 +42,7 @@ final class PhabricatorPholioMockTestDataGenerator
       $image = new PholioImage();
       $image->setFilePHID($file->getPHID());
       $image->setSequence($sequence++);
+      $image->attachMock($mock);
       $images[] = $image;
     }
 

src/applications/pholio/phid/PholioPHIDTypeImage.php

@@ -0,0 +1,47 @@
+<?php
+
+final class PholioPHIDTypeImage extends PhabricatorPHIDType {
+
+  const TYPECONST = 'PIMG';
+
+  public function getTypeConstant() {
+    return self::TYPECONST;
+  }
+
+  public function getTypeName() {
+    return pht('Image');
+  }
+
+  public function newObject() {
+    return new PholioImage();
+  }
+
+  public function loadObjects(
+    PhabricatorObjectQuery $query,
+    array $phids) {
+
+    return id(new PholioImageQuery())
+      ->setViewer($query->getViewer())
+      ->withPHIDs($phids)
+      ->execute();
+  }
+
+  public function loadHandles(
+    PhabricatorHandleQuery $query,
+    array $handles,
+    array $objects) {
+
+    foreach ($handles as $phid => $handle) {
+      $image = $objects[$phid];
+
+      $id = $image->getID();
+      $mock_id = $image->getMockID();
+      $name = $image->getName();
+
+      $handle->setURI("/M{$mock_id}/{$id}/");
+      $handle->setName($name);
+      $handle->setFullName($name);
+    }
+  }
+
+}

src/applications/pholio/query/PholioImageQuery.php

@@ -0,0 +1,148 @@
+<?php
+
+/**
+ * @group pholio
+ */
+final class PholioImageQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $ids;
+  private $phids;
+  private $mockIDs;
+  private $obsolete;
+
+  private $needInlineComments;
+  private $mockCache = array();
+
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
+  public function withMockIDs(array $mock_ids) {
+    $this->mockIDs = $mock_ids;
+    return $this;
+  }
+
+  public function withObsolete($obsolete) {
+    $this->obsolete = $obsolete;
+    return $this;
+  }
+
+  public function needInlineComments($need_inline_comments) {
+    $this->needInlineComments = $need_inline_comments;
+    return $this;
+  }
+
+  public function setMockCache($mock_cache) {
+    $this->mockCache = $mock_cache;
+    return $this;
+  }
+  public function getMockCache() {
+    return $this->mockCache;
+  }
+
+  protected function loadPage() {
+    $table = new PholioImage();
+    $conn_r = $table->establishConnection('r');
+
+    $data = queryfx_all(
+      $conn_r,
+      'SELECT * FROM %T %Q %Q %Q',
+      $table->getTableName(),
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));
+
+    $images = $table->loadAllFromArray($data);
+
+    return $images;
+  }
+
+  private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+    $where = array();
+
+    $where[] = $this->buildPagingClause($conn_r);
+
+    if ($this->ids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'id IN (%Ld)',
+        $this->ids);
+    }
+
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
+    if ($this->mockIDs) {
+      $where[] = qsprintf(
+        $conn_r,
+        'mockID IN (%Ld)',
+        $this->mockIDs);
+    }
+
+    if ($this->obsolete !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'isObsolete = %d',
+        $this->obsolete);
+    }
+
+    return $this->formatWhereClause($where);
+  }
+
+  protected function willFilterPage(array $images) {
+    assert_instances_of($images, 'PholioImage');
+
+    $file_phids = mpull($images, 'getFilePHID');
+    $all_files = mpull(id(new PhabricatorFile())->loadAllWhere(
+      'phid IN (%Ls)',
+      $file_phids), null, 'getPHID');
+
+    if ($this->needInlineComments) {
+      $all_inline_comments = id(new PholioTransactionComment())
+        ->loadAllWhere('imageid IN (%Ld)',
+          mpull($images, 'getID'));
+      $all_inline_comments = mgroup($all_inline_comments, 'getImageID');
+    }
+
+    foreach ($images as $image) {
+      $file = idx($all_files, $image->getFilePHID());
+      if (!$file) {
+        $file = PhabricatorFile::loadBuiltin($this->getViewer(), 'missing.png');
+      }
+      $image->attachFile($file);
+      if ($this->needInlineComments) {
+        $inlines = idx($all_inline_comments, $image->getID(), array());
+        $image->attachInlineComments($inlines);
+      }
+    }
+
+    if ($this->getMockCache()) {
+      $mocks = $this->getMockCache();
+    } else {
+      $mock_ids = mpull($images, 'getMockID');
+      // DO NOT set needImages to true; recursion results!
+      $mocks = id(new PholioMockQuery())
+        ->setViewer($this->getViewer())
+        ->withIDs($mock_ids)
+        ->execute();
+      $mocks = mpull($mocks, null, 'getID');
+    }
+    foreach ($images as $image) {
+      $image->attachMock($mocks[$image->getMockID()]);
+    }
+
+    return $images;
+  }
+
+}

src/applications/pholio/query/PholioMockQuery.php

@@ -111,41 +111,20 @@ final class PholioMockQuery
   private function loadImages(array $mocks) {
     assert_instances_of($mocks, 'PholioMock');
 
-    $mock_ids = mpull($mocks, 'getID');
+    $mock_map = mpull($mocks, null, 'getID');
-    $all_images = id(new PholioImage())->loadAllWhere(
+    $all_images = id(new PholioImageQuery())
-      'mockID IN (%Ld) AND isObsolete = %d',
+      ->setViewer($this->getViewer())
-      $mock_ids,
+      ->setMockCache($mock_map)
-      0);
+      ->withMockIDs(array_keys($mock_map))
-
+      ->needInlineComments($this->needInlineComments)
-    $file_phids = mpull($all_images, 'getFilePHID');
+      ->execute();
-    $all_files = mpull(id(new PhabricatorFile())->loadAllWhere(
-      'phid IN (%Ls)',
-      $file_phids), null, 'getPHID');
-
-    if ($this->needInlineComments) {
-      $all_inline_comments = id(new PholioTransactionComment())
-        ->loadAllWhere('imageid IN (%Ld)',
-          mpull($all_images, 'getID'));
-      $all_inline_comments = mgroup($all_inline_comments, 'getImageID');
-    }
-
-    foreach ($all_images as $image) {
-      $file = idx($all_files, $image->getFilePHID());
-      if (!$file) {
-        $file = PhabricatorFile::loadBuiltin($this->getViewer(), 'missing.png');
-      }
-      $image->attachFile($file);
-      if ($this->needInlineComments) {
-        $inlines = idx($all_images, $image->getID(), array());
-        $image->attachInlineComments($inlines);
-      }
-    }
 
     $image_groups = mgroup($all_images, 'getMockID');
 
     foreach ($mocks as $mock) {
       $mock_images = $image_groups[$mock->getID()];
-      $mock->attachImages($mock_images);
+      $mock->attachAllImages($mock_images);
+      $mock->attachImages(mfilter($mock_images, 'getIsObsolete', true));
     }
   }
 

src/applications/pholio/storage/PholioImage.php

@@ -4,7 +4,9 @@
  * @group pholio
  */
 final class PholioImage extends PholioDAO
-  implements PhabricatorMarkupInterface {
+  implements
+    PhabricatorMarkupInterface,
+    PhabricatorPolicyInterface {
 
   const MARKUP_FIELD_DESCRIPTION  = 'markup:description';
 
@@ -14,9 +16,11 @@ final class PholioImage extends PholioDAO
   protected $description = '';
   protected $sequence;
   protected $isObsolete = 0;
+  protected $replacesImagePHID = null;
 
-  private $inlineComments;
+  private $inlineComments = self::ATTACHABLE;
-  private $file;
+  private $file = self::ATTACHABLE;
+  private $mock = self::ATTACHABLE;
 
   public function getConfiguration() {
     return array(
@@ -25,9 +29,30 @@ final class PholioImage extends PholioDAO
   }
 
   public function generatePHID() {
-    return PhabricatorPHID::generateNewPHID('PIMG');
+    return PhabricatorPHID::generateNewPHID(PholioPHIDTypeImage::TYPECONST);
   }
 
+  public function attachFile(PhabricatorFile $file) {
+    $this->file = $file;
+    return $this;
+  }
+
+  public function getFile() {
+    $this->assertAttached($this->file);
+    return $this->file;
+  }
+
+  public function attachMock(PholioMock $mock) {
+    $this->mock = $mock;
+    return $this;
+  }
+
+  public function getMock() {
+    $this->assertAttached($this->mock);
+    return $this->mock;
+  }
+
+
   public function attachInlineComments(array $inline_comments) {
     assert_instances_of($inline_comments, 'PholioTransactionComment');
     $this->inlineComments = $inline_comments;
@@ -35,9 +60,7 @@ final class PholioImage extends PholioDAO
   }
 
   public function getInlineComments() {
-    if ($this->inlineComments === null) {
+    $this->assertAttached($this->inlineComments);
-      throw new Exception("Call attachImages() before getImages()!");
-    }
     return $this->inlineComments;
   }
 
@@ -66,16 +89,19 @@ final class PholioImage extends PholioDAO
     return (bool)$this->getID();
   }
 
-  public function attachFile(PhabricatorFile $file) {
+/* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */
-    $this->file = $file;
+
-    return $this;
+  public function getCapabilities() {
+    return $this->getMock()->getCapabilities();
   }
 
-  public function getFile() {
+  public function getPolicy($capability) {
-    if ($this->file === null) {
+    return $this->getMock()->getPolicy($capability);
-      throw new Exception("Call attachFile() before getFile()!");
   }
-    return $this->file;
+
+  // really the *mock* controls who can see an image
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return $this->getMock()->hasAutomaticCapability($capability, $viewer);
   }
 
 }

src/applications/pholio/storage/PholioMock.php

@@ -22,9 +22,10 @@ final class PholioMock extends PholioDAO
   protected $coverPHID;
   protected $mailKey;
 
-  private $images;
+  private $images = self::ATTACHABLE;
-  private $coverFile;
+  private $allImages = self::ATTACHABLE;
-  private $tokenCount;
+  private $coverFile = self::ATTACHABLE;
+  private $tokenCount = self::ATTACHABLE;
 
   public function getConfiguration() {
     return array(
@@ -43,6 +44,9 @@ final class PholioMock extends PholioDAO
     return parent::save();
   }
 
+  /**
+   * These should be the images currently associated with the Mock.
+   */
   public function attachImages(array $images) {
     assert_instances_of($images, 'PholioImage');
     $this->images = $images;
@@ -50,28 +54,37 @@ final class PholioMock extends PholioDAO
   }
 
   public function getImages() {
-    if ($this->images === null) {
+    $this->assertAttached($this->images);
-      throw new Exception("Call attachImages() before getImages()!");
-    }
     return $this->images;
   }
 
+  /**
+   * These should be *all* images associated with the Mock. This includes
+   * images which have been removed and / or replaced from the Mock.
+   */
+  public function attachAllImages(array $images) {
+    assert_instances_of($images, 'PholioImage');
+    $this->allImages = $images;
+    return $this;
+  }
+
+  public function getAllImages() {
+    $this->assertAttached($this->images);
+    return $this->allImages;
+  }
+
   public function attachCoverFile(PhabricatorFile $file) {
     $this->coverFile = $file;
     return $this;
   }
 
   public function getCoverFile() {
-    if ($this->coverFile === null) {
+    $this->assertAttached($this->coverFile);
-      throw new Exception("Call attachCoverFile() before getCoverFile()!");
-    }
     return $this->coverFile;
   }
 
   public function getTokenCount() {
-    if ($this->tokenCount === null) {
+    $this->assertAttached($this->tokenCount);
-      throw new Exception("Call attachTokenCount() before getTokenCount()!");
-    }
     return $this->tokenCount;
   }
 
@@ -80,6 +93,30 @@ final class PholioMock extends PholioDAO
     return $this;
   }
 
+  public function getImageHistorySet($image_id) {
+    $images = $this->getAllImages();
+    $images = mpull($images, null, 'getID');
+    $selected_image = $images[$image_id];
+
+    $replace_map = mpull($images, null, 'getReplacesImagePHID');
+    $phid_map = mpull($images, null, 'getPHID');
+
+    // find the earliest image
+    $image = $selected_image;
+    while (isset($phid_map[$image->getReplacesImagePHID()])) {
+      $image = $phid_map[$image->getReplacesImagePHID()];
+    }
+
+    // now build history moving forward
+    $history = array($image->getID() => $image);
+    while (isset($replace_map[$image->getPHID()])) {
+      $image = $replace_map[$image->getPHID()];
+      $history[$image->getID()] = $image;
+    }
+
+    return $history;
+  }
+
 
 /* -(  PhabricatorSubscribableInterface Implementation  )-------------------- */
 

src/applications/pholio/storage/PholioTransaction.php

@@ -36,6 +36,10 @@ final class PholioTransaction extends PhabricatorApplicationTransaction {
       case PholioTransactionType::TYPE_IMAGE_FILE:
         $phids = array_merge($phids, $new, $old);
         break;
+      case PholioTransactionType::TYPE_IMAGE_REPLACE:
+        $phids[] = $new;
+        $phids[] = $old;
+        break;
       case PholioTransactionType::TYPE_IMAGE_DESCRIPTION:
       case PholioTransactionType::TYPE_IMAGE_NAME:
         $phids[] = key($new);
@@ -69,6 +73,7 @@ final class PholioTransaction extends PhabricatorApplicationTransaction {
       case PholioTransactionType::TYPE_IMAGE_DESCRIPTION:
         return 'edit';
       case PholioTransactionType::TYPE_IMAGE_FILE:
+      case PholioTransactionType::TYPE_IMAGE_REPLACE:
         return 'attach';
     }
 
@@ -115,6 +120,13 @@ final class PholioTransaction extends PhabricatorApplicationTransaction {
           $this->renderHandleLink($author_phid),
           $count);
         break;
+      case PholioTransactionType::TYPE_IMAGE_REPLACE:
+        return pht(
+          '%s replaced %s with %s.',
+          $this->renderHandleLink($author_phid),
+          $this->renderHandleLink($old),
+          $this->renderHandleLink($new));
+        break;
       case PholioTransactionType::TYPE_IMAGE_FILE:
         $add = array_diff($new, $old);
         $rem = array_diff($old, $new);
@@ -197,6 +209,7 @@ final class PholioTransaction extends PhabricatorApplicationTransaction {
           $this->renderHandleLink($author_phid),
           $this->renderHandleLink($object_phid));
         break;
+      case PholioTransactionType::TYPE_IMAGE_REPLACE:
       case PholioTransactionType::TYPE_IMAGE_FILE:
         return pht(
           '%s updated images of %s.',
@@ -259,6 +272,8 @@ final class PholioTransaction extends PhabricatorApplicationTransaction {
       case PholioTransactionType::TYPE_IMAGE_NAME:
       case PholioTransactionType::TYPE_IMAGE_DESCRIPTION:
         return PhabricatorTransactions::COLOR_BLUE;
+      case PholioTransactionType::TYPE_IMAGE_REPLACE:
+        return PhabricatorTransactions::COLOR_YELLOW;
       case PholioTransactionType::TYPE_IMAGE_FILE:
         $add = array_diff($new, $old);
         $rem = array_diff($old, $new);

src/applications/pholio/view/PholioMockImagesView.php

@@ -70,6 +70,7 @@ final class PholioMockImagesView extends AphrontView {
         'id' => $image->getID(),
         'fullURI' => $image->getFile()->getBestURI(),
         'pageURI' => '/M'.$mock->getID().'/'.$image->getID().'/',
+        'historyURI' => '/pholio/image/history/'.$image->getID().'/',
         'width' => $x,
         'height' => $y,
         'title' => $image->getName(),

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

@@ -1475,6 +1475,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
         'type' => 'sql',
         'name' => $this->getPatchPath('20130723.taskstarttime.sql'),
       ),
+      '20130722.pholioreplace.sql' => array(
+        'type' => 'sql',
+        'name' => $this->getPatchPath('20130722.pholioreplace.sql'),
+      ),
     );
   }
 }

webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js

@@ -728,6 +728,13 @@ JX.behavior('pholio-mock-view', function(config) {
       'View Full Image');
     info.push(full_link);
 
+    var history_link = JX.$N(
+      'a',
+      { href: image.historyURI },
+      'View Image History');
+    info.push(history_link);
+
+
     for (var ii = 0; ii < info.length; ii++) {
       info[ii] = JX.$N('div', {className: 'pholio-image-info-item'}, info[ii]);
     }