Mostly modularize the Differential "update" transaction

Summary: Ref T13099. Move most of the "Update" logic to modular transactions Test Plan: Created and updated revisions. Flushed the task queue. Grepped for `TYPE_UPDATE`. Reviewed update transactions in the timeline and feed. Maniphest Tasks: T13099 Differential Revision: https://secure.phabricator.com/D19175
2024-11-09 16:32:39 +01:00 · 2018-03-06 08:12:04 -08:00 · 2018-03-06 08:12:04 -08:00 · 743d1ac426
commit 743d1ac426
parent 44f0664d2c
10 changed files with 208 additions and 208 deletions
--- a/resources/sql/autopatches/20140212.dx.1.armageddon.php
+++ b/resources/sql/autopatches/20140212.dx.1.armageddon.php
@ -75,7 +75,7 @@ foreach ($rows as $row) {

  if ($diff_id || $row['action'] == DifferentialAction::ACTION_UPDATE) {
    $xactions[] = array(
-      'type' => DifferentialTransaction::TYPE_UPDATE,
+      'type' => DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE,
      'old' => null,
      'new' => $diff_id,
    );
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -598,6 +598,7 @@ phutil_register_library_map(array(
    'DifferentialRevisionTitleTransaction' => 'applications/differential/xaction/DifferentialRevisionTitleTransaction.php',
    'DifferentialRevisionTransactionType' => 'applications/differential/xaction/DifferentialRevisionTransactionType.php',
    'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/DifferentialRevisionUpdateHistoryView.php',
+    'DifferentialRevisionUpdateTransaction' => 'applications/differential/xaction/DifferentialRevisionUpdateTransaction.php',
    'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php',
    'DifferentialRevisionVoidTransaction' => 'applications/differential/xaction/DifferentialRevisionVoidTransaction.php',
    'DifferentialRevisionWrongStateTransaction' => 'applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php',
@ -5822,6 +5823,7 @@ phutil_register_library_map(array(
    'DifferentialRevisionTitleTransaction' => 'DifferentialRevisionTransactionType',
    'DifferentialRevisionTransactionType' => 'PhabricatorModularTransactionType',
    'DifferentialRevisionUpdateHistoryView' => 'AphrontView',
+    'DifferentialRevisionUpdateTransaction' => 'DifferentialRevisionTransactionType',
    'DifferentialRevisionViewController' => 'DifferentialController',
    'DifferentialRevisionVoidTransaction' => 'DifferentialRevisionTransactionType',
    'DifferentialRevisionWrongStateTransaction' => 'DifferentialRevisionTransactionType',
--- a/src/applications/differential/conduit/DifferentialConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialConduitAPIMethod.php
@ -58,7 +58,7 @@ abstract class DifferentialConduitAPIMethod extends ConduitAPIMethod {

    $xactions = array();
    $xactions[] = array(
-      'type' => DifferentialRevisionEditEngine::KEY_UPDATE,
+      'type' => DifferentialRevisionUpdateTransaction::EDITKEY,
      'value' => $diff->getPHID(),
    );

--- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php
+++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php
@ -7,8 +7,6 @@ final class DifferentialRevisionEditEngine

  const ENGINECONST = 'differential.revision';

-  const KEY_UPDATE = 'update';
-
  const ACTIONGROUP_REVIEW = 'review';
  const ACTIONGROUP_REVISION = 'revision';

@ -123,12 +121,13 @@ final class DifferentialRevisionEditEngine
    $fields = array();

    $fields[] = id(new PhabricatorHandlesEditField())
-      ->setKey(self::KEY_UPDATE)
+      ->setKey(DifferentialRevisionUpdateTransaction::EDITKEY)
      ->setLabel(pht('Update Diff'))
      ->setDescription(pht('New diff to create or update the revision with.'))
      ->setConduitDescription(pht('Create or update a revision with a diff.'))
      ->setConduitTypeDescription(pht('PHID of the diff.'))
-      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+      ->setTransactionType(
+        DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE)
      ->setHandleParameterType(new AphrontPHIDListHTTPParameterType())
      ->setSingleValue($diff_phid)
      ->setIsConduitOnly(!$diff)
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@ -33,7 +33,7 @@ final class DifferentialTransactionEditor
  }

  public function getDiffUpdateTransaction(array $xactions) {
-    $type_update = DifferentialTransaction::TYPE_UPDATE;
+    $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;

    foreach ($xactions as $xaction) {
      if ($xaction->getTransactionType() == $type_update) {
@ -76,7 +76,6 @@ final class DifferentialTransactionEditor
    $types[] = PhabricatorTransactions::TYPE_INLINESTATE;

    $types[] = DifferentialTransaction::TYPE_INLINE;
-    $types[] = DifferentialTransaction::TYPE_UPDATE;

    return $types;
  }
@ -88,12 +87,6 @@ final class DifferentialTransactionEditor
    switch ($xaction->getTransactionType()) {
      case DifferentialTransaction::TYPE_INLINE:
        return null;
-      case DifferentialTransaction::TYPE_UPDATE:
-        if ($this->getIsNewObject()) {
-          return null;
-        } else {
-          return $object->getActiveDiff()->getPHID();
-        }
    }

    return parent::getCustomTransactionOldValue($object, $xaction);
@ -104,8 +97,6 @@ final class DifferentialTransactionEditor
    PhabricatorApplicationTransaction $xaction) {

    switch ($xaction->getTransactionType()) {
-      case DifferentialTransaction::TYPE_UPDATE:
-        return $xaction->getNewValue();
      case DifferentialTransaction::TYPE_INLINE:
        return null;
    }
@ -120,29 +111,6 @@ final class DifferentialTransactionEditor
    switch ($xaction->getTransactionType()) {
      case DifferentialTransaction::TYPE_INLINE:
        return;
-      case DifferentialTransaction::TYPE_UPDATE:
-        if (!$this->getIsCloseByCommit()) {
-          if ($object->isNeedsRevision() ||
-              $object->isChangePlanned() ||
-              $object->isAbandoned()) {
-            $object->setModernRevisionStatus(
-              DifferentialRevisionStatus::NEEDS_REVIEW);
-          }
-        }
-
-        $diff = $this->requireDiff($xaction->getNewValue());
-
-        $this->updateRevisionLineCounts($object, $diff);
-
-        if ($this->repositoryPHIDOverride !== false) {
-          $object->setRepositoryPHID($this->repositoryPHIDOverride);
-        } else {
-          $object->setRepositoryPHID($diff->getRepositoryPHID());
-        }
-
-        $object->attachActiveDiff($diff);
-        $object->setActiveDiffPHID($diff->getPHID());
-        return;
    }

    return parent::applyCustomInternalTransaction($object, $xaction);
@ -196,7 +164,7 @@ final class DifferentialTransactionEditor
      // commit.
    } else {
      switch ($xaction->getTransactionType()) {
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
          $downgrade_rejects = true;
          if (!$is_sticky_accept) {
            // If "sticky accept" is disabled, also downgrade the accepts.
@ -243,7 +211,7 @@ final class DifferentialTransactionEditor

    $is_commandeer = false;
    switch ($xaction->getTransactionType()) {
-      case DifferentialTransaction::TYPE_UPDATE:
+      case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
        if ($this->getIsCloseByCommit()) {
          // Don't bother with any of this if this update is a side effect of
          // commit detection.
@ -293,7 +261,7 @@ final class DifferentialTransactionEditor
    if (!$this->didExpandInlineState) {
      switch ($xaction->getTransactionType()) {
        case PhabricatorTransactions::TYPE_COMMENT:
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
        case DifferentialTransaction::TYPE_INLINE:
          $this->didExpandInlineState = true;

@ -343,45 +311,6 @@ final class DifferentialTransactionEditor
        if ($reply && !$reply->getHasReplies()) {
          $reply->setHasReplies(1)->save();
        }
-        return;
-      case DifferentialTransaction::TYPE_UPDATE:
-        // Now that we're inside the transaction, do a final check.
-        $diff = $this->requireDiff($xaction->getNewValue());
-
-        // TODO: It would be slightly cleaner to just revalidate this
-        // transaction somehow using the same validation code, but that's
-        // not easy to do at the moment.
-
-        $revision_id = $diff->getRevisionID();
-        if ($revision_id && ($revision_id != $object->getID())) {
-          throw new Exception(
-            pht(
-              'Diff is already attached to another revision. You lost '.
-              'a race?'));
-        }
-
-        // TODO: This can race with diff updates, particularly those from
-        // Harbormaster. See discussion in T8650.
-        $diff->setRevisionID($object->getID());
-        $diff->save();
-
-        // If there are any outstanding buildables for this diff, tell
-        // Harbormaster that their containers need to be updated. This is
-        // common, because `arc` creates buildables so it can upload lint
-        // and unit results.
-
-        $buildables = id(new HarbormasterBuildableQuery())
-          ->setViewer(PhabricatorUser::getOmnipotentUser())
-          ->withManualBuildables(false)
-          ->withBuildablePHIDs(array($diff->getPHID()))
-          ->execute();
-        foreach ($buildables as $buildable) {
-          $buildable->sendMessage(
-            $this->getActor(),
-            HarbormasterMessageType::BUILDABLE_CONTAINER,
-            true);
-        }
-
        return;
    }

@ -437,7 +366,7 @@ final class DifferentialTransactionEditor

    foreach ($xactions as $xaction) {
      switch ($xaction->getTransactionType()) {
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
          $diff = $this->requireDiff($xaction->getNewValue(), true);

          // Update these denormalized index tables when we attach a new
@ -554,44 +483,6 @@ final class DifferentialTransactionEditor
    return $xactions;
  }

-
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
-    array $xactions) {
-
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    $config_self_accept_key = 'differential.allow-self-accept';
-    $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
-
-    foreach ($xactions as $xaction) {
-      switch ($type) {
-        case DifferentialTransaction::TYPE_UPDATE:
-          $diff = $this->loadDiff($xaction->getNewValue());
-          if (!$diff) {
-            $errors[] = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Invalid'),
-              pht('The specified diff does not exist.'),
-              $xaction);
-          } else if (($diff->getRevisionID()) &&
-            ($diff->getRevisionID() != $object->getID())) {
-            $errors[] = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Invalid'),
-              pht(
-                'You can not update this revision to the specified diff, '.
-                'because the diff is already attached to another revision.'),
-              $xaction);
-          }
-          break;
-      }
-    }
-
-    return $errors;
-  }
-
  protected function sortTransactions(array $xactions) {
    $xactions = parent::sortTransactions($xactions);

@ -674,7 +565,7 @@ final class DifferentialTransactionEditor
      $action = parent::getMailAction($object, $xactions);

      $strongest = $this->getStrongestAction($object, $xactions);
-      $type_update = DifferentialTransaction::TYPE_UPDATE;
+      $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;
      if ($strongest->getTransactionType() == $type_update) {
        $show_lines = true;
      }
@ -772,7 +663,7 @@ final class DifferentialTransactionEditor
    $update_xaction = null;
    foreach ($xactions as $xaction) {
      switch ($xaction->getTransactionType()) {
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
          $update_xaction = $xaction;
          break;
      }
@ -1053,7 +944,7 @@ final class DifferentialTransactionEditor
    return $query->executeOne();
  }

-  private function requireDiff($phid, $need_changesets = false) {
+  public function requireDiff($phid, $need_changesets = false) {
    $diff = $this->loadDiff($phid, $need_changesets);
    if (!$diff) {
      throw new Exception(pht('Diff "%s" does not exist!', $phid));
@ -1274,7 +1165,7 @@ final class DifferentialTransactionEditor
    $has_update = false;
    $has_commit = false;

-    $type_update = DifferentialTransaction::TYPE_UPDATE;
+    $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;
    foreach ($xactions as $xaction) {
      if ($xaction->getTransactionType() != $type_update) {
        continue;
@ -1721,27 +1612,6 @@ final class DifferentialTransactionEditor
    return true;
  }

-  private function updateRevisionLineCounts(
-    DifferentialRevision $revision,
-    DifferentialDiff $diff) {
-
-    $revision->setLineCount($diff->getLineCount());
-
-    $conn = $revision->establishConnection('r');
-
-    $row = queryfx_one(
-      $conn,
-      'SELECT SUM(addLines) A, SUM(delLines) D FROM %T
-        WHERE diffID = %d',
-      id(new DifferentialChangeset())->getTableName(),
-      $diff->getID());
-
-    if ($row) {
-      $revision->setAddedLineCount((int)$row['A']);
-      $revision->setRemovedLineCount((int)$row['D']);
-    }
-  }
-
  private function requireReviewers(DifferentialRevision $revision) {
    if ($revision->hasAttachedReviewers()) {
      return;
--- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
+++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
@ -278,8 +278,10 @@ final class DifferentialDiffExtractionEngine extends Phobject {
        ->setNewValue($revision->getModernRevisionStatus());
    }

+    $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;
+
    $xactions[] = id(new DifferentialTransaction())
-      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+      ->setTransactionType($type_update)
      ->setIgnoreOnNoEffect(true)
      ->setNewValue($new_diff->getPHID())
      ->setMetadataValue('isCommitUpdate', true);
--- a/src/applications/differential/lipsum/PhabricatorDifferentialRevisionTestDataGenerator.php
+++ b/src/applications/differential/lipsum/PhabricatorDifferentialRevisionTestDataGenerator.php
@ -22,14 +22,14 @@ final class PhabricatorDifferentialRevisionTestDataGenerator
    $revision->setTestPlan($this->generateDescription());

    $diff = $this->generateDiff($author);
+    $type_update = DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE;

    $xactions = array();

    $xactions[] = id(new DifferentialTransaction())
-      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+      ->setTransactionType($type_update)
      ->setNewValue($diff->getPHID());

-
    id(new DifferentialTransactionEditor())
      ->setActor($author)
      ->setContentSource($this->getLipsumContentSource())
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@ -6,7 +6,6 @@ final class DifferentialTransaction
  private $isCommandeerSideEffect;

  const TYPE_INLINE  = 'differential:inline';
-  const TYPE_UPDATE  = 'differential:update';
  const TYPE_ACTION  = 'differential:action';

  const MAILTAG_REVIEWERS      = 'differential-reviewers';
@ -75,18 +74,6 @@ final class DifferentialTransaction
    $new = $this->getNewValue();

    switch ($this->getTransactionType()) {
-      case self::TYPE_UPDATE:
-        // Older versions of this transaction have an ID for the new value,
-        // and/or do not record the old value. Only hide the transaction if
-        // the new value is a PHID, indicating that this is a newer style
-        // transaction.
-        if ($old === null) {
-          if (phid_get_type($new) == DifferentialDiffPHIDType::TYPECONST) {
-            return true;
-          }
-        }
-        break;
-
      case DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE:
        // Don't hide the initial "X requested review: ..." transaction from
        // mail or feed even when it occurs during creation. We need this
@ -139,11 +126,6 @@ final class DifferentialTransaction
          }
        }
        break;
-      case self::TYPE_UPDATE:
-        if ($new) {
-          $phids[] = $new;
-        }
-        break;
    }

    return $phids;
@ -153,8 +135,6 @@ final class DifferentialTransaction
    switch ($this->getTransactionType()) {
      case self::TYPE_ACTION:
        return 3;
-      case self::TYPE_UPDATE:
-        return 2;
    }

    return parent::getActionStrength();
@ -165,13 +145,6 @@ final class DifferentialTransaction
    switch ($this->getTransactionType()) {
      case self::TYPE_INLINE:
        return pht('Commented On');
-      case self::TYPE_UPDATE:
-        $old = $this->getOldValue();
-        if ($old === null) {
-          return pht('Request');
-        } else {
-          return pht('Updated');
-        }
      case self::TYPE_ACTION:
        $map = array(
          DifferentialAction::ACTION_ACCEPT => pht('Accepted'),
@ -209,7 +182,7 @@ final class DifferentialTransaction
            break;
        }
        break;
-      case self::TYPE_UPDATE:
+      case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
        $old = $this->getOldValue();
        if ($old === null) {
          $tags[] = self::MAILTAG_REVIEW_REQUEST;
@ -248,28 +221,6 @@ final class DifferentialTransaction
        return pht(
          '%s added inline comments.',
          $author_handle);
-      case self::TYPE_UPDATE:
-        if ($this->getMetadataValue('isCommitUpdate')) {
-          return pht(
-            'This revision was automatically updated to reflect the '.
-            'committed changes.');
-        } else if ($new) {
-          // TODO: Migrate to PHIDs and use handles here?
-          if (phid_get_type($new) == DifferentialDiffPHIDType::TYPECONST) {
-            return pht(
-              '%s updated this revision to %s.',
-              $author_handle,
-              $this->renderHandleLink($new));
-          } else {
-            return pht(
-              '%s updated this revision.',
-              $author_handle);
-          }
-        } else {
-          return pht(
-            '%s updated this revision.',
-            $author_handle);
-        }
      case self::TYPE_ACTION:
        switch ($new) {
          case DifferentialAction::ACTION_CLOSE:
@ -347,11 +298,6 @@ final class DifferentialTransaction
          '%s added inline comments to %s.',
          $author_link,
          $object_link);
-      case self::TYPE_UPDATE:
-        return pht(
-          '%s updated the diff for %s.',
-          $author_link,
-          $object_link);
      case self::TYPE_ACTION:
        switch ($new) {
          case DifferentialAction::ACTION_ACCEPT:
@ -462,8 +408,6 @@ final class DifferentialTransaction
    switch ($this->getTransactionType()) {
      case self::TYPE_INLINE:
        return 'fa-comment';
-      case self::TYPE_UPDATE:
-        return 'fa-refresh';
      case self::TYPE_ACTION:
        switch ($this->getNewValue()) {
          case DifferentialAction::ACTION_CLOSE:
@ -526,8 +470,6 @@ final class DifferentialTransaction

  public function getColor() {
    switch ($this->getTransactionType()) {
-      case self::TYPE_UPDATE:
-        return PhabricatorTransactions::COLOR_SKY;
      case self::TYPE_ACTION:
        switch ($this->getNewValue()) {
          case DifferentialAction::ACTION_CLOSE:
--- a/src/applications/differential/xaction/DifferentialRevisionUpdateTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionUpdateTransaction.php
@ -0,0 +1,185 @@
+<?php
+
+final class DifferentialRevisionUpdateTransaction
+  extends DifferentialRevisionTransactionType {
+
+  const TRANSACTIONTYPE = 'differential:update';
+  const EDITKEY = 'update';
+
+  public function generateOldValue($object) {
+    return $object->getActiveDiffPHID();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $should_review = $this->shouldRequestReviewAfterUpdate($object);
+    if ($should_review) {
+      $object->setModernRevisionStatus(
+        DifferentialRevisionStatus::NEEDS_REVIEW);
+    }
+
+    $editor = $this->getEditor();
+    $diff = $editor->requireDiff($value);
+
+    $this->updateRevisionLineCounts($object, $diff);
+
+    $object->setRepositoryPHID($diff->getRepositoryPHID());
+    $object->setActiveDiffPHID($diff->getPHID());
+    $object->attachActiveDiff($diff);
+  }
+
+  private function shouldRequestReviewAfterUpdate($object) {
+    if ($this->isCommitUpdate()) {
+      return false;
+    }
+
+    $should_update =
+      $object->isNeedsRevision() ||
+      $object->isChangePlanned() ||
+      $object->isAbandoned();
+    if ($should_update) {
+      return true;
+    }
+
+    return false;
+  }
+
+  public function applyExternalEffects($object, $value) {
+    $editor = $this->getEditor();
+    $diff = $editor->requireDiff($value);
+
+    // TODO: This can race with diff updates, particularly those from
+    // Harbormaster. See discussion in T8650.
+    $diff->setRevisionID($object->getID());
+    $diff->save();
+
+    // If there are any outstanding buildables for this diff, tell
+    // Harbormaster that their containers need to be updated. This is
+    // common, because `arc` creates buildables so it can upload lint
+    // and unit results.
+
+    $buildables = id(new HarbormasterBuildableQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withManualBuildables(false)
+      ->withBuildablePHIDs(array($diff->getPHID()))
+      ->execute();
+    foreach ($buildables as $buildable) {
+      $buildable->sendMessage(
+        $this->getActor(),
+        HarbormasterMessageType::BUILDABLE_CONTAINER,
+        true);
+    }
+  }
+
+  public function getColor() {
+    return 'sky';
+  }
+
+  public function getIcon() {
+    return 'fa-refresh';
+  }
+
+  public function getActionName() {
+    if ($this->isCreateTransaction()) {
+      return pht('Request');
+    } else {
+      return pht('Updated');
+    }
+  }
+
+  public function getActionStrength() {
+    return 2;
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    if ($this->isCommitUpdate()) {
+      return pht(
+        'This revision was automatically updated to reflect the '.
+        'committed changes.');
+    }
+
+    // NOTE: Very, very old update transactions did not have a new value or
+    // did not use a diff PHID as a new value. This was changed years ago,
+    // but wasn't migrated. We might consider migrating if this causes issues.
+
+    return pht(
+      '%s updated this revision to %s.',
+      $this->renderAuthor(),
+      $this->renderNewHandle());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the diff for %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $diff_phid = null;
+    foreach ($xactions as $xaction) {
+      $diff_phid = $xaction->getNewValue();
+
+      $diff = id(new DifferentialDiffQuery())
+        ->withPHIDs(array($diff_phid))
+        ->setViewer($this->getActor())
+        ->executeOne();
+      if (!$diff) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Specified diff ("%s") does not exist.',
+            $diff_phid),
+          $xaction);
+        continue;
+      }
+
+      if ($diff->getRevisionID()) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'You can not update this revision with the specified diff ("%s") '.
+            'because the diff is already attached to another revision.',
+            $diff_phid),
+          $xaction);
+        continue;
+      }
+    }
+
+    if (!$diff_phid && !$object->getActiveDiffPHID()) {
+      $errors[] = $this->newInvalidError(
+        pht(
+          'You must specify an initial diff when creating a revision.'));
+    }
+
+    return $errors;
+  }
+
+  public function isCommitUpdate() {
+    return (bool)$this->getMetadataValue('isCommitUpdate');
+  }
+
+  private function updateRevisionLineCounts(
+    DifferentialRevision $revision,
+    DifferentialDiff $diff) {
+
+    $revision->setLineCount($diff->getLineCount());
+
+    $conn = $revision->establishConnection('r');
+
+    $row = queryfx_one(
+      $conn,
+      'SELECT SUM(addLines) A, SUM(delLines) D FROM %T
+        WHERE diffID = %d',
+      id(new DifferentialChangeset())->getTableName(),
+      $diff->getID());
+
+    if ($row) {
+      $revision->setAddedLineCount((int)$row['A']);
+      $revision->setRemovedLineCount((int)$row['D']);
+    }
+  }
+
+}
--- a/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php
+++ b/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php
@ -37,7 +37,7 @@ final class ReleephDiffChurnFieldSpecification
        case PhabricatorTransactions::TYPE_COMMENT:
          $comments++;
          break;
-        case DifferentialTransaction::TYPE_UPDATE:
+        case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
          $updates++;
          break;
        case DifferentialTransaction::TYPE_ACTION: