Genericize transactions in Pholio

Summary:
Split Pholio's transaction implementation into generic and application-specific parts. Moves us toward generic transactions, with support for:

  - Editing and deleting comments.
  - Setting visibility of individual comments (I'm not a fan of this feature but we'll see).

I want to move everything to a more generic piece of infrastructure but there's very little they can share right now so adding transactions to, e.g., Paste or Macros (T2157) means massive amounts of similar code.

Tons of work left to do here, but I think it basically works. Here's a screenshot:

{F26820}

Test Plan: Made transactions in Pholio.

Reviewers: btrahan, vrana, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2104

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

resources/sql/patches/20121209.pholioxactions.sql

@@ -0,0 +1,51 @@
+DROP TABLE {$NAMESPACE}_pholio.pholio_transaction;
+DROP TABLE {$NAMESPACE}_pholio.pholio_pixelcomment;
+
+CREATE TABLE {$NAMESPACE}_pholio.pholio_transaction (
+  id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
+  phid VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  commentPHID VARCHAR(64) COLLATE utf8_bin,
+  commentVersion INT UNSIGNED NOT NULL,
+  transactionType VARCHAR(32) NOT NULL COLLATE utf8_bin,
+  oldValue LONGTEXT NOT NULL COLLATE utf8_bin,
+  newValue LONGTEXT NOT NULL COLLATE utf8_bin,
+  contentSource LONGTEXT NOT NULL COLLATE utf8_bin,
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL,
+
+  UNIQUE KEY `key_phid` (phid),
+  KEY `key_object` (objectPHID)
+
+) ENGINE=InnoDB, COLLATE utf8_general_ci;
+
+CREATE TABLE {$NAMESPACE}_pholio.pholio_transaction_comment (
+  id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
+  phid VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  transactionPHID VARCHAR(64) COLLATE utf8_bin,
+  authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  commentVersion INT UNSIGNED NOT NULL,
+  content LONGTEXT NOT NULL COLLATE utf8_bin,
+  contentSource LONGTEXT NOT NULL COLLATE utf8_bin,
+  isDeleted BOOL NOT NULL,
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL,
+
+  mockID INT UNSIGNED,
+  imageID INT UNSIGNED,
+  x INT UNSIGNED,
+  y INT UNSIGNED,
+  width INT UNSIGNED,
+  height INT UNSIGNED,
+
+  UNIQUE KEY `key_phid` (phid),
+  UNIQUE KEY `key_version` (transactionPHID, commentVersion),
+  UNIQUE KEY `key_draft` (authorPHID, mockID, transactionPHID)
+
+) ENGINE=InnoDB, COLLATE utf8_general_ci;
+

src/__phutil_library_map__.php

