Page MenuHomePhabricator
Files F15409137

D10489.id25694.diff
No OneTemporary
Actions

D10489.id25694.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -330,6 +330,7 @@
'DifferentialReviewersField' => 'applications/differential/customfield/DifferentialReviewersField.php',
'DifferentialReviewersView' => 'applications/differential/view/DifferentialReviewersView.php',
'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php',
+ 'DifferentialRevisionCloseDetailsController' => 'applications/differential/controller/DifferentialRevisionCloseDetailsController.php',
'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php',
'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php',
'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php',
@@ -3223,6 +3224,7 @@
'PhabricatorDestructibleInterface',
'PhabricatorProjectInterface',
),
+ 'DifferentialRevisionCloseDetailsController' => 'DifferentialController',
'DifferentialRevisionDetailView' => 'AphrontView',
'DifferentialRevisionEditController' => 'DifferentialController',
'DifferentialRevisionHasTaskEdgeType' => 'PhabricatorEdgeType',
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
@@ -67,6 +67,8 @@
=> 'DifferentialRevisionEditController',
'revision/land/(?:(?P<id>[1-9]\d*))/(?P<strategy>[^/]+)/'
=> 'DifferentialRevisionLandController',
+ 'revision/closedetails/(?P<phid>[^/]+)/'
+ => 'DifferentialRevisionCloseDetailsController',
'comment/' => array(
'preview/(?P<id>[1-9]\d*)/' => 'DifferentialCommentPreviewController',
'save/(?P<id>[1-9]\d*)/' => 'DifferentialCommentSaveController',
diff --git a/src/applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php
--- a/src/applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php
@@ -81,9 +81,20 @@
}
}
+ // grab some extra information about the Differential Revision: field...
+ $revision_id_field = new DifferentialRevisionIDField();
+ $revision_id_value = idx(
+ $corpus_map,
+ $revision_id_field->getFieldKeyForConduit());
+ $revision_id_valid_domain = PhabricatorEnv::getProductionURI('');
+
return array(
'errors' => $this->errors,
'fields' => $values,
+ 'revisionIDFieldInfo' => array(
+ 'value' => $revision_id_value,
+ 'validDomain' => $revision_id_valid_domain,
+ ),
);
}
diff --git a/src/applications/differential/controller/DifferentialRevisionCloseDetailsController.php b/src/applications/differential/controller/DifferentialRevisionCloseDetailsController.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/controller/DifferentialRevisionCloseDetailsController.php
@@ -0,0 +1,113 @@
+<?php
+
+final class DifferentialRevisionCloseDetailsController
+ extends DifferentialController {
+
+ private $phid;
+
+ public function willProcessRequest(array $data) {
+ $this->phid = idx($data, 'phid');
+ }
+
+ public function processRequest() {
+ $request = $this->getRequest();
+
+ $viewer = $request->getUser();
+ $xaction_phid = $this->phid;
+
+ $xaction = id(new PhabricatorObjectQuery())
+ ->withPHIDs(array($xaction_phid))
+ ->setViewer($viewer)
+ ->executeOne();
+ if (!$xaction) {
+ return new Aphront404Response();
+ }
+
+ $obj_phid = $xaction->getObjectPHID();
+ $obj_handle = id(new PhabricatorHandleQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($obj_phid))
+ ->executeOne();
+
+ $body = $this->getRevisionMatchExplanation(
+ $xaction->getMetadataValue('revisionMatchData'),
+ $obj_handle);
+
+ $dialog = id(new AphrontDialogView())
+ ->setUser($viewer)
+ ->setTitle(pht('Commit Close Explanation'))
+ ->appendParagraph($body)
+ ->addCancelButton($obj_handle->getURI());
+
+ return id(new AphrontDialogResponse())->setDialog($dialog);
+ }
+
+ private function getRevisionMatchExplanation(
+ $revision_match_data,
+ PhabricatorObjectHandle $obj_handle) {
+
+ if (!$revision_match_data) {
+ return pht(
+ 'This commit was made before this feature was built and thus this '.
+ 'information is unavailable.');
+ }
+
+ $body_why = array();
+ if ($revision_match_data['usedURI']) {
+ return pht(
+ 'We found a "Differential Revision" field with value "%s" in the '.
+ 'commit message, and the domain on the URI matches this install, so '.
+ 'we linked this commit to %s.',
+ $revision_match_data['foundURI'],
+ phutil_tag(
+ 'a',
+ array(
+ 'href' => $obj_handle->getURI(),),
+ $obj_handle->getName()));
+ } else if ($revision_match_data['foundURI']) {
+ $body_why[] = pht(
+ 'We found a "Differential Revision" field with value "%s" in the '.
+ 'commit message, but the domain on this URI did not match the '.
+ 'configured domain for this install, "%s", so we ignored it under '.
+ 'the assumption that it refers to some third-party revision.',
+ $revision_match_data['foundURI'],
+ $revision_match_data['validDomain']);
+ } else {
+ $body_why[] = pht(
+ 'We didn\'t find a "Differential Revision" field in the commit '.
+ 'message.');
+ }
+
+ switch ($revision_match_data['matchHashType']) {
+ case ArcanistDifferentialRevisionHash::HASH_GIT_TREE:
+ $hash_info = true;
+ $hash_type = 'tree';
+ break;
+ case ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT:
+ case ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT:
+ $hash_info = true;
+ $hash_type = 'commit';
+ break;
+ default:
+ $hash_info = false;
+ break;
+ }
+ if ($hash_info) {
+ $diff_link = phutil_tag(
+ 'a',
+ array(
+ 'href' => $obj_handle->getURI(),),
+ $obj_handle->getName());
+ $body_why = pht(
+ 'This commit and the active diff of %s had the same %s hash '.
+ '(%s) so we linked this commit to %s.',
+ $diff_link,
+ $hash_type,
+ $revision_match_data['matchHashValue'],
+ $diff_link);
+ }
+
+ return phutil_implode_html("\n", $body_why);
+
+ }
+}
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -306,6 +306,22 @@
return parent::getTitle();
}
+ public function renderExtraInformationLink() {
+ if ($this->getMetadataValue('revisionMatchData')) {
+ $details_href =
+ '/differential/revision/closedetails/'.$this->getPHID().'/';
+ $details_link = javelin_tag(
+ 'a',
+ array(
+ 'href' => $details_href,
+ 'sigil' => 'workflow',
+ ),
+ pht('Explain Why'));
+ return $details_link;
+ }
+ return parent::renderExtraInformationLink();
+ }
+
public function getTitleForFeed(PhabricatorFeedStory $story) {
$author_phid = $this->getAuthorPHID();
$object_phid = $this->getObjectPHID();
diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php
--- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php
+++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php
@@ -4,12 +4,28 @@
extends DiffusionLowLevelQuery {
private $ref;
+ private $revisionMatchData = array(
+ 'usedURI' => null,
+ 'foundURI' => null,
+ 'validDomain' => null,
+ 'matchHashType' => null,
+ 'matchHashValue' => null,
+ );
public function withCommitRef(DiffusionCommitRef $ref) {
$this->ref = $ref;
return $this;
}
+ public function getRevisionMatchData() {
+ return $this->revisionMatchData;
+ }
+
+ private function setRevisionMatchData($key, $value) {
+ $this->revisionMatchData[$key] = $value;
+ return $this;
+ }
+
public function executeQuery() {
$ref = $this->ref;
$message = $ref->getMessage();
@@ -25,10 +41,21 @@
->execute();
$fields = $result['fields'];
+ $revision_id = idx($fields, 'revisionID');
+ if ($revision_id) {
+ $this->setRevisionMatchData('usedURI', true);
+ } else {
+ $this->setRevisionMatchData('usedURI', false);
+ }
+ $revision_id_info = $result['revisionIDFieldInfo'];
+ $this->setRevisionMatchData('foundURI', $revision_id_info['value']);
+ $this->setRevisionMatchData(
+ 'validDomain',
+ $revision_id_info['validDomain']);
+
// If there is no "Differential Revision:" field in the message, try to
// identify the revision by doing a hash lookup.
- $revision_id = idx($fields, 'revisionID');
if (!$revision_id && $hashes) {
$hash_list = array();
foreach ($hashes as $hash) {
@@ -36,12 +63,33 @@
}
$revisions = id(new DifferentialRevisionQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->needHashes(true)
->withCommitHashes($hash_list)
->execute();
if (!empty($revisions)) {
$revision = $this->pickBestRevision($revisions);
$fields['revisionID'] = $revision->getID();
+ $revision_hashes = $revision->getHashes();
+ $revision_hashes = mpull($revision_hashes, 'getHashType');
+ // sort the hashes in the order the mighty
+ // @{class:ArcanstDifferentialRevisionHash} does; probably unnecessary
+ // but should future proof things nicely.
+ $revision_hashes = array_select_keys(
+ $revision_hashes,
+ ArcanistDifferentialRevisionHash::getTypes());
+ foreach ($hashes as $hash) {
+ $revision_hash = $revision_hashes[$hash->getHashType()];
+ if ($revision_hash->getHashValue() == $hash->getHashValue()) {
+ $this->setRevisionMatchData(
+ 'matchHashType',
+ $hash->getHashType());
+ $this->setRevisionMatchData(
+ 'matchHashValue',
+ $hash->getHashValue());
+ break;
+ }
+ }
}
}
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -53,11 +53,12 @@
$differential_app = 'PhabricatorDifferentialApplication';
$revision_id = null;
+ $low_level_query = null;
if (PhabricatorApplication::isClassInstalled($differential_app)) {
- $field_values = id(new DiffusionLowLevelCommitFieldsQuery())
+ $low_level_query = id(new DiffusionLowLevelCommitFieldsQuery())
->setRepository($repository)
- ->withCommitRef($ref)
- ->execute();
+ ->withCommitRef($ref);
+ $field_values = $low_level_query->execute();
$revision_id = idx($field_values, 'revisionID');
if (!empty($field_values['reviewedByPHIDs'])) {
@@ -160,9 +161,15 @@
$commit_close_xaction->setMetadataValue(
'authorName',
$data->getAuthorName());
- $commit_close_xaction->setMetadataValue(
- 'commitHashes',
- $hashes);
+
+ if ($low_level_query) {
+ $commit_close_xaction->setMetadataValue(
+ 'revisionMatchData',
+ $low_level_query->getRevisionMatchData());
+ $data->setCommitDetail(
+ 'revisionMatchData',
+ $low_level_query->getRevisionMatchData());
+ }
$diff = $this->generateFinalDiff($revision, $acting_as_phid);

File Metadata

Mime Type
text/plain
Expires
Thu, Mar 20, 3:23 AM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7336395
Default Alt Text
D10489.id25694.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility