Make Pholio mock edits (mostly) transaction-oriented

Summary:
  - Use transactions to apply edits.
  - Use Editor to apply transactions.
  - Some special casing for tricky stuff I don't want to deal with yet (mock images).

Test Plan: {F22368}

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2097

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

src/__phutil_library_map__.php

@@ -1203,6 +1203,7 @@ phutil_register_library_map(array(
     'PhamePostViewController' => 'applications/phame/controller/post/PhamePostViewController.php',
     'PhameResourceController' => 'applications/phame/controller/PhameResourceController.php',
     'PhameSkinSpecification' => 'applications/phame/skins/PhameSkinSpecification.php',
+    'PholioConstants' => 'applications/pholio/constants/PholioConstants.php',
     'PholioController' => 'applications/pholio/controller/PholioController.php',
     'PholioDAO' => 'applications/pholio/storage/PholioDAO.php',
     'PholioImage' => 'applications/pholio/storage/PholioImage.php',
@@ -1217,6 +1218,7 @@ phutil_register_library_map(array(
     'PholioPixelComment' => 'applications/pholio/storage/PholioPixelComment.php',
     'PholioTransaction' => 'applications/pholio/storage/PholioTransaction.php',
     'PholioTransactionQuery' => 'applications/pholio/query/PholioTransactionQuery.php',
+    'PholioTransactionType' => 'applications/pholio/constants/PholioTransactionType.php',
     'PhortuneMonthYearExpiryControl' => 'applications/phortune/control/PhortuneMonthYearExpiryControl.php',
     'PhortuneStripeBaseController' => 'applications/phortune/stripe/controller/PhortuneStripeBaseController.php',
     'PhortuneStripePaymentFormView' => 'applications/phortune/stripe/view/PhortuneStripePaymentFormView.php',
@@ -2431,6 +2433,7 @@ phutil_register_library_map(array(
       1 => 'PhabricatorMarkupInterface',
     ),
     'PholioTransactionQuery' => 'PhabricatorOffsetPagedQuery',
+    'PholioTransactionType' => 'PholioConstants',
     'PhortuneMonthYearExpiryControl' => 'AphrontFormControl',
     'PhortuneStripeBaseController' => 'PhabricatorController',
     'PhortuneStripePaymentFormView' => 'AphrontView',

src/applications/pholio/constants/PholioConstants.php

@@ -0,0 +1,21 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+abstract class PholioConstants {
+
+}

src/applications/pholio/constants/PholioTransactionType.php

@@ -0,0 +1,26 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class PholioTransactionType extends PholioConstants {
+
+  const TYPE_NONE         = 'none';
+  const TYPE_NAME         = 'name';
+  const TYPE_DESCRIPTION  = 'description';
+  const TYPE_VIEW_POLICY  = 'viewPolicy';
+
+}

src/applications/pholio/controller/PholioMockCommentController.php

@@ -53,24 +53,20 @@ final class PholioMockCommentController extends PholioController {
       return id(new AphrontDialogResponse())->setDialog($dialog);
     }
 
-
     $content_source = PhabricatorContentSource::newForSource(
       PhabricatorContentSource::SOURCE_WEB,
       array(
         'ip' => $request->getRemoteAddr(),
       ));
 
-    // TODO: Move this to an Editor.
-
     $xaction = id(new PholioTransaction())
-      // TODO: Formalize transaction types.
-      ->setTransactionType('none')
-      ->setAuthorPHID($user->getPHID())
-      ->setComment($comment)
-      ->setContentSource($content_source)
-      ->setMockID($mock->getID());
+      ->setTransactionType(PholioTransactionType::TYPE_NONE)
+      ->setComment($comment);
 
-    $xaction->save();
+    id(new PholioMockEditor())
+      ->setActor($user)
+      ->setContentSource($content_source)
+      ->applyTransactions($mock, array($xaction));
 
     return id(new AphrontRedirectResponse())->setURI($mock_uri);
   }

src/applications/pholio/controller/PholioMockEditController.php

@@ -33,68 +33,119 @@ final class PholioMockEditController extends PholioController {
 
 
     if ($this->id) {
-      // TODO: Support!
-      throw new Exception("NOT SUPPORTED YET");
+      $mock = id(new PholioMockQuery())
+        ->setViewer($user)
+        ->requireCapabilities(
+          array(
+            PhabricatorPolicyCapability::CAN_VIEW,
+            PhabricatorPolicyCapability::CAN_EDIT,
+          ))
+        ->withIDs(array($this->id))
+        ->executeOne();
+
+      if (!$mock) {
+        return new Aphront404Response();
+      }
+
+      $title = pht('Edit Mock');
+
+      $is_new = false;
     } else {
       $mock = new PholioMock();
       $mock->setAuthorPHID($user->getPHID());
       $mock->setViewPolicy(PhabricatorPolicies::POLICY_USER);
 
       $title = pht('Create Mock');
+
+      $is_new = true;
     }
 
     $e_name = true;
     $e_images = true;
     $errors = array();
 
-    if ($request->isFormPost()) {
-      $mock->setName($request->getStr('name'));
-      $mock->setDescription($request->getStr('description'));
-      $mock->setViewPolicy($request->getStr('can_view'));
+    $v_name = $mock->getName();
+    $v_desc = $mock->getDescription();
+    $v_view = $mock->getViewPolicy();
 
-      if (!strlen($mock->getName())) {
+    if ($request->isFormPost()) {
+      $xactions = array();
+
+      $type_name = PholioTransactionType::TYPE_NAME;
+      $type_desc = PholioTransactionType::TYPE_DESCRIPTION;
+      $type_view = PholioTransactionType::TYPE_VIEW_POLICY;
+
+      $v_name = $request->getStr('name');
+      $v_desc = $request->getStr('description');
+      $v_view = $request->getStr('can_view');
+
+      $xactions[$type_name] = $v_name;
+      $xactions[$type_desc] = $v_desc;
+      $xactions[$type_view] = $v_view;
+
+      if (!strlen($request->getStr('name'))) {
         $e_name = 'Required';
         $errors[] = pht('You must name the mock.');
       }
 
-      $files = array();
-
-      $file_phids = $request->getArr('file_phids');
-      if ($file_phids) {
-        $files = id(new PhabricatorFileQuery())
-          ->setViewer($user)
-          ->withPHIDs($file_phids)
-          ->execute();
-      }
-
-      if (!$files) {
-        $e_images = 'Required';
-        $errors[] = pht('You must add at least one image to the mock.');
-      } else {
-        $mock->setCoverPHID(head($files)->getPHID());
-      }
-
-      $sequence = 0;
-
       $images = array();
-      foreach ($files as $file) {
-        $image = new PholioImage();
-        $image->setName('');
-        $image->setDescription('');
-        $image->setFilePHID($file->getPHID());
-        $image->setSequence($sequence++);
+      if ($is_new) {
+        // TODO: Make this transactional and allow edits?
 
-        $images[] = $image;
+        $files = array();
+
+        $file_phids = $request->getArr('file_phids');
+        if ($file_phids) {
+          $files = id(new PhabricatorFileQuery())
+            ->setViewer($user)
+            ->withPHIDs($file_phids)
+            ->execute();
+        }
+
+        if (!$files) {
+          $e_images = 'Required';
+          $errors[] = pht('You must add at least one image to the mock.');
+        } else {
+          $mock->setCoverPHID(head($files)->getPHID());
+        }
+
+        $sequence = 0;
+
+        foreach ($files as $file) {
+          $image = new PholioImage();
+          $image->setFilePHID($file->getPHID());
+          $image->setSequence($sequence++);
+
+          $images[] = $image;
+        }
       }
 
-
       if (!$errors) {
-        // TODO: Make transaction-object based and move to editor.
+        $content_source = PhabricatorContentSource::newForSource(
+          PhabricatorContentSource::SOURCE_WEB,
+          array(
+            'ip' => $request->getRemoteAddr(),
+          ));
+
+        foreach ($xactions as $type => $value) {
+          $xactions[$type] = id(new PholioTransaction())
+            ->setTransactionType($type)
+            ->setNewValue($value);
+        }
+
         $mock->openTransaction();
-          $mock->save();
-          foreach ($images as $image) {
-            $image->setMockID($mock->getID());
-            $image->save();
+          $editor = id(new PholioMockEditor())
+            ->setContentSource($content_source)
+            ->setActor($user);
+
+          $editor->applyTransactions($mock, $xactions);
+
+          if ($images) {
+            foreach ($images as $image) {
+              // TODO: Move into editor?
+              $image->setMockID($mock->getID());
+              $image->save();
+            }
           }
         $mock->saveTransaction();
 
@@ -126,19 +177,22 @@ final class PholioMockEditController extends PholioController {
       ->setObject($mock)
       ->execute();
 
+    // NOTE: Make this show up correctly on the rendered form.
+    $mock->setViewPolicy($v_view);
+
     $form = id(new AphrontFormView())
       ->setUser($user)
       ->setFlexible(true)
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setName('name')
-          ->setValue($mock->getName())
+          ->setValue($v_name)
           ->setLabel(pht('Name'))
           ->setError($e_name))
       ->appendChild(
         id(new PhabricatorRemarkupControl())
           ->setName('description')
-          ->setValue($mock->getDescription())
+          ->setValue($v_desc)
           ->setLabel(pht('Description')))
       ->appendChild(
         id(new AphrontFormDragAndDropUploadControl($request))

src/applications/pholio/controller/PholioMockViewController.php

@@ -196,14 +196,76 @@ final class PholioMockViewController extends PholioController {
     foreach ($xactions as $xaction) {
       $author = $this->getHandle($xaction->getAuthorPHID());
 
-      $view->addEvent(
-        id(new PhabricatorTimelineEventView())
-          ->setUserHandle($author)
-          ->setTitle($author->renderLink().' added a comment.')
-          ->appendChild(
-            $engine->getOutput(
-              $xaction,
-              PholioTransaction::MARKUP_FIELD_COMMENT)));
+      $old = $xaction->getOldValue();
+      $new = $xaction->getNewValue();
+
+      $xaction_visible = true;
+      $title = null;
+
+      switch ($xaction->getTransactionType()) {
+        case PholioTransactionType::TYPE_NONE:
+          $title = pht(
+            '%s added a comment.',
+            $author->renderLink());
+          break;
+        case PholioTransactionType::TYPE_NAME:
+          if ($old === null) {
+            $xaction_visible = false;
+            break;
+          }
+          $title = pht(
+            '%s renamed this mock from "%s" to "%s".',
+            $author->renderLink(),
+            phutil_escape_html($old),
+            phutil_escape_html($new));
+          break;
+        case PholioTransactionType::TYPE_DESCRIPTION:
+          if ($old === null) {
+            $xaction_visible = false;
+            break;
+          }
+          // TODO: Show diff, like Maniphest.
+          $title = pht(
+            '%s updated the description of this mock. '.
+            'The old description was: %s',
+            $author->renderLink(),
+            phutil_escape_html($old));
+          break;
+        case PholioTransactionType::TYPE_VIEW_POLICY:
+          if ($old === null) {
+            $xaction_visible = false;
+            break;
+          }
+          // TODO: Render human-readable.
+          $title = pht(
+            '%s changed the visibility of this mock from "%s" to "%s".',
+            $author->renderLink(),
+            phutil_escape_html($old),
+            phutil_escape_html($new));
+          break;
+        default:
+          throw new Exception("Unknown transaction type '{$type}'!");
+      }
+
+      if (!$xaction_visible) {
+        // Some transactions aren't useful to human viewers, like
+        // the initial transactions which set the mock's name and description.
+        continue;
+      }
+
+      $event = id(new PhabricatorTimelineEventView())
+        ->setUserHandle($author);
+
+      $event->setTitle($title);
+
+      if (strlen($xaction->getComment())) {
+        $event->appendChild(
+          $engine->getOutput(
+            $xaction,
+            PholioTransaction::MARKUP_FIELD_COMMENT));
+      }
+
+      $view->addEvent($event);
     }
 
     return $view;

src/applications/pholio/editor/PholioMockEditor.php

@@ -21,6 +21,128 @@
  */
 final class PholioMockEditor extends PhabricatorEditor {
 
+  private $contentSource;
 
+  public function setContentSource(PhabricatorContentSource $content_source) {
+    $this->contentSource = $content_source;
+    return $this;
+  }
+
+  public function getContentSource() {
+    return $this->contentSource;
+  }
+
+  public function applyTransactions(PholioMock $mock, array $xactions) {
+    assert_instances_of($xactions, 'PholioTransaction');
+
+    $actor = $this->requireActor();
+    if (!$this->contentSource) {
+      throw new Exception(
+        "Call setContentSource() before applyTransactions()!");
+    }
+
+    foreach ($xactions as $xaction) {
+      $xaction->setContentSource($this->contentSource);
+      $xaction->setAuthorPHID($actor->getPHID());
+    }
+
+    foreach ($xactions as $key => $xaction) {
+      $has_effect = $this->applyTransaction($mock, $xaction);
+      if (!$has_effect) {
+        unset($xactions[$key]);
+      }
+    }
+
+    if (!$xactions) {
+      return;
+    }
+
+    $mock->openTransaction();
+      $mock->save();
+
+      foreach ($xactions as $xaction) {
+        $xaction->setMockID($mock->getID());
+        $xaction->save();
+      }
+    $mock->saveTransaction();
+
+    PholioIndexer::indexMock($mock);
+
+    return $this;
+  }
+
+  private function applyTransaction(
+    PholioMock $mock,
+    PholioTransaction $xaction) {
+
+    $type = $xaction->getTransactionType();
+
+    $old = null;
+    switch ($type) {
+      case PholioTransactionType::TYPE_NONE:
+        $old = null;
+        break;
+      case PholioTransactionType::TYPE_NAME:
+        $old = $mock->getName();
+        break;
+      case PholioTransactionType::TYPE_DESCRIPTION:
+        $old = $mock->getDescription();
+        break;
+      case PholioTransactionType::TYPE_VIEW_POLICY:
+        $old = $mock->getViewPolicy();
+        break;
+      default:
+        throw new Exception("Unknown transaction type '{$type}'!");
+    }
+
+    $xaction->setOldValue($old);
+
+    if (!$this->transactionHasEffect($xaction)) {
+      return false;
+    }
+
+    switch ($type) {
+      case PholioTransactionType::TYPE_NONE:
+        break;
+      case PholioTransactionType::TYPE_NAME:
+        $mock->setName($xaction->getNewValue());
+        break;
+      case PholioTransactionType::TYPE_DESCRIPTION:
+        $mock->setDescription($xaction->getNewValue());
+        break;
+      case PholioTransactionType::TYPE_VIEW_POLICY:
+        $mock->setViewPolicy($xaction->getNewValue());
+        break;
+      default:
+        throw new Exception("Unknown transaction type '{$type}'!");
+    }
+
+    return true;
+  }
+
+  private function transactionHasEffect(PholioTransaction $xaction) {
+    $effect = false;
+
+    $type = $xaction->getTransactionType();
+    switch ($type) {
+      case PholioTransactionType::TYPE_NONE:
+      case PholioTransactionType::TYPE_NAME:
+      case PholioTransactionType::TYPE_DESCRIPTION:
+      case PholioTransactionType::TYPE_VIEW_POLICY:
+        $effect = ($xaction->getOldValue() !== $xaction->getNewValue());
+        break;
+      default:
+        throw new Exception("Unknown transaction type '{$type}'!");
+    }
+
+    if (!$effect) {
+      if (strlen($xaction->getComment())) {
+        $xaction->setTransactionType(PholioTransactionType::TYPE_NONE);
+        $effect = true;
+      }
+    }
+
+    return $effect;
+  }
 
 }

src/applications/pholio/storage/PholioImage.php

@@ -26,8 +26,8 @@ final class PholioImage extends PholioDAO
 
   protected $mockID;
   protected $filePHID;
-  protected $name;
-  protected $description;
+  protected $name = '';
+  protected $description = '';
   protected $sequence;
 
 

src/applications/pholio/storage/PholioTransaction.php

@@ -29,7 +29,7 @@ final class PholioTransaction extends PholioDAO
   protected $transactionType;
   protected $oldValue;
   protected $newValue;
-  protected $comment;
+  protected $comment = '';
   protected $metadata = array();
   protected $contentSource;