@@ -597,6 +597,13 @@ phutil_register_library_map(array(
     'PhabricatorApplicationSlowvote' => 'applications/slowvote/application/PhabricatorApplicationSlowvote.php',
     'PhabricatorApplicationStatusView' => 'applications/meta/view/PhabricatorApplicationStatusView.php',
     'PhabricatorApplicationSubscriptions' => 'applications/subscriptions/application/PhabricatorApplicationSubscriptions.php',
+    'PhabricatorApplicationTransaction' => 'applications/transactions/storage/PhabricatorApplicationTransaction.php',
+    'PhabricatorApplicationTransactionComment' => 'applications/transactions/storage/PhabricatorApplicationTransactionComment.php',
+    'PhabricatorApplicationTransactionCommentEditor' => 'applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php',
+    'PhabricatorApplicationTransactionCommentQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php',
+    'PhabricatorApplicationTransactionEditor' => 'applications/transactions/editor/PhabricatorApplicationTransactionEditor.php',
+    'PhabricatorApplicationTransactionQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionQuery.php',
+    'PhabricatorApplicationTransactions' => 'applications/transactions/application/PhabricatorApplicationTransactions.php',
     'PhabricatorApplicationUIExamples' => 'applications/uiexample/application/PhabricatorApplicationUIExamples.php',
     'PhabricatorApplicationsListController' => 'applications/meta/controller/PhabricatorApplicationsListController.php',
     'PhabricatorAuditActionConstants' => 'applications/audit/constants/PhabricatorAuditActionConstants.php',
@@ -1129,6 +1136,7 @@ phutil_register_library_map(array(
     'PhabricatorTimelineView' => 'view/layout/PhabricatorTimelineView.php',
     'PhabricatorTimer' => 'applications/countdown/storage/PhabricatorTimer.php',
     'PhabricatorTransactionView' => 'view/layout/PhabricatorTransactionView.php',
+    'PhabricatorTransactions' => 'applications/transactions/constants/PhabricatorTransactions.php',
     'PhabricatorTransformedFile' => 'applications/files/storage/PhabricatorTransformedFile.php',
     'PhabricatorTranslation' => 'infrastructure/internationalization/PhabricatorTranslation.php',
     'PhabricatorTrivialTestCase' => 'infrastructure/testing/__tests__/PhabricatorTrivialTestCase.php',
@@ -1225,9 +1233,9 @@ phutil_register_library_map(array(
     'PholioMockListController' => 'applications/pholio/controller/PholioMockListController.php',
     'PholioMockQuery' => 'applications/pholio/query/PholioMockQuery.php',
     'PholioMockViewController' => 'applications/pholio/controller/PholioMockViewController.php',
-    'PholioPixelComment' => 'applications/pholio/storage/PholioPixelComment.php',
     'PholioReplyHandler' => 'applications/pholio/mail/PholioReplyHandler.php',
     'PholioTransaction' => 'applications/pholio/storage/PholioTransaction.php',
+    'PholioTransactionComment' => 'applications/pholio/storage/PholioTransactionComment.php',
     'PholioTransactionQuery' => 'applications/pholio/query/PholioTransactionQuery.php',
     'PholioTransactionType' => 'applications/pholio/constants/PholioTransactionType.php',
     'PhortuneMonthYearExpiryControl' => 'applications/phortune/control/PhortuneMonthYearExpiryControl.php',
@@ -1843,6 +1851,17 @@ phutil_register_library_map(array(
     'PhabricatorApplicationSlowvote' => 'PhabricatorApplication',
     'PhabricatorApplicationStatusView' => 'AphrontView',
     'PhabricatorApplicationSubscriptions' => 'PhabricatorApplication',
+    'PhabricatorApplicationTransaction' => 'PhabricatorLiskDAO',
+    'PhabricatorApplicationTransactionComment' =>
+    array(
+      0 => 'PhabricatorLiskDAO',
+      1 => 'PhabricatorMarkupInterface',
+    ),
+    'PhabricatorApplicationTransactionCommentEditor' => 'PhabricatorEditor',
+    'PhabricatorApplicationTransactionCommentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorApplicationTransactionEditor' => 'PhabricatorEditor',
+    'PhabricatorApplicationTransactionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorApplicationTransactions' => 'PhabricatorApplication',
     'PhabricatorApplicationUIExamples' => 'PhabricatorApplication',
     'PhabricatorApplicationsListController' => 'PhabricatorController',
     'PhabricatorAuditAddCommentController' => 'PhabricatorAuditController',
@@ -2437,21 +2456,13 @@ phutil_register_library_map(array(
     ),
     'PholioMockCommentController' => 'PholioController',
     'PholioMockEditController' => 'PholioController',
-    'PholioMockEditor' => 'PhabricatorEditor',
+    'PholioMockEditor' => 'PhabricatorApplicationTransactionEditor',
     'PholioMockListController' => 'PholioController',
     'PholioMockQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PholioMockViewController' => 'PholioController',
-    'PholioPixelComment' =>
-    array(
-      0 => 'PholioDAO',
-      1 => 'PhabricatorMarkupInterface',
-    ),
     'PholioReplyHandler' => 'PhabricatorMailReplyHandler',
-    'PholioTransaction' =>
+    'PholioTransaction' => 'PhabricatorApplicationTransaction',
-    array(
+    'PholioTransactionComment' => 'PhabricatorApplicationTransactionComment',
-      0 => 'PholioDAO',
-      1 => 'PhabricatorMarkupInterface',
-    ),
     'PholioTransactionQuery' => 'PhabricatorOffsetPagedQuery',
     'PholioTransactionType' => 'PholioConstants',
     'PhortuneMonthYearExpiryControl' => 'AphrontFormControl',

src/applications/phid/PhabricatorPHIDConstants.php

@@ -30,4 +30,7 @@ final class PhabricatorPHIDConstants {
   const PHID_TYPE_ANSW    = 'ANSW';
   const PHID_TYPE_MOCK    = 'MOCK';
 
+  const PHID_TYPE_XACT    = 'XACT';
+  const PHID_TYPE_XCMT    = 'XCMT';
+
 }

src/applications/phid/storage/PhabricatorPHID.php

@@ -7,13 +7,21 @@ final class PhabricatorPHID {
   protected $ownerPHID;
   protected $parentPHID;
 
-  public static function generateNewPHID($type) {
+  public static function generateNewPHID($type, $subtype = null) {
     if (!$type) {
       throw new Exception("Can not generate PHID with no type.");
     }
 
-    $uniq = Filesystem::readRandomCharacters(20);
+    if ($subtype === null) {
-    return 'PHID-'.$type.'-'.$uniq;
+      $uniq_len = 20;
+      $type_str = "{$type}";
+    } else {
+      $uniq_len = 15;
+      $type_str = "{$type}-{$subtype}";
+    }
+
+    $uniq = Filesystem::readRandomCharacters($uniq_len);
+    return "PHID-{$type_str}-{$uniq}";
   }
 
   public static function fromObjectName($name) {

src/applications/pholio/constants/PholioTransactionType.php

@@ -2,10 +2,7 @@
 
 final class PholioTransactionType extends PholioConstants {
 
-  const TYPE_NONE         = 'none';
   const TYPE_NAME         = 'name';
   const TYPE_DESCRIPTION  = 'description';
-  const TYPE_VIEW_POLICY  = 'viewPolicy';
-  const TYPE_SUBSCRIBERS  = 'subscribers';
 
 }

src/applications/pholio/controller/PholioMockCommentController.php

@@ -44,8 +44,10 @@ final class PholioMockCommentController extends PholioController {
       ));
 
     $xaction = id(new PholioTransaction())
-      ->setTransactionType(PholioTransactionType::TYPE_NONE)
+      ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
-      ->setComment($comment);
+      ->attachComment(
+        id(new PholioTransactionComment())
+          ->setContent($comment));
 
     id(new PholioMockEditor())
       ->setActor($user)

src/applications/pholio/controller/PholioMockEditController.php

@@ -57,7 +57,7 @@ final class PholioMockEditController extends PholioController {
 
       $type_name = PholioTransactionType::TYPE_NAME;
       $type_desc = PholioTransactionType::TYPE_DESCRIPTION;
-      $type_view = PholioTransactionType::TYPE_VIEW_POLICY;
+      $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY;
 
       $v_name = $request->getStr('name');
       $v_desc = $request->getStr('description');

src/applications/pholio/controller/PholioMockViewController.php

@@ -25,7 +25,8 @@ final class PholioMockViewController extends PholioController {
     }
 
     $xactions = id(new PholioTransactionQuery())
-      ->withMockIDs(array($mock->getID()))
+      ->setViewer($user)
+      ->withObjectPHIDs(array($mock->getPHID()))
       ->execute();
 
     $subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID(
@@ -33,12 +34,6 @@ final class PholioMockViewController extends PholioController {
 
     $phids = array();
     $phids[] = $mock->getAuthorPHID();
-    foreach ($xactions as $xaction) {
-      $phids[] = $xaction->getAuthorPHID();
-      foreach ($xaction->getRequiredHandlePHIDs() as $hphid) {
-        $phids[] = $hphid;
-      }
-    }
     foreach ($subscribers as $subscriber) {
       $phids[] = $subscriber;
     }
@@ -49,7 +44,11 @@ final class PholioMockViewController extends PholioController {
       ->setViewer($user);
     $engine->addObject($mock, PholioMock::MARKUP_FIELD_DESCRIPTION);
     foreach ($xactions as $xaction) {
-      $engine->addObject($xaction, PholioTransaction::MARKUP_FIELD_COMMENT);
+      if ($xaction->getComment()) {
+        $engine->addObject(
+          $xaction->getComment(),
+          PholioTransaction::MARKUP_FIELD_COMMENT);
+      }
     }
     $engine->process();
 
@@ -198,113 +197,20 @@ final class PholioMockViewController extends PholioController {
     $view = new PhabricatorTimelineView();
 
     foreach ($xactions as $xaction) {
-      $author = $this->getHandle($xaction->getAuthorPHID());
+      if ($xaction->shouldHide()) {
-
-      $old = $xaction->getOldValue();
-      $new = $xaction->getNewValue();
-
-      $xaction_visible = true;
-      $title = null;
-      $type = $xaction->getTransactionType();
-
-      switch ($type) {
-        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;
-        case PholioTransactionType::TYPE_SUBSCRIBERS:
-          $rem = array_diff($old, $new);
-          $add = array_diff($new, $old);
-
-          $add_l = array();
-          foreach ($add as $phid) {
-            $add_l[] = $this->getHandle($phid)->renderLink();
-          }
-          $add_l = implode(', ', $add_l);
-
-          $rem_l = array();
-          foreach ($rem as $phid) {
-            $rem_l[] = $this->getHandle($phid)->renderLink();
-          }
-          $rem_l = implode(', ', $rem_l);
-
-          if ($add && $rem) {
-            $title = pht(
-              '%s edited subscriber(s), added %d: %s; removed %d: %s.',
-              $author->renderLink(),
-              $add_l,
-              count($add),
-              $rem_l,
-              count($rem));
-          } else if ($add) {
-            $title = pht(
-              '%s added %d subscriber(s): %s.',
-              $author->renderLink(),
-              count($add),
-              $add_l);
-          } else if ($rem) {
-            $title = pht(
-              '%s removed %d subscribers: %s.',
-              $author->renderLink(),
-              count($rem),
-              $rem_l);
-          }
-          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;
       }
 
+      $title = $xaction->getTitle();
+
       $event = id(new PhabricatorTimelineEventView())
-        ->setUserHandle($author);
+        ->setUserHandle($xaction->getHandle($xaction->getAuthorPHID()))
+        ->setTitle($title);
 
-      $event->setTitle($title);
+      if ($xaction->getComment()) {
-
-      if (strlen($xaction->getComment())) {
         $event->appendChild(
           $engine->getOutput(
-            $xaction,
+            $xaction->getComment(),
             PholioTransaction::MARKUP_FIELD_COMMENT));
       }
 

src/applications/pholio/editor/PholioMockEditor.php

@@ -3,111 +3,88 @@
 /**
  * @group pholio
  */
-final class PholioMockEditor extends PhabricatorEditor {
+final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
 
-  private $contentSource;
+  public function getTransactionTypes() {
+    $types = parent::getTransactionTypes();
 
-  public function setContentSource(PhabricatorContentSource $content_source) {
+    $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
-    $this->contentSource = $content_source;
+    $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
-    return $this;
+
+    $types[] = PholioTransactionType::TYPE_NAME;
+    $types[] = PholioTransactionType::TYPE_DESCRIPTION;
+    return $types;
   }
 
-  public function getContentSource() {
+  protected function didApplyTransactions(
-    return $this->contentSource;
+    PhabricatorLiskDAO $object,
-  }
+    array $xactions) {
-
+//    $this->sendMail($object, $xactions);
-  public function applyTransactions(PholioMock $mock, array $xactions) {
+//    PholioIndexer::indexMock($mock);
-    assert_instances_of($xactions, 'PholioTransaction');
-
-    $actor = $this->requireActor();
-    if (!$this->contentSource) {
-      throw new Exception(
-        "Call setContentSource() before applyTransactions()!");
-    }
-
-    $is_new = !$mock->getID();
-
-    $comments = array();
-    foreach ($xactions as $xaction) {
-      if (strlen($xaction->getComment())) {
-        $comments[] = $xaction->getComment();
-      }
-      $type = $xaction->getTransactionType();
-      if ($type == PholioTransactionType::TYPE_DESCRIPTION) {
-        $comments[] = $xaction->getNewValue();
-      }
-    }
-
-    $mentioned_phids = PhabricatorMarkupEngine::extractPHIDsFromMentions(
-      $comments);
-    $subscribe_phids = $mentioned_phids;
-
-    // Attempt to subscribe the actor.
-    $subscribe_phids[] = $actor->getPHID();
-
-    if ($subscribe_phids) {
-      if ($mock->getID()) {
-        $old_subs = PhabricatorSubscribersQuery::loadSubscribersForPHID(
-          $mock->getPHID());
-      } else {
-        $old_subs = array();
-      }
-
-      $new_subs = array_merge($old_subs, $mentioned_phids);
-      $xaction = id(new PholioTransaction())
-        ->setTransactionType(PholioTransactionType::TYPE_SUBSCRIBERS)
-        ->setOldValue($old_subs)
-        ->setNewValue($new_subs);
-      array_unshift($xactions, $xaction);
-    }
-
-    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();
+  protected function getCustomTransactionOldValue(
-      $mock->save();
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
 
-      foreach ($xactions as $xaction) {
+    switch ($xaction->getTransactionType()) {
-        $xaction->setMockID($mock->getID());
+      case PholioTransactionType::TYPE_NAME:
-        $xaction->save();
+        return $object->getName();
+      case PholioTransactionType::TYPE_DESCRIPTION:
+        return $object->getDescription();
+    }
   }
 
-      // Apply ID/PHID-dependent transactions.
+  protected function getCustomTransactionNewValue(
-      foreach ($xactions as $xaction) {
+    PhabricatorLiskDAO $object,
-        $type = $xaction->getTransactionType();
+    PhabricatorApplicationTransaction $xaction) {
-        switch ($type) {
+
-          case PholioTransactionType::TYPE_SUBSCRIBERS:
+    switch ($xaction->getTransactionType()) {
-            $subeditor = id(new PhabricatorSubscriptionsEditor())
+      case PholioTransactionType::TYPE_NAME:
-              ->setObject($mock)
+      case PholioTransactionType::TYPE_DESCRIPTION:
-              ->setActor($this->requireActor())
+        return $xaction->getNewValue();
-              ->subscribeExplicit($xaction->getNewValue())
+    }
-              ->save();
+  }
+
+  protected function applyCustomInternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case PholioTransactionType::TYPE_NAME:
+        $object->setName($xaction->getNewValue());
+        if ($object->getOriginalName() === null) {
+          $object->setOriginalName($xaction->getNewValue());
+        }
+        break;
+      case PholioTransactionType::TYPE_DESCRIPTION:
+        $object->setDescription($xaction->getNewValue());
         break;
     }
   }
 
-    $mock->saveTransaction();
+  protected function applyCustomExternalTransaction(
-
+    PhabricatorLiskDAO $object,
-    $this->sendMail($mock, $xactions, $is_new, $mentioned_phids);
+    PhabricatorApplicationTransaction $xaction) {
-
+    return;
-    PholioIndexer::indexMock($mock);
-
-    return $this;
   }
 
+  protected function mergeTransactions(
+    PhabricatorApplicationTransaction $u,
+    PhabricatorApplicationTransaction $v) {
+
+    $type = $u->getTransactionType();
+    switch ($type) {
+      case PholioTransactionType::TYPE_NAME:
+      case PholioTransactionType::TYPE_DESCRIPTION:
+        return $v;
+    }
+
+    return parent::mergeTransactions($u, $v);
+  }
+
+
   private function sendMail(
     PholioMock $mock,
     array $xactions,
@@ -194,119 +171,5 @@ final class PholioMockEditor extends PhabricatorEditor {
     return PhabricatorEnv::getEnvConfig('metamta.pholio.subject-prefix');
   }
 
-  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;
-      case PholioTransactionType::TYPE_SUBSCRIBERS:
-        $old = PhabricatorSubscribersQuery::loadSubscribersForPHID(
-          $mock->getPHID());
-        break;
-      default:
-        throw new Exception("Unknown transaction type '{$type}'!");
-    }
-
-    $xaction->setOldValue($old);
-
-    if (!$this->transactionHasEffect($mock, $xaction)) {
-      return false;
-    }
-
-    switch ($type) {
-      case PholioTransactionType::TYPE_NONE:
-        break;
-      case PholioTransactionType::TYPE_NAME:
-        $mock->setName($xaction->getNewValue());
-        if ($mock->getOriginalName() === null) {
-          $mock->setOriginalName($xaction->getNewValue());
-        }
-        break;
-      case PholioTransactionType::TYPE_DESCRIPTION:
-        $mock->setDescription($xaction->getNewValue());
-        break;
-      case PholioTransactionType::TYPE_VIEW_POLICY:
-        $mock->setViewPolicy($xaction->getNewValue());
-        break;
-      case PholioTransactionType::TYPE_SUBSCRIBERS:
-        // This applies later.
-        break;
-      default:
-        throw new Exception("Unknown transaction type '{$type}'!");
-    }
-
-    return true;
-  }
-
-  private function transactionHasEffect(
-    PholioMock $mock,
-    PholioTransaction $xaction) {
-
-    $effect = false;
-
-    $old = $xaction->getOldValue();
-    $new = $xaction->getNewValue();
-
-    $type = $xaction->getTransactionType();
-    switch ($type) {
-      case PholioTransactionType::TYPE_NONE:
-      case PholioTransactionType::TYPE_NAME:
-      case PholioTransactionType::TYPE_DESCRIPTION:
-      case PholioTransactionType::TYPE_VIEW_POLICY:
-        $effect = ($old !== $new);
-        break;
-      case PholioTransactionType::TYPE_SUBSCRIBERS:
-        $old = nonempty($old, array());
-        $old_map = array_fill_keys($old, true);
-        $filtered = $old;
-
-        foreach ($new as $phid) {
-          if ($mock->getAuthorPHID() == $phid) {
-            // The author may not be explicitly subscribed.
-            continue;
-          }
-          if (isset($old_map[$phid])) {
-            // This PHID was already subscribed.
-            continue;
-          }
-          $filtered[] = $phid;
-        }
-
-        $old = array_keys($old_map);
-        $new = array_values($filtered);
-
-        $xaction->setOldValue($old);
-        $xaction->setNewValue($new);
-
-        $effect = ($old !== $new);
-        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/query/PholioTransactionQuery.php

@@ -4,45 +4,10 @@
  * @group pholio
  */
 final class PholioTransactionQuery
-  extends PhabricatorOffsetPagedQuery {
+  extends PhabricatorApplicationTransactionQuery {
 
-  private $mockIDs;
+  protected function getTemplateApplicationTransaction() {
-
+    return new PholioTransaction();
-  public function withMockIDs(array $ids) {
-    $this->mockIDs = $ids;
-    return $this;
-  }
-
-  public function execute() {
-    $table = new PholioTransaction();
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T x %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    return $table->loadAllFromArray($data);
-  }
-
-  private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
-    $where = array();
-
-    if ($this->mockIDs) {
-      $where[] = qsprintf(
-        $conn_r,
-        'mockID IN (%Ld)',
-        $this->mockIDs);
-    }
-
-    return $this->formatWhereClause($where);
-  }
-
-  private function buildOrderClause(AphrontDatabaseConnection $conn_r) {
-    return 'ORDER BY id ASC';
   }
 
 }

src/applications/pholio/storage/PholioPixelComment.php

@@ -1,46 +0,0 @@
-<?php
-
-/**
- * @group pholio
- */
-final class PholioPixelComment extends PholioDAO
-  implements PhabricatorMarkupInterface {
-
-  const MARKUP_FIELD_COMMENT  = 'markup:comment';
-
-  protected $mockID;
-  protected $transactionID;
-  protected $authorPHID;
-  protected $imageID;
-
-  protected $x;
-  protected $y;
-  protected $width;
-  protected $height;
-  protected $comment;
-
-
-/* -(  PhabricatorMarkupInterface  )----------------------------------------- */
-
-
-  public function getMarkupFieldKey($field) {
-    return 'MP:'.$this->getID();
-  }
-
-  public function newMarkupEngine($field) {
-    return PhabricatorMarkupEngine::newMarkupEngine(array());
-  }
-
-  public function getMarkupText($field) {
-    return $this->getComment();
-  }
-
-  public function didMarkupText($field, $output, PhutilMarkupEngine $engine) {
-    return $output;
-  }
-
-  public function shouldUseMarkupCache($field) {
-    return ($this->getID() && $this->getTransactionID());
-  }
-
-}

src/applications/pholio/storage/PholioTransaction.php

@@ -3,104 +3,60 @@
 /**
  * @group pholio
  */
-final class PholioTransaction extends PholioDAO
+final class PholioTransaction extends PhabricatorApplicationTransaction {
-  implements PhabricatorMarkupInterface {
 
-  const MARKUP_FIELD_COMMENT  = 'markup:comment';
+  public function getApplicationName() {
-
+    return 'pholio';
-  protected $mockID;
-  protected $authorPHID;
-  protected $transactionType;
-  protected $oldValue;
-  protected $newValue;
-  protected $comment = '';
-  protected $metadata = array();
-  protected $contentSource;
-
-  public function getConfiguration() {
-    return array(
-      self::CONFIG_SERIALIZATION => array(
-        'oldValue' => self::SERIALIZATION_JSON,
-        'newValue' => self::SERIALIZATION_JSON,
-        'metadata' => self::SERIALIZATION_JSON,
-      ),
-    ) + parent::getConfiguration();
   }
 
-  public function setContentSource(PhabricatorContentSource $content_source) {
+  public function getApplicationTransactionType() {
-    $this->contentSource = $content_source->serialize();
+    return PhabricatorPHIDConstants::PHID_TYPE_MOCK;
-    return $this;
   }
 
-  public function getContentSource() {
+  public function getApplicationTransactionCommentObject() {
-    return PhabricatorContentSource::newFromSerialized($this->contentSource);
+    return new PholioTransactionComment();
   }
 
-  public function getRequiredHandlePHIDs() {
+  public function getApplicationObjectTypeName() {
+    return pht('mock');
+  }
+
+  public function shouldHide() {
+    $old = $this->getOldValue();
+
     switch ($this->getTransactionType()) {
-      case PholioTransactionType::TYPE_SUBSCRIBERS:
+      case PholioTransactionType::TYPE_NAME:
-        return array_merge(
+      case PholioTransactionType::TYPE_DESCRIPTION:
-          $this->getOldValue(),
+        return ($old === null);
-          $this->getNewValue());
-      default:
-        return array();
     }
+
+    return parent::shouldHide();
+  }
+
+  public function getTitle() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case PholioTransactionType::TYPE_NAME:
+        return pht(
+          '%s renamed this mock from "%s" to "%s".',
+          $this->renderHandleLink($author_phid),
+          phutil_escape_html($old),
+          phutil_escape_html($new));
+        break;
+      case PholioTransactionType::TYPE_DESCRIPTION:
+        return pht(
+          '%s updated the description of this mock. '.
+          'The old description was: %s',
+          $this->renderHandleLink($author_phid),
+          phutil_escape_html($old));
+    }
+
+    return parent::getTitle();
   }
 
 
-/* -(  PhabricatorSubscribableInterface Implementation  )-------------------- */
-
-
-  public function isAutomaticallySubscribed($phid) {
-    return ($this->authorPHID == $phid);
-  }
-
-
-/* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */
-
-
-  public function getCapabilities() {
-    return array(
-      PhabricatorPolicyCapability::CAN_VIEW,
-      PhabricatorPolicyCapability::CAN_EDIT,
-    );
-  }
-
-  public function getPolicy($capability) {
-    switch ($capability) {
-      case PhabricatorPolicyCapability::CAN_VIEW:
-        return $this->getViewPolicy();
-      case PhabricatorPolicyCapability::CAN_EDIT:
-        return PhabricatorPolicies::POLICY_NOONE;
-    }
-  }
-
-  public function hasAutomaticCapbility($capability, PhabricatorUser $viewer) {
-    return ($viewer->getPHID() == $this->getAuthorPHID());
-  }
-
-
-/* -(  PhabricatorMarkupInterface  )----------------------------------------- */
-
-
-  public function getMarkupFieldKey($field) {
-    return 'MX:'.$this->getID();
-  }
-
-  public function newMarkupEngine($field) {
-    return PhabricatorMarkupEngine::newMarkupEngine(array());
-  }
-
-  public function getMarkupText($field) {
-    return $this->getComment();
-  }
-
-  public function didMarkupText($field, $output, PhutilMarkupEngine $engine) {
-    return $output;
-  }
-
-  public function shouldUseMarkupCache($field) {
-    return (bool)$this->getID();
-  }
-
 }

src/applications/pholio/storage/PholioTransactionComment.php

@@ -0,0 +1,19 @@
+<?php
+
+/**
+ * @group pholio
+ */
+final class PholioTransactionComment
+  extends PhabricatorApplicationTransactionComment {
+
+  protected $imageID;
+  protected $x;
+  protected $y;
+  protected $width;
+  protected $height;
+
+  public function getApplicationTransactionObject() {
+    return new PholioTransaction();
+  }
+
+}

src/applications/transactions/application/PhabricatorApplicationTransactions.php

@@ -0,0 +1,15 @@
+<?php
+
+final class PhabricatorApplicationTransactions extends PhabricatorApplication {
+
+  public function shouldAppearInLaunchView() {
+    return false;
+  }
+
+  public function getRoutes() {
+    return array(
+    );
+  }
+
+}
+

src/applications/transactions/constants/PhabricatorTransactions.php

@@ -0,0 +1,10 @@
+<?php
+
+final class PhabricatorTransactions {
+
+  const TYPE_COMMENT      = 'core:comment';
+  const TYPE_SUBSCRIBERS  = 'core:subscribers';
+  const TYPE_VIEW_POLICY  = 'core:view-policy';
+  const TYPE_EDIT_POLICY  = 'core:edit-policy';
+
+}

src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php

@@ -0,0 +1,102 @@
+<?php
+
+final class PhabricatorApplicationTransactionCommentEditor
+  extends PhabricatorEditor {
+
+  private $contentSource;
+
+  public function setContentSource(PhabricatorContentSource $content_source) {
+    $this->contentSource = $content_source;
+    return $this;
+  }
+
+  public function getContentSource() {
+    return $this->contentSource;
+  }
+
+  /**
+   * Edit a transaction's comment. This method effects the required create,
+   * update or delete to set the transaction's comment to the provided comment.
+   */
+  public function applyEdit(
+    PhabricatorApplicationTransaction $xaction,
+    PhabricatorApplicationTransactionComment $comment) {
+
+    $this->validateEdit($xaction, $comment);
+
+    $actor = $this->requireActor();
+
+    $comment->setContentSource($this->getContentSource());
+    $comment->setAuthorPHID($actor->getPHID());
+
+    // TODO: This needs to be more sophisticated once we have meta-policies.
+    $comment->setViewPolicy(PhabricatorPolicies::POLICY_PUBLIC);
+    $comment->setEditPolicy($actor->getPHID());
+
+    $xaction->openTransaction();
+      $xaction->beginReadLocking();
+        if ($xaction->getID()) {
+          $xaction->reload();
+        }
+
+        $new_version = $xaction->getCommentVersion() + 1;
+
+        $comment->setCommentVersion($new_version);
+        $comment->setTransactionPHID($xaction->getPHID());
+        $comment->save();
+
+        $xaction->setCommentVersion($new_version);
+        $xaction->setCommentPHID($comment->getPHID());
+        $xaction->setViewPolicy($comment->getViewPolicy());
+        $xaction->setEditPolicy($comment->getEditPolicy());
+        $xaction->save();
+
+      $xaction->endReadLocking();
+    $xaction->saveTransaction();
+
+    $xaction->attachComment($comment);
+
+    // TODO: Emit an event for notifications/feed? Can we handle them
+    // generically?
+
+    return $this;
+  }
+
+  /**
+   * Validate that the edit is permissible, and the actor has permission to
+   * perform it.
+   */
+  private function validateEdit(
+    PhabricatorApplicationTransaction $xaction,
+    PhabricatorApplicationTransactionComment $comment) {
+
+    if (!$xaction->getPHID()) {
+      throw new Exception(
+        "Transaction must have a PHID before calling applyEdit()!");
+    }
+
+    if ($comment->getPHID()) {
+      throw new Exception(
+        "Transaction comment must not yet have a PHID!");
+    }
+
+    if (!$this->getContentSource()) {
+      throw new Exception(
+        "Call setContentSource() before applyEdit()!");
+    }
+
+    $actor = $this->requireActor();
+
+    PhabricatorPolicyFilter::requireCapability(
+      $actor,
+      $xaction,
+      PhabricatorPolicyCapability::CAN_VIEW);
+
+    PhabricatorPolicyFilter::requireCapability(
+      $actor,
+      $xaction,
+      PhabricatorPolicyCapability::CAN_EDIT);
+  }
+
+
+}

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -0,0 +1,487 @@
+<?php
+
+abstract class PhabricatorApplicationTransactionEditor
+  extends PhabricatorEditor {
+
+  private $contentSource;
+  private $object;
+  private $xactions;
+
+  private $isNewObject;
+  private $mentionedPHIDs;
+
+  protected function getIsNewObject() {
+    return $this->isNewObject;
+  }
+
+  protected function getMentionedPHIDs() {
+    return $this->mentionedPHIDs;
+  }
+
+  public function getTransactionTypes() {
+    $types = array(
+      PhabricatorTransactions::TYPE_COMMENT,
+    );
+
+    if ($this->object instanceof PhabricatorSubscribableInterface) {
+      $types[] = PhabricatorTransactions::TYPE_SUBSCRIBERS;
+    }
+
+    return $types;
+  }
+
+  private function adjustTransactionValues(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    $old = $this->getTransactionOldValue($object, $xaction);
+    $xaction->setOldValue($old);
+
+    $new = $this->getTransactionNewValue($object, $xaction);
+    $xaction->setNewValue($new);
+  }
+
+  private function getTransactionOldValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        if ($object->getPHID()) {
+          $old_phids = PhabricatorSubscribersQuery::loadSubscribersForPHID(
+            $object->getPHID());
+        } else {
+          $old_phids = array();
+        }
+        return array_values($old_phids);
+      case PhabricatorTransactions::TYPE_VIEW_POLICY:
+        return $object->getViewPolicy();
+      case PhabricatorTransactions::TYPE_EDIT_POLICY:
+        return $object->getEditPolicy();
+      default:
+        return $this->getCustomTransactionOldValue($object, $xaction);
+    }
+  }
+
+  private function getTransactionNewValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        return $this->getPHIDTransactionNewValue($xaction);
+      case PhabricatorTransactions::TYPE_VIEW_POLICY:
+      case PhabricatorTransactions::TYPE_EDIT_POLICY:
+        return $xaction->getNewValue();
+      default:
+        return $this->getCustomTransactionNewValue($object, $xaction);
+    }
+  }
+
+  protected function getCustomTransactionOldValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    throw new Exception("Capability not supported!");
+  }
+
+  protected function getCustomTransactionNewValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    throw new Exception("Capability not supported!");
+  }
+
+  protected function transactionHasEffect(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    return ($xaction->getOldValue() !== $xaction->getNewValue());
+  }
+
+  private function applyInternalEffects(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_COMMENT:
+        break;
+      case PhabricatorTransactions::TYPE_VIEW_POLICY:
+        $object->setViewPolicy($xaction->getNewValue());
+        break;
+      case PhabricatorTransactions::TYPE_EDIT_POLICY:
+        $object->setEditPolicy($xaction->getNewValue());
+        break;
+      default:
+        return $this->applyCustomInternalTransaction($object, $xaction);
+    }
+  }
+
+  private function applyExternalEffects(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_COMMENT:
+        break;
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        $subeditor = id(new PhabricatorSubscriptionsEditor())
+          ->setObject($object)
+          ->setActor($this->requireActor())
+          ->subscribeExplicit($xaction->getNewValue())
+          ->save();
+        break;
+      default:
+        return $this->applyCustomExternalTransaction($object, $xaction);
+    }
+  }
+
+  protected function applyCustomInternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    throw new Exception("Capability not supported!");
+  }
+
+  protected function applyCustomExternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+    throw new Exception("Capability not supported!");
+  }
+
+  public function setContentSource(PhabricatorContentSource $content_source) {
+    $this->contentSource = $content_source;
+    return $this;
+  }
+
+  public function getContentSource() {
+    return $this->contentSource;
+  }
+
+  protected function didApplyTransactions(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+    return;
+  }
+
+  final public function applyTransactions(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    $this->object = $object;
+    $this->xactions = $xactions;
+    $this->isNewObject = ($object->getPHID() === null);
+
+    $this->validateEditParameters($object, $xactions);
+
+
+    $actor = $this->requireActor();
+
+    $mention_xaction = $this->buildMentionTransaction($object, $xactions);
+    if ($mention_xaction) {
+      $xactions[] = $mention_xaction;
+    }
+
+    $xactions = $this->combineTransactions($xactions);
+
+    foreach ($xactions as $xaction) {
+      // TODO: This needs to be more sophisticated once we have meta-policies.
+      $xaction->setViewPolicy(PhabricatorPolicies::POLICY_PUBLIC);
+      $xaction->setEditPolicy($actor->getPHID());
+
+      $xaction->setAuthorPHID($actor->getPHID());
+      $xaction->setContentSource($this->getContentSource());
+    }
+
+    foreach ($xactions as $xaction) {
+      $this->adjustTransactionValues($object, $xaction);
+    }
+
+    foreach ($xactions as $key => $xaction) {
+      if (!$this->transactionHasEffect($object, $xaction)) {
+        // TODO: Raise these to the user.
+        if ($xaction->getComment()) {
+          $xaction->setTransactionType(
+            PhabricatorTransactions::TYPE_COMMENT);
+          $xaction->setOldValue(null);
+          $xaction->setNewValue(null);
+        } else {
+          unset($xactions[$key]);
+        }
+      }
+    }
+
+    $xactions = $this->sortTransactions($xactions);
+
+    $comment_editor = id(new PhabricatorApplicationTransactionCommentEditor())
+      ->setActor($actor)
+      ->setContentSource($this->getContentSource());
+
+    $object->openTransaction();
+      foreach ($xactions as $xaction) {
+        $this->applyInternalEffects($object, $xaction);
+      }
+
+      $object->save();
+
+      foreach ($xactions as $xaction) {
+        $xaction->setObjectPHID($object->getPHID());
+        if ($xaction->getComment()) {
+          $xaction->setPHID($xaction->generatePHID());
+          $comment_editor->applyEdit($xaction, $xaction->getComment());
+        } else {
+          $xaction->save();
+        }
+      }
+
+      foreach ($xactions as $xaction) {
+        $this->applyExternalEffects($object, $xaction);
+      }
+    $object->saveTransaction();
+
+    // TODO: Send mail.
+    // TODO: Index object.
+    // TODO: Publish feed/notifications.
+
+    $this->didApplyTransactions($object, $xactions);
+
+    return $this;
+  }
+
+  private function validateEditParameters(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    if (!$this->getContentSource()) {
+      throw new Exception(
+        "Call setContentSource() before applyTransactions()!");
+    }
+
+    // Do a bunch of sanity checks that the incoming transactions are fresh.
+    // They should be unsaved and have only "transactionType" and "newValue"
+    // set.
+
+    $types = array_fill_keys($this->getTransactionTypes(), true);
+
+    assert_instances_of($xactions, 'PhabricatorApplicationTransaction');
+    foreach ($xactions as $xaction) {
+      if ($xaction->getPHID() || $xaction->getID()) {
+        throw new Exception(
+          "You can not apply transactions which already have IDs/PHIDs!");
+      }
+      if ($xaction->getObjectPHID()) {
+        throw new Exception(
+          "You can not apply transactions which already have objectPHIDs!");
+      }
+      if ($xaction->getAuthorPHID()) {
+        throw new Exception(
+          "You can not apply transactions which already have authorPHIDs!");
+      }
+      if ($xaction->getCommentPHID()) {
+        throw new Exception(
+          "You can not apply transactions which already have commentPHIDs!");
+      }
+      if ($xaction->getCommentVersion() !== 0) {
+        throw new Exception(
+          "You can not apply transactions which already have commentVersions!");
+      }
+      if ($xaction->getOldValue() !== null) {
+        throw new Exception(
+          "You can not apply transactions which already have oldValue!");
+      }
+
+      $type = $xaction->getTransactionType();
+      if (empty($types[$type])) {
+        throw new Exception("Transaction has unknown type '{$type}'.");
+      }
+    }
+
+    // The actor must have permission to view and edit the object.
+
+    $actor = $this->requireActor();
+
+    PhabricatorPolicyFilter::requireCapability(
+      $actor,
+      $xaction,
+      PhabricatorPolicyCapability::CAN_VIEW);
+
+    PhabricatorPolicyFilter::requireCapability(
+      $actor,
+      $xaction,
+      PhabricatorPolicyCapability::CAN_EDIT);
+  }
+
+  private function buildMentionTransaction(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    if (!($object instanceof PhabricatorSubscribableInterface)) {
+      return null;
+    }
+
+    $texts = array();
+    foreach ($xactions as $xaction) {
+      $texts[] = $this->getMentionableTextsFromTransaction($xaction);
+    }
+    $texts = array_mergev($texts);
+
+    $phids = PhabricatorMarkupEngine::extractPHIDsFromMentions($texts);
+
+    $this->mentionedPHIDs = $phids;
+
+    if (!$phids) {
+      return null;
+    }
+
+    $xaction = newv(get_class(head($xactions)), array());
+    $xaction->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS);
+    $xaction->setNewValue(array('+' => $phids));
+
+    return $xaction;
+  }
+
+  protected function getMentionableTextsFromTransaction(
+    PhabricatorApplicationTransaction $transaction) {
+    $texts = array();
+    if ($transaction->getComment()) {
+      $texts[] = $transaction->getComment()->getContent();
+    }
+    return $texts;
+  }
+
+  protected function mergeTransactions(
+    PhabricatorApplicationTransaction $u,
+    PhabricatorApplicationTransaction $v) {
+
+    $type = $u->getTransactionType();
+
+    switch ($type) {
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        return $this->mergePHIDTransactions($u, $v);
+    }
+
+    // By default, do not merge the transactions.
+    return null;
+  }
+
+
+  /**
+   * Attempt to combine similar transactions into a smaller number of total
+   * transactions. For example, two transactions which edit the title of an
+   * object can be merged into a single edit.
+   */
+  private function combineTransactions(array $xactions) {
+    $stray_comments = array();
+
+    $result = array();
+    $types = array();
+    foreach ($xactions as $key => $xaction) {
+      $type = $xaction->getTransactionType();
+      if (isset($types[$type])) {
+        foreach ($types[$type] as $other_key) {
+          $merged = $this->mergeTransactions($result[$other_key], $xaction);
+          if ($merged) {
+            $result[$other_key] = $merged;
+
+            if ($xaction->getComment() &&
+                ($xaction->getComment() !== $merged->getComment())) {
+              $stray_comments[] = $xaction->getComment();
+            }
+
+            if ($result[$other_key]->getComment() &&
+                ($result[$other_key]->getComment() !== $merged->getComment())) {
+              $stray_comments[] = $result[$other_key]->getComment();
+            }
+
+            // Move on to the next transaction.
+            continue 2;
+          }
+        }
+      }
+      $result[$key] = $xaction;
+      $types[$type][] = $key;
+    }
+
+    // If we merged any comments away, restore them.
+    foreach ($stray_comments as $comment) {
+      $xaction = newv(get_class(head($result)), array());
+      $xaction->setTransactionType(PhabricatorTransactions::TYPE_COMMENT);
+      $xaction->setComment($comment);
+      $result[] = $xaction;
+    }
+
+    return array_values($result);
+  }
+
+  protected function mergePHIDTransactions(
+    PhabricatorApplicationTransaction $u,
+    PhabricatorApplicationTransaction $v) {
+
+    $result = $u->getNewValue();
+    foreach ($v->getNewValue() as $key => $value) {
+      $result[$key] = array_merge($value, idx($result, $key, array()));
+    }
+    $u->setNewValue($result);
+
+    return $u;
+  }
+
+
+  protected function getPHIDTransactionNewValue(
+    PhabricatorApplicationTransaction $xaction) {
+
+    $old = array_combine($xaction->getOldValue(), $xaction->getOldValue());
+
+    $new = $xaction->getNewValue();
+    $new_add = idx($new, '+', array());
+    unset($new['+']);
+    $new_rem = idx($new, '-', array());
+    unset($new['-']);
+    $new_set = idx($new, '=', null);
+    if ($new_set !== null) {
+      $new_set = array_combine($new_set, $new_set);
+    }
+    unset($new['=']);
+
+    if ($new) {
+      throw new Exception(
+        "Invalid 'new' value for PHID transaction. Value should contain only ".
+        "keys '+' (add PHIDs), '-' (remove PHIDs) and '=' (set PHIDS).");
+    }
+
+    $result = array();
+
+    foreach ($old as $phid) {
+      if ($new_set !== null && empty($new_set[$phid])) {
+        continue;
+      }
+      $result[$phid] = $phid;
+    }
+
+    if ($new_set !== null) {
+      foreach ($new_set as $phid) {
+        $result[$phid] = $phid;
+      }
+    }
+
+    foreach ($new_add as $phid) {
+      $result[$phid] = $phid;
+    }
+
+    foreach ($new_rem as $phid) {
+      unset($result[$phid]);
+    }
+
+    return array_values($result);
+  }
+
+  protected function sortTransactions(array $xactions) {
+    $head = array();
+    $tail = array();
+
+    // Move bare comments to the end, so the actions preceed them.
+    foreach ($xactions as $xaction) {
+      $type = $xaction->getTransactionType();
+      if ($type == PhabricatorTransactions::TYPE_COMMENT) {
+        $tail[] = $xaction;
+      } else {
+        $head[] = $xaction;
+      }
+    }
+
+    return array_values(array_merge($head, $tail));
+  }
+
+
+}

src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php

@@ -0,0 +1,62 @@
+<?php
+
+final class PhabricatorApplicationTransactionCommentQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $template;
+
+  private $phids;
+  private $transactionPHIDs;
+
+  public function setTemplate(
+    PhabricatorApplicationTransactionComment $template) {
+    $this->template = $template;
+    return $this;
+  }
+
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
+  public function withTranactionPHIDs(array $transaction_phids) {
+    $this->transactionPHIDs = $transaction_phids;
+    return $this;
+  }
+
+  public function loadPage() {
+    $table = $this->template;
+    $conn_r = $table->establishConnection('r');
+
+    $data = queryfx_all(
+      $conn_r,
+      'SELECT * FROM %T xc %Q %Q %Q',
+      $table->getTableName(),
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));
+
+    return $table->loadAllFromArray($data);
+  }
+
+  private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+    $where = array();
+
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
+    if ($this->transactionPHIDs) {
+      $where[] = qsprintf(
+        $conn_r,
+        'transactionPHID IN (%Ls)',
+        $this->transactionPHIDs);
+    }
+
+    return $this->formatWhereClause($where);
+  }
+
+}

src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php

@@ -0,0 +1,143 @@
+<?php
+
+abstract class PhabricatorApplicationTransactionQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $phids;
+  private $objectPHIDs;
+  private $authorPHIDs;
+
+  private $needComments = true;
+  private $needHandles  = true;
+
+  abstract protected function getTemplateApplicationTransaction();
+
+  protected function buildMoreWhereClauses(AphrontDatabaseConnection $conn_r) {
+    return array();
+  }
+
+  protected function getReversePaging() {
+    return true;
+  }
+
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
+  public function withObjectPHIDs(array $object_phids) {
+    $this->objectPHIDs = $object_phids;
+    return $this;
+  }
+
+  public function withAuthorPHIDs(array $author_phids) {
+    $this->authorPHIDs = $author_phids;
+    return $this;
+  }
+
+  public function needComments($need) {
+    $this->needComments = $need;
+    return $this;
+  }
+
+  public function needHandles($need) {
+    $this->needHandles = $need;
+    return $this;
+  }
+
+  public function loadPage() {
+    $table = $this->getTemplateApplicationTransaction();
+    $conn_r = $table->establishConnection('r');
+
+    $data = queryfx_all(
+      $conn_r,
+      'SELECT * FROM %T x %Q %Q %Q',
+      $table->getTableName(),
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));
+
+    $xactions = $table->loadAllFromArray($data);
+
+    if ($this->needComments) {
+      $comment_phids = array_filter(mpull($xactions, 'getCommentPHID'));
+
+      $comments = array();
+      if ($comment_phids) {
+        $comments = id(new PhabricatorApplicationTransactionCommentQuery())
+          ->setTemplate($table->getApplicationTransactionCommentObject())
+          ->setViewer($this->getViewer())
+          ->withPHIDs($comment_phids)
+          ->execute();
+        $comments = mpull($comments, null, 'getPHID');
+      }
+
+      foreach ($xactions as $xaction) {
+        if ($xaction->getCommentPHID()) {
+          $comment = idx($comments, $xaction->getCommentPHID());
+          if ($comment) {
+            $xaction->attachComment($comment);
+          }
+        }
+      }
+    } else {
+      foreach ($xactions as $xaction) {
+        $xaction->setCommentNotLoaded(true);
+      }
+    }
+
+    if ($this->needHandles) {
+      $phids = array();
+      foreach ($xactions as $xaction) {
+        $phids[$xaction->getPHID()] = $xaction->getRequiredHandlePHIDs();
+      }
+      $handles = array();
+      $merged = array_mergev($phids);
+      if ($merged) {
+        $handles = id(new PhabricatorObjectHandleData($merged))
+          ->setViewer($this->getViewer())
+          ->loadHandles();
+      }
+      foreach ($xactions as $xaction) {
+        $xaction->setHandles(
+          array_select_keys(
+            $handles,
+            $phids[$xaction->getPHID()]));
+      }
+    }
+
+    return $xactions;
+  }
+
+  private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+    $where = array();
+
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
+    if ($this->objectPHIDs) {
+      $where[] = qsprintf(
+        $conn_r,
+        'objectPHID IN (%Ls)',
+        $this->objectPHIDs);
+    }
+
+    if ($this->authorPHIDs) {
+      $where[] = qsprintf(
+        $conn_r,
+        'authorPHID IN (%Ls)',
+        $this->authorPHIDs);
+    }
+
+    foreach ($this->buildMoreWhereClauses($conn_r) as $clause) {
+      $where[] = $clause;
+    }
+
+    return $this->formatWhereClause($where);
+  }
+
+}

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -0,0 +1,222 @@
+<?php
+
+abstract class PhabricatorApplicationTransaction
+  extends PhabricatorLiskDAO
+  implements PhabricatorPolicyInterface{
+
+  const MARKUP_FIELD_COMMENT  = 'markup:comment';
+
+  protected $phid;
+  protected $objectPHID;
+  protected $authorPHID;
+  protected $viewPolicy;
+  protected $editPolicy;
+
+  protected $commentPHID;
+  protected $commentVersion = 0;
+
+  protected $transactionType;
+  protected $oldValue;
+  protected $newValue;
+
+  protected $contentSource;
+
+  private $comment;
+  private $commentNotLoaded;
+
+  private $handles;
+
+  abstract public function getApplicationTransactionType();
+  abstract public function getApplicationTransactionCommentObject();
+  abstract public function getApplicationObjectTypeName();
+
+  public function generatePHID() {
+    $type = PhabricatorPHIDConstants::PHID_TYPE_XACT;
+    $subtype = $this->getApplicationTransactionType();
+
+    return PhabricatorPHID::generateNewPHID($type, $subtype);
+  }
+
+  public function getConfiguration() {
+    return array(
+      self::CONFIG_AUX_PHID => true,
+      self::CONFIG_SERIALIZATION => array(
+        'oldValue' => self::SERIALIZATION_JSON,
+        'newValue' => self::SERIALIZATION_JSON,
+      ),
+    ) + parent::getConfiguration();
+  }
+
+  public function setContentSource(PhabricatorContentSource $content_source) {
+    $this->contentSource = $content_source->serialize();
+    return $this;
+  }
+
+  public function getContentSource() {
+    return PhabricatorContentSource::newFromSerialized($this->contentSource);
+  }
+
+  public function getComment() {
+    if ($this->commentNotLoaded) {
+      throw new Exception("Comment for this transaction was not loaded.");
+    }
+    return $this->comment;
+  }
+
+  public function attachComment(
+    PhabricatorApplicationTransactionComment $comment) {
+    $this->comment = $comment;
+    $this->commentNotLoaded = false;
+    return $this;
+  }
+
+  public function setCommentNotLoaded($not_loaded) {
+    $this->commentNotLoaded = $not_loaded;
+    return $this;
+  }
+
+/* -(  Rendering  )---------------------------------------------------------- */
+
+  public function getRequiredHandlePHIDs() {
+    $phids = array();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    $phids[] = array($this->getAuthorPHID());
+    switch ($this->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        $phids[] = $old;
+        $phids[] = $new;
+        break;
+    }
+
+    return array_mergev($phids);
+  }
+
+  public function setHandles(array $handles) {
+    $this->handles = $handles;
+    return $this;
+  }
+
+  public function getHandle($phid) {
+    if (empty($this->handles[$phid])) {
+      throw new Exception(
+        "Transaction requires a handle ('{$phid}') it did not load.");
+    }
+    return $this->handles[$phid];
+  }
+
+  protected function renderHandleLink($phid) {
+    return $this->getHandle($phid)->renderLink();
+  }
+
+  protected function renderHandleList(array $phids) {
+    $links = array();
+    foreach ($phids as $phid) {
+      $links[] = $this->renderHandleLink($phid);
+    }
+    return implode(', ', $links);
+  }
+
+  public function shouldHide() {
+    switch ($this->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_VIEW_POLICY:
+      case PhabricatorTransactions::TYPE_EDIT_POLICY:
+        if ($this->getOldValue() === null) {
+          return true;
+        } else {
+          return false;
+        }
+        break;
+    }
+
+    return false;
+  }
+
+  public function getTitle() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_COMMENT:
+        return pht(
+          '%s added a comment.',
+          $this->renderHandleLink($author_phid));
+      case PhabricatorTransactions::TYPE_VIEW_POLICY:
+        // TODO: Render human-readable.
+        return pht(
+          '%s changed the visibility of this %s from "%s" to "%s".',
+          $this->renderHandleLink($author_phid),
+          $this->getApplicationObjectTypeName(),
+          phutil_escape_html($old),
+          phutil_escape_html($new));
+      case PhabricatorTransactions::TYPE_EDIT_POLICY:
+        // TODO: Render human-readable.
+        return pht(
+          '%s changed the edit policy of this %s from "%s" to "%s".',
+          $this->renderHandleLink($author_phid),
+          $this->getApplicationObjectTypeName(),
+          phutil_escape_html($old),
+          phutil_escape_html($new));
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        $add = array_diff($new, $old);
+        $rem = array_diff($old, $new);
+
+        if ($add && $rem) {
+          return pht(
+            '%s edited subscriber(s), added %d: %s; removed %d: %s.',
+            $this->renderHandleLink($author_phid),
+            count($add),
+            $this->renderHandleList($add),
+            count($rem),
+            $this->renderHandleList($rem));
+        } else if ($add) {
+          return pht(
+            '%s added %d subscriber(s): %s.',
+            $this->renderHandleLink($author_phid),
+            count($add),
+            $this->renderHandleList($add));
+        } else {
+          return pht(
+            '%s removed %d subscribers: %s.',
+            $this->renderHandleLink($author_phid),
+            count($rem),
+            $this->renderHandleList($rem));
+        }
+        break;
+      default:
+        return pht(
+          '%s edited this %s.',
+          $this->renderHandleLink($author_phid),
+          $this->getApplicationObjectTypeName());
+    }
+  }
+
+
+/* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
+  }
+
+  public function getPolicy($capability) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        return $this->getViewPolicy();
+      case PhabricatorPolicyCapability::CAN_EDIT:
+        return $this->getEditPolicy();
+    }
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return ($viewer->getPHID() == $this->getAuthorPHID());
+  }
+
+}

src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php

@@ -0,0 +1,103 @@
+<?php
+
+abstract class PhabricatorApplicationTransactionComment
+  extends PhabricatorLiskDAO
+  implements PhabricatorMarkupInterface, PhabricatorPolicyInterface {
+
+  protected $transactionPHID;
+  protected $commentVersion;
+  protected $authorPHID;
+  protected $viewPolicy;
+  protected $editPolicy;
+  protected $content;
+  protected $contentSource;
+  protected $isDeleted = 0;
+
+  abstract public function getApplicationTransactionObject();
+
+  public function generatePHID() {
+    return PhabricatorPHID::generateNewPHID(
+      PhabricatorPHIDConstants::PHID_TYPE_XCMT);
+  }
+
+  public function getConfiguration() {
+    return array(
+      self::CONFIG_AUX_PHID => true,
+    ) + parent::getConfiguration();
+  }
+
+  public function getApplicationName() {
+    return $this->getApplicationTransactionObject()->getApplicationName();
+  }
+
+  public function getTableName() {
+    $xaction = $this->getApplicationTransactionObject();
+    return self::getTableNameFromTransaction($xaction);
+  }
+
+  public static function getTableNameFromTransaction(
+    PhabricatorApplicationTransaction $xaction) {
+    return $xaction->getTableName().'_comment';
+  }
+
+  public function setContentSource(PhabricatorContentSource $content_source) {
+    $this->contentSource = $content_source->serialize();
+    return $this;
+  }
+
+  public function getContentSource() {
+    return PhabricatorContentSource::newFromSerialized($this->contentSource);
+  }
+
+
+/* -(  PhabricatorMarkupInterface  )----------------------------------------- */
+
+
+  public function getMarkupFieldKey($field) {
+    return PhabricatorPHIDConstants::PHID_TYPE_XCMT.':'.$this->getPHID();
+  }
+
+
+  public function newMarkupEngine($field) {
+    return PhabricatorMarkupEngine::newMarkupEngine(array());
+  }
+
+
+  public function getMarkupText($field) {
+    return $this->getContent();
+  }
+
+
+  public function didMarkupText($field, $output, PhutilMarkupEngine $engine) {
+    return $output;
+  }
+
+
+  public function shouldUseMarkupCache($field) {
+    return (bool)$this->getPHID();
+  }
+
+/* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
+  }
+
+  public function getPolicy($capability) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        return $this->getViewPolicy();
+      case PhabricatorPolicyCapability::CAN_EDIT:
+        return $this->getEditPolicy();
+    }
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return ($viewer->getPHID() == $this->getAuthorPHID());
+  }
+
+}

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

@@ -1044,6 +1044,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
         'type'    => 'sql',
         'name'    => $this->getPatchPath('owners-exclude.sql'),
       ),
+      '20121209.pholioxactions.sql' => array(
+        'type'    => 'sql',
+        'name'    => $this->getPatchPath('20121209.pholioxactions.sql'),
+      ),
     );
   }