Make project profile picture edits transactional

Summary: Ref T4379. Make changing pictures use apptransactions.

Test Plan: {F111554}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4379

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

src/applications/project/controller/PhabricatorProjectProfilePictureController.php

@@ -75,12 +75,24 @@ final class PhabricatorProjectProfilePictureController
 
       if (!$errors) {
         if ($is_default) {
-          $project->setProfileImagePHID(null);
+          $new_value = null;
         } else {
-          $project->setProfileImagePHID($xformed->getPHID());
+          $new_value = $xformed->getPHID();
-          $xformed->attachToObject($viewer, $project->getPHID());
         }
-        $project->save();
+
+        $xactions = array();
+        $xactions[] = id(new PhabricatorProjectTransaction())
+          ->setTransactionType(PhabricatorProjectTransaction::TYPE_IMAGE)
+          ->setNewValue($new_value);
+
+        $editor = id(new PhabricatorProjectTransactionEditor())
+          ->setActor($viewer)
+          ->setContentSourceFromRequest($request)
+          ->setContinueOnMissingFields(true)
+          ->setContinueOnNoEffect(true);
+
+        $editor->applyTransactions($project, $xactions);
+
         return id(new AphrontRedirectResponse())->setURI($project_uri);
       }
     }

src/applications/project/editor/PhabricatorProjectTransactionEditor.php

@@ -13,6 +13,7 @@ final class PhabricatorProjectTransactionEditor
 
     $types[] = PhabricatorProjectTransaction::TYPE_NAME;
     $types[] = PhabricatorProjectTransaction::TYPE_STATUS;
+    $types[] = PhabricatorProjectTransaction::TYPE_IMAGE;
 
     return $types;
   }
@@ -26,6 +27,8 @@ final class PhabricatorProjectTransactionEditor
         return $object->getName();
       case PhabricatorProjectTransaction::TYPE_STATUS:
         return $object->getStatus();
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
+        return $object->getProfileImagePHID();
     }
 
     return parent::getCustomTransactionOldValue($object, $xaction);
@@ -38,6 +41,7 @@ final class PhabricatorProjectTransactionEditor
     switch ($xaction->getTransactionType()) {
       case PhabricatorProjectTransaction::TYPE_NAME:
       case PhabricatorProjectTransaction::TYPE_STATUS:
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
         return $xaction->getNewValue();
     }
 
@@ -55,6 +59,9 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_STATUS:
         $object->setStatus($xaction->getNewValue());
         return;
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
+        $object->setProfileImagePHID($xaction->getNewValue());
+        return;
       case PhabricatorTransactions::TYPE_EDGE:
         return;
       case PhabricatorTransactions::TYPE_VIEW_POLICY:
@@ -113,6 +120,7 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorTransactions::TYPE_JOIN_POLICY:
       case PhabricatorTransactions::TYPE_EDGE:
       case PhabricatorProjectTransaction::TYPE_STATUS:
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
         return;
     }
 
@@ -156,6 +164,7 @@ final class PhabricatorProjectTransactionEditor
     switch ($xaction->getTransactionType()) {
       case PhabricatorProjectTransaction::TYPE_NAME:
       case PhabricatorProjectTransaction::TYPE_STATUS:
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
         PhabricatorPolicyFilter::requireCapability(
           $this->requireActor(),
           $object,
@@ -202,4 +211,20 @@ final class PhabricatorProjectTransactionEditor
     return true;
   }
 
+  protected function extractFilePHIDsFromCustomTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
+        $new = $xaction->getNewValue();
+        if ($new) {
+          return array($new);
+        }
+        break;
+    }
+
+    return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
+  }
+
 }

src/applications/project/storage/PhabricatorProjectTransaction.php

@@ -4,8 +4,11 @@ final class PhabricatorProjectTransaction
   extends PhabricatorApplicationTransaction {
 
   const TYPE_NAME       = 'project:name';
-  const TYPE_MEMBERS    = 'project:members';
   const TYPE_STATUS     = 'project:status';
+  const TYPE_IMAGE      = 'project:image';
+
+  // NOTE: This is deprecated, members are just a normal edge now.
+  const TYPE_MEMBERS    = 'project:members';
 
   public function getApplicationName() {
     return 'project';
@@ -26,6 +29,10 @@ final class PhabricatorProjectTransaction
         $rem = array_diff($old, $new);
         $req_phids = array_merge($add, $rem);
         break;
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
+        $req_phids[] = $old;
+        $req_phids[] = $new;
+        break;
     }
 
     return array_merge($req_phids, parent::getRequiredHandlePHIDs());
@@ -59,6 +66,24 @@ final class PhabricatorProjectTransaction
             '%s reopened this project.',
             $author_handle);
         }
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
+        // TODO: Some day, it would be nice to show the images.
+        if (!$old) {
+          return pht(
+            '%s set this project\'s image to %s.',
+            $author_handle,
+            $this->renderHandleLink($new));
+        } else if (!$new) {
+          return pht(
+            '%s removed this project\'s image.',
+            $author_handle);
+        } else {
+          return pht(
+            '%s updated this project\'s image from %s to %s.',
+            $author_handle,
+            $this->renderHandleLink($old),
+            $this->renderHandleLink($new));
+        }
       case PhabricatorProjectTransaction::TYPE_MEMBERS:
         $add = array_diff($new, $old);
         $rem = array_diff($old, $new);