Make the Pholio Mock "getImages" / "getAllImages" API more clear

Summary:
Depends on D19920. Ref T11351. Currently, "images" and "all images" are attached to Mocks separately, and `getImages()` gets you only some images.

Clean this up slightly:

  - One attach method; attach everything.
  - Two getters, one for "images" (returns all images); one for "active images" (returns active images).

Test Plan: Browsed around Pholio without any apparent behavioral changes.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11351

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

src/applications/pholio/controller/PholioMockCommentController.php

@@ -24,7 +24,7 @@ final class PholioMockCommentController extends PholioController {
 
     $draft = PhabricatorDraft::buildFromRequest($request);
 
-    $mock_uri = '/M'.$mock->getID();
+    $mock_uri = $mock->getURI();
 
     $comment = $request->getStr('comment');
 
@@ -33,7 +33,7 @@ final class PholioMockCommentController extends PholioController {
     $inline_comments = id(new PholioTransactionComment())->loadAllWhere(
       'authorphid = %s AND transactionphid IS NULL AND imageid IN (%Ld)',
       $viewer->getPHID(),
-      mpull($mock->getImages(), 'getID'));
+      mpull($mock->getActiveImages(), 'getID'));
 
     if (!$inline_comments || strlen($comment)) {
       $xactions[] = id(new PholioTransaction())

src/applications/pholio/controller/PholioMockEditController.php

@@ -25,7 +25,7 @@ final class PholioMockEditController extends PholioController {
       $title = pht('Edit Mock: %s', $mock->getName());
 
       $is_new = false;
-      $mock_images = $mock->getImages();
+      $mock_images = $mock->getActiveImages();
       $files = mpull($mock_images, 'getFile');
       $mock_images = mpull($mock_images, null, 'getFilePHID');
     } else {

src/applications/pholio/query/PholioMockQuery.php

@@ -58,7 +58,7 @@ final class PholioMockQuery
   }
 
   protected function loadPage() {
-    $mocks = $this->loadStandardPage(new PholioMock());
+    $mocks = $this->loadStandardPage($this->newResultObject());
 
     if ($mocks && $this->needImages) {
       self::loadImages($this->getViewer(), $mocks, $this->needInlineComments);
@@ -127,9 +127,7 @@ final class PholioMockQuery
 
     foreach ($mocks as $mock) {
       $mock_images = idx($image_groups, $mock->getPHID(), array());
-      $mock->attachAllImages($mock_images);
-      $active_images = mfilter($mock_images, 'getIsObsolete', true);
-      $mock->attachImages(msort($active_images, 'getSequence'));
+      $mock->attachImages($mock_images);
     }
   }
 

src/applications/pholio/query/PholioMockSearchEngine.php

@@ -111,7 +111,7 @@ final class PholioMockSearchEngine extends PhabricatorApplicationSearchEngine {
         ->setImageURI($image_uri)
         ->setImageSize($x, $y)
         ->setDisabled($mock->isClosed())
-        ->addIconCount('fa-picture-o', count($mock->getImages()))
+        ->addIconCount('fa-picture-o', count($mock->getActiveImages()))
         ->addIconCount('fa-trophy', $mock->getTokenCount());
 
       if ($mock->getAuthorPHID()) {

src/applications/pholio/storage/PholioMock.php

@@ -30,7 +30,6 @@ final class PholioMock extends PholioDAO
   protected $spacePHID;
 
   private $images = self::ATTACHABLE;
-  private $allImages = self::ATTACHABLE;
   private $coverFile = self::ATTACHABLE;
   private $tokenCount = self::ATTACHABLE;
 
@@ -93,33 +92,28 @@ 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');
+    $images = mpull($images, null, 'getPHID');
+    $images = msort($images, 'getSequence');
     $this->images = $images;
     return $this;
   }
 
   public function getImages() {
-    $this->assertAttached($this->images);
-    return $this->images;
+    return $this->assertAttached($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 getActiveImages() {
+    $images = $this->getImages();
 
-  public function getAllImages() {
-    $this->assertAttached($this->images);
-    return $this->allImages;
+    foreach ($images as $phid => $image) {
+      if ($image->getIsObsolete()) {
+        unset($images[$phid]);
+      }
+    }
+
+    return $images;
   }
 
   public function attachCoverFile(PhabricatorFile $file) {
@@ -143,7 +137,7 @@ final class PholioMock extends PholioDAO
   }
 
   public function getImageHistorySet($image_id) {
-    $images = $this->getAllImages();
+    $images = $this->getImages();
     $images = mpull($images, null, 'getID');
     $selected_image = $images[$image_id];
 

src/applications/pholio/view/PholioMockEmbedView.php

@@ -25,7 +25,7 @@ final class PholioMockEmbedView extends AphrontView {
     $thumbnail = null;
     if (!empty($this->images)) {
       $images_to_show = array_intersect_key(
-        $this->mock->getImages(), array_flip($this->images));
+        $this->mock->getActiveImages(), array_flip($this->images));
     }
 
     $xform = PhabricatorFileTransform::getTransformByKey(
@@ -54,7 +54,7 @@ final class PholioMockEmbedView extends AphrontView {
       ->setImageURI($thumbnail)
       ->setImageSize($x, $y)
       ->setDisabled($mock->isClosed())
-      ->addIconCount('fa-picture-o', count($mock->getImages()))
+      ->addIconCount('fa-picture-o', count($mock->getActiveImages()))
       ->addIconCount('fa-trophy', $mock->getTokenCount());
 
     return $item;

src/applications/pholio/view/PholioMockImagesView.php

@@ -74,7 +74,7 @@ final class PholioMockImagesView extends AphrontView {
 
     $images = array();
     $current_set = 0;
-    foreach ($mock->getAllImages() as $image) {
+    foreach ($mock->getImages() as $image) {
       $file = $image->getFile();
       $metadata = $file->getMetadata();
       $x = idx($metadata, PhabricatorFile::METADATA_IMAGE_WIDTH);
@@ -110,7 +110,7 @@ final class PholioMockImagesView extends AphrontView {
       );
     }
 
-    $ids = mpull($mock->getImages(), 'getID');
+    $ids = mpull($mock->getActiveImages(), 'getID');
     if ($this->imageID && isset($ids[$this->imageID])) {
       $selected_id = $this->imageID;
     } else {
@@ -118,7 +118,7 @@ final class PholioMockImagesView extends AphrontView {
     }
 
     $navsequence = array();
-    foreach ($mock->getImages() as $image) {
+    foreach ($mock->getActiveImages() as $image) {
       $navsequence[] = $image->getID();
     }
 

src/applications/pholio/view/PholioMockThumbGridView.php

@@ -12,7 +12,7 @@ final class PholioMockThumbGridView extends AphrontView {
   public function render() {
     $mock = $this->mock;
 
-    $all_images = $mock->getAllImages();
+    $all_images = $mock->getImages();
     $all_images = mpull($all_images, null, 'getPHID');
 
     $history = mpull($all_images, 'getReplacesImagePHID', 'getPHID');
@@ -25,10 +25,10 @@ final class PholioMockThumbGridView extends AphrontView {
     }
 
     // Figure out the columns. Start with all the active images.
-    $images = mpull($mock->getImages(), null, 'getPHID');
+    $images = mpull($mock->getActiveImages(), null, 'getPHID');
 
     // Now, find deleted images: obsolete images which were not replaced.
-    foreach ($mock->getAllImages() as $image) {
+    foreach ($mock->getImages() as $image) {
       if (!$image->getIsObsolete()) {
         // Image is current.
         continue;

src/applications/pholio/view/PholioTransactionView.php

@@ -97,7 +97,7 @@ final class PholioTransactionView
   private function renderInlineContent(PholioTransaction $inline) {
     $comment = $inline->getComment();
     $mock = $this->getMock();
-    $images = $mock->getAllImages();
+    $images = $mock->getImages();
     $images = mpull($images, null, 'getID');
 
     $image = idx($images, $comment->getImageID());

src/applications/pholio/xaction/PholioImageFileTransaction.php

@@ -6,7 +6,7 @@ final class PholioImageFileTransaction
   const TRANSACTIONTYPE = 'image-file';
 
   public function generateOldValue($object) {
-    $images = $object->getImages();
+    $images = $object->getActiveImages();
     return array_values(mpull($images, 'getPHID'));
   }
 
@@ -24,7 +24,7 @@ final class PholioImageFileTransaction
     $new_map = array_fuse($this->getNewValue());
 
     $obsolete_map = array_diff_key($old_map, $new_map);
-    $images = $object->getImages();
+    $images = $object->getActiveImages();
     foreach ($images as $seq => $image) {
       if (isset($obsolete_map[$image->getPHID()])) {
         $image->setIsObsolete(1);