Add a "Changes Since Last Action" view to Differential revisions

Summary:
Ref T13151. See PHI616. Fixes T8163.

This adds `/D123/new/`, which shows the changes to the revision since the last timeline action you took.

It also adds a link to this view to diff update emails.

Test Plan:
  - Followed this link with a recent comment and no touches since update, ended up with sensible diff selections.
  - Updated revision, generated email, saw an appropriate link.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13151, T8163

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

src/applications/differential/application/PhabricatorDifferentialApplication.php

@@ -43,7 +43,10 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication {
 
   public function getRoutes() {
     return array(
-      '/D(?P<id>[1-9]\d*)' => 'DifferentialRevisionViewController',
+      '/D(?P<id>[1-9]\d*)' => array(
+        '' => 'DifferentialRevisionViewController',
+        '/(?P<filter>new)/' => 'DifferentialRevisionViewController',
+      ),
       '/differential/' => array(
         '(?:query/(?P<queryKey>[^/]+)/)?'
           => 'DifferentialRevisionListController',

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -6,6 +6,7 @@ final class DifferentialRevisionViewController
   private $revisionID;
   private $changesetCount;
   private $hiddenChangesets;
+  private $warnings = array();
 
   public function shouldAllowPublic() {
     return true;
@@ -68,9 +69,17 @@ final class DifferentialRevisionViewController
 
     $revision->attachActiveDiff(last($diffs));
 
-    $diff_vs = $request->getInt('vs');
+    $diff_vs = $this->getOldDiffID($revision, $diffs);
-    $target_id = $request->getInt('id');
+    if ($diff_vs instanceof AphrontResponse) {
-    $target = idx($diffs, $target_id, end($diffs));
+      return $diff_vs;
+    }
+
+    $target_id = $this->getNewDiffID($revision, $diffs);
+    if ($target_id instanceof AphrontResponse) {
+      return $target_id;
+    }
+
+    $target = $diffs[$target_id];
 
     $target_manual = $target;
     if (!$target_id) {
@@ -81,10 +90,6 @@ final class DifferentialRevisionViewController
       }
     }
 
-    if (empty($diffs[$diff_vs])) {
-      $diff_vs = null;
-    }
-
     $repository = null;
     $repository_phid = $target->getRepositoryPHID();
     if ($repository_phid) {
@@ -170,7 +175,7 @@ final class DifferentialRevisionViewController
     }
 
     $handles = $this->loadViewerHandles($object_phids);
-    $warnings = array();
+    $warnings = $this->warnings;
 
     $request_uri = $request->getRequestURI();
 
@@ -1312,4 +1317,133 @@ final class DifferentialRevisionViewController
       ->setShowViewAll(true);
   }
 
+  private function getOldDiffID(DifferentialRevision $revision, array $diffs) {
+    assert_instances_of($diffs, 'DifferentialDiff');
+    $request = $this->getRequest();
+
+    $diffs = mpull($diffs, null, 'getID');
+
+    $is_new = ($request->getURIData('filter') === 'new');
+    $old_id = $request->getInt('vs');
+
+    // This is ambiguous, so just 404 rather than trying to figure out what
+    // the user expects.
+    if ($is_new && $old_id) {
+      return new Aphront404Response();
+    }
+
+    if ($is_new) {
+      $viewer = $this->getViewer();
+
+      $xactions = id(new DifferentialTransactionQuery())
+        ->setViewer($viewer)
+        ->withObjectPHIDs(array($revision->getPHID()))
+        ->withAuthorPHIDs(array($viewer->getPHID()))
+        ->setOrder('newest')
+        ->setLimit(1)
+        ->execute();
+
+      if (!$xactions) {
+        $this->warnings[] = id(new PHUIInfoView())
+          ->setTitle(pht('No Actions'))
+          ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
+          ->appendChild(
+            pht(
+              'Showing all changes because you have never taken an '.
+              'action on this revision.'));
+      } else {
+        $xaction = head($xactions);
+
+        // Find the transactions which updated this revision. We want to
+        // figure out which diff was active when you last took an action.
+        $updates = id(new DifferentialTransactionQuery())
+          ->setViewer($viewer)
+          ->withObjectPHIDs(array($revision->getPHID()))
+          ->withTransactionTypes(
+            array(
+              DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE,
+            ))
+          ->setOrder('oldest')
+          ->execute();
+
+        // Sort the diffs into two buckets: those older than your last action
+        // and those newer than your last action.
+        $older = array();
+        $newer = array();
+        foreach ($updates as $update) {
+          // If you updated the revision with "arc diff", try to count that
+          // update as "before your last action".
+          if ($update->getDateCreated() <= $xaction->getDateCreated()) {
+            $older[] = $update->getNewValue();
+          } else {
+            $newer[] = $update->getNewValue();
+          }
+        }
+
+        if (!$newer) {
+          $this->warnings[] = id(new PHUIInfoView())
+            ->setTitle(pht('No Recent Updates'))
+            ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
+            ->appendChild(
+              pht(
+                'Showing all changes because the diff for this revision '.
+                'has not been updated since your last action.'));
+        } else {
+          $older = array_fuse($older);
+
+          // Find the most recent diff from before the last action.
+          $old = null;
+          foreach ($diffs as $diff) {
+            if (!isset($older[$diff->getPHID()])) {
+              break;
+            }
+
+            $old = $diff;
+          }
+
+          // It's possible we may not find such a diff: transactions may have
+          // been removed from the database, for example. If we miss, just
+          // fail into some reasonable state since 404'ing would be perplexing.
+          if ($old) {
+            $this->warnings[] = id(new PHUIInfoView())
+              ->setTitle(pht('New Changes Shown'))
+              ->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
+              ->appendChild(
+                pht(
+                  'Showing changes since the last action you took on this '.
+                  'revision.'));
+
+            $old_id = $old->getID();
+          }
+        }
+      }
+    }
+
+    if (isset($diffs[$old_id])) {
+      return $old_id;
+    }
+
+    return null;
+  }
+
+  private function getNewDiffID(DifferentialRevision $revision, array $diffs) {
+    assert_instances_of($diffs, 'DifferentialDiff');
+    $request = $this->getRequest();
+
+    $diffs = mpull($diffs, null, 'getID');
+
+    $is_new = ($request->getURIData('filter') === 'new');
+    $new_id = $request->getInt('id');
+
+    if ($is_new && $new_id) {
+      return new Aphront404Response();
+    }
+
+    if (isset($diffs[$new_id])) {
+      return $new_id;
+    }
+
+    return (int)last_key($diffs);
+  }
+
 }

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -624,7 +624,9 @@ final class DifferentialTransactionEditor
     $body = new PhabricatorMetaMTAMailBody();
     $body->setViewer($this->requireActor());
 
-    $revision_uri = PhabricatorEnv::getProductionURI('/D'.$object->getID());
+    $revision_uri = $object->getURI();
+    $revision_uri = PhabricatorEnv::getProductionURI($revision_uri);
+    $new_uri = $revision_uri.'/new/';
 
     $this->addHeadersAndCommentsToMailBody(
       $body,
@@ -645,19 +647,6 @@ final class DifferentialTransactionEditor
       $this->appendInlineCommentsForMail($object, $inlines, $body);
     }
 
-    $changed_uri = $this->getChangedPriorToCommitURI();
-    if ($changed_uri) {
-      $body->addLinkSection(
-        pht('CHANGED PRIOR TO COMMIT'),
-        $changed_uri);
-    }
-
-    $this->addCustomFieldsToMailBody($body, $object, $xactions);
-
-    $body->addLinkSection(
-      pht('REVISION DETAIL'),
-      $revision_uri);
-
     $update_xaction = null;
     foreach ($xactions as $xaction) {
       switch ($xaction->getTransactionType()) {
@@ -669,7 +658,28 @@ final class DifferentialTransactionEditor
 
     if ($update_xaction) {
       $diff = $this->requireDiff($update_xaction->getNewValue(), true);
+    } else {
+      $diff = null;
+    }
 
+    $changed_uri = $this->getChangedPriorToCommitURI();
+    if ($changed_uri) {
+      $body->addLinkSection(
+        pht('CHANGED PRIOR TO COMMIT'),
+        $changed_uri);
+    }
+
+    $this->addCustomFieldsToMailBody($body, $object, $xactions);
+
+    if (!$this->getIsNewObject()) {
+      $body->addLinkSection(pht('CHANGES SINCE LAST ACTION'), $new_uri);
+    }
+
+    $body->addLinkSection(
+      pht('REVISION DETAIL'),
+      $revision_uri);
+
+    if ($update_xaction) {
       $body->addTextSection(
         pht('AFFECTED FILES'),
         $this->renderAffectedFilesForMail($diff));

src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php

@@ -307,7 +307,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
     $content = phabricator_form(
       $this->getUser(),
       array(
-        'action' => '#toc',
+        'action' => '/D'.$revision_id.'#toc',
       ),
       array(
         $table,