Page MenuHomePhabricator
Files F15336247

D19541.diff
No OneTemporary
Actions

D19541.diff
View Options

diff --git a/src/applications/differential/application/PhabricatorDifferentialApplication.php b/src/applications/differential/application/PhabricatorDifferentialApplication.php
--- a/src/applications/differential/application/PhabricatorDifferentialApplication.php
+++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php
@@ -43,7 +43,10 @@
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',
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -6,6 +6,7 @@
private $revisionID;
private $changesetCount;
private $hiddenChangesets;
+ private $warnings = array();
public function shouldAllowPublic() {
return true;
@@ -68,9 +69,17 @@
$revision->attachActiveDiff(last($diffs));
- $diff_vs = $request->getInt('vs');
- $target_id = $request->getInt('id');
- $target = idx($diffs, $target_id, end($diffs));
+ $diff_vs = $this->getOldDiffID($revision, $diffs);
+ if ($diff_vs instanceof AphrontResponse) {
+ 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 @@
}
}
- if (empty($diffs[$diff_vs])) {
- $diff_vs = null;
- }
-
$repository = null;
$repository_phid = $target->getRepositoryPHID();
if ($repository_phid) {
@@ -170,7 +175,7 @@
}
$handles = $this->loadViewerHandles($object_phids);
- $warnings = array();
+ $warnings = $this->warnings;
$request_uri = $request->getRequestURI();
@@ -1312,4 +1317,133 @@
->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);
+ }
+
}
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -624,7 +624,9 @@
$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,6 +647,21 @@
$this->appendInlineCommentsForMail($object, $inlines, $body);
}
+ $update_xaction = null;
+ foreach ($xactions as $xaction) {
+ switch ($xaction->getTransactionType()) {
+ case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
+ $update_xaction = $xaction;
+ break;
+ }
+ }
+
+ if ($update_xaction) {
+ $diff = $this->requireDiff($update_xaction->getNewValue(), true);
+ } else {
+ $diff = null;
+ }
+
$changed_uri = $this->getChangedPriorToCommitURI();
if ($changed_uri) {
$body->addLinkSection(
@@ -654,22 +671,15 @@
$this->addCustomFieldsToMailBody($body, $object, $xactions);
+ if (!$this->getIsNewObject()) {
+ $body->addLinkSection(pht('CHANGES SINCE LAST ACTION'), $new_uri);
+ }
+
$body->addLinkSection(
pht('REVISION DETAIL'),
$revision_uri);
- $update_xaction = null;
- foreach ($xactions as $xaction) {
- switch ($xaction->getTransactionType()) {
- case DifferentialRevisionUpdateTransaction::TRANSACTIONTYPE:
- $update_xaction = $xaction;
- break;
- }
- }
-
if ($update_xaction) {
- $diff = $this->requireDiff($update_xaction->getNewValue(), true);
-
$body->addTextSection(
pht('AFFECTED FILES'),
$this->renderAffectedFilesForMail($diff));
diff --git a/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php b/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php
--- a/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php
+++ b/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php
@@ -307,7 +307,7 @@
$content = phabricator_form(
$this->getUser(),
array(
- 'action' => '#toc',
+ 'action' => '/D'.$revision_id.'#toc',
),
array(
$table,

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 9, 6:34 PM (2 d, 14 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7227772
Default Alt Text
D19541.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility