From 06de605992881269693cb74cf7c0777ecc9d6f64 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 16 Nov 2015 09:53:08 -0800
Subject: [PATCH] Extract PHIDs from transactions later, fixing Paste
 extraction/attachment

Summary:
Fixes T9787. Currently, file PHID extraction logic happens very early, before we normalize/merge/etc the transactions.

In D14390, I changed how the CONTENT transaction works: before, callers would pass in a file PHID. Afterward, they just pass in the content.

Passing in the content is generaly easier and feels more correct, but inadvertenly broke PHID extraction because converting the content into a file PHID now happened after we extracted the PHID. So we'd extract the entire text of the paste as a "file PHID", which wouldn't work.

Instead, extract file PHIDs later. This impacts a couple of other applications (Conpherence, Pholio) which receive an object or have an unusual file-oriented transaction.

Test Plan:
  - Made a new paste, verified the raw file attached to it properly.
  - Made and updated a mock, verified all the files attached properly.
  - Updated a Conpherence room image, verified the files attached properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9787

Differential Revision: https://secure.phabricator.com/D14494
---
 .../conpherence/editor/ConpherenceEditor.php  |  3 +--
 .../pholio/editor/PholioMockEditor.php        | 26 ++++++++++++++-----
 ...habricatorApplicationTransactionEditor.php |  3 +--
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php
index 299b799d3e..7d3aa02d2f 100644
--- a/src/applications/conpherence/editor/ConpherenceEditor.php
+++ b/src/applications/conpherence/editor/ConpherenceEditor.php
@@ -623,9 +623,8 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
 
     switch ($xaction->getTransactionType()) {
       case ConpherenceTransaction::TYPE_PICTURE:
-          return array($xaction->getNewValue()->getPHID());
       case ConpherenceTransaction::TYPE_PICTURE_CROP:
-          return array($xaction->getNewValue());
+        return array($xaction->getNewValue());
     }
 
     return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php
index 14d3a0af16..31d1602202 100644
--- a/src/applications/pholio/editor/PholioMockEditor.php
+++ b/src/applications/pholio/editor/PholioMockEditor.php
@@ -120,19 +120,31 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
 
+    $images = $this->getNewImages();
+    $images = mpull($images, null, 'getPHID');
+
     switch ($xaction->getTransactionType()) {
       case PholioTransaction::TYPE_IMAGE_FILE:
-        $new = $xaction->getNewValue();
-        $phids = array();
-        foreach ($new as $key => $images) {
-          $phids[] = mpull($images, 'getFilePHID');
+        $file_phids = array();
+        foreach ($xaction->getNewValue() as $image_phid) {
+          $image = idx($images, $image_phid);
+          if (!$image) {
+            continue;
+          }
+          $file_phids[] = $image->getFilePHID();
         }
-        return array_mergev($phids);
+        return $file_phids;
       case PholioTransaction::TYPE_IMAGE_REPLACE:
-        return array($xaction->getNewValue()->getFilePHID());
+        $image_phid = $xaction->getNewValue();
+        $image = idx($images, $image_phid);
+
+        if ($image) {
+          return array($image->getFilePHID());
+        }
+        break;
     }
 
-    return array();
+    return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
   }
 
 
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index 1f390ed468..d61b498edd 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -775,8 +775,6 @@ abstract class PhabricatorApplicationTransactionEditor
         throw new PhabricatorApplicationTransactionValidationException($errors);
       }
 
-      $file_phids = $this->extractFilePHIDs($object, $xactions);
-
       if ($object->getID()) {
         foreach ($xactions as $xaction) {
 
@@ -822,6 +820,7 @@ abstract class PhabricatorApplicationTransactionEditor
     }
 
     $xactions = $this->sortTransactions($xactions);
+    $file_phids = $this->extractFilePHIDs($object, $xactions);
 
     if ($is_preview) {
       $this->loadHandles($xactions);