@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Use repository identities, not denormalized strings, to identify authors for "Revision closed by commit X" stories
Summary:
Ref T12164. Ref T13276. Currently, the parsing pipeline copies the author and committer names and PHIDs into the transcaction record as metadata. They are then rendered directly from the metadata.
This makes planned changes to the parsing pipeline (to prevent races when multiple commits matching a single revision are pushed simultaneously) more difficult, and generally won't work with repository identities.
Instead, load the commit and use its identities.
Test Plan: Loaded a revision, saw the same story rendering for a "Closed by commit" story.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13276, T12164
Differential Revision: https://secure.phabricator.com/D20418
+29
-52
+21
-29
src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php
+21
-29
src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php
···
83
83
}
84
84
85
85
public function getTitle() {
86
-
if (!$this->getMetadataValue('isCommitClose')) {
87
-
return pht(
88
-
'%s closed this revision.',
89
-
$this->renderAuthor());
90
-
}
91
-
92
86
$commit_phid = $this->getMetadataValue('commitPHID');
93
-
$committer_phid = $this->getMetadataValue('committerPHID');
94
-
$author_phid = $this->getMetadataValue('authorPHID');
95
-
96
-
if ($committer_phid) {
97
-
$committer_name = $this->renderHandle($committer_phid);
87
+
if ($commit_phid) {
88
+
$commit = id(new DiffusionCommitQuery())
89
+
->setViewer($this->getViewer())
90
+
->withPHIDs(array($commit_phid))
91
+
->needIdentities(true)
92
+
->executeOne();
98
93
} else {
99
-
$committer_name = $this->getMetadataValue('committerName');
94
+
$commit = null;
100
95
}
101
96
102
-
if ($author_phid) {
103
-
$author_name = $this->renderHandle($author_phid);
104
-
} else {
105
-
$author_name = $this->getMetadatavalue('authorName');
97
+
if (!$commit) {
98
+
return pht(
99
+
'%s closed this revision.',
100
+
$this->renderAuthor());
106
101
}
107
102
108
-
$same_phid =
109
-
strlen($committer_phid) &&
110
-
strlen($author_phid) &&
111
-
($committer_phid == $author_phid);
103
+
$author_phid = $commit->getAuthorDisplayPHID();
104
+
$committer_phid = $commit->getCommitterDisplayPHID();
112
105
113
-
$same_name =
114
-
!strlen($committer_phid) &&
115
-
!strlen($author_phid) &&
116
-
($committer_name == $author_name);
117
-
118
-
if ($same_name || $same_phid) {
106
+
if (!$author_phid) {
107
+
return pht(
108
+
'Closed by commit %s.',
109
+
$this->renderHandle($commit_phid));
110
+
} else if (!$committer_phid || ($committer_phid === $author_phid)) {
119
111
return pht(
120
112
'Closed by commit %s (authored by %s).',
121
113
$this->renderHandle($commit_phid),
122
-
$author_name);
114
+
$this->renderHandle($author_phid));
123
115
} else {
124
116
return pht(
125
117
'Closed by commit %s (authored by %s, committed by %s).',
126
118
$this->renderHandle($commit_phid),
127
-
$author_name,
128
-
$committer_name);
119
+
$this->renderHandle($author_phid),
120
+
$this->renderHandle($committer_phid));
129
121
}
130
122
}
131
123
+7
-9
src/applications/repository/management/PhabricatorRepositoryManagementUnpublishWorkflow.php
+7
-9
src/applications/repository/management/PhabricatorRepositoryManagementUnpublishWorkflow.php
···
241
241
if ($xactions) {
242
242
foreach ($xactions as $xaction) {
243
243
$metadata = $xaction->getMetadata();
244
-
if (idx($metadata, 'isCommitClose')) {
245
-
if (idx($metadata, 'commitPHID') === $src->getPHID()) {
246
-
echo tsprintf(
247
-
"%s\n",
248
-
pht(
249
-
'MANUAL Revision "%s" was likely closed improperly by "%s".',
250
-
$dst->getMonogram(),
251
-
$src->getMonogram()));
252
-
}
244
+
if (idx($metadata, 'commitPHID') === $src->getPHID()) {
245
+
echo tsprintf(
246
+
"%s\n",
247
+
pht(
248
+
'MANUAL Revision "%s" was likely closed improperly by "%s".',
249
+
$dst->getMonogram(),
250
+
$src->getMonogram()));
253
251
}
254
252
}
255
253
}
+1
-14
src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+1
-14
src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
···
244
244
245
245
$commit_close_xaction = id(new DifferentialTransaction())
246
246
->setTransactionType($type_close)
247
-
->setNewValue(true)
248
-
->setMetadataValue('isCommitClose', true);
247
+
->setNewValue(true);
249
248
250
249
$commit_close_xaction->setMetadataValue(
251
250
'commitPHID',
252
251
$commit->getPHID());
253
-
$commit_close_xaction->setMetadataValue(
254
-
'committerPHID',
255
-
$committer_phid);
256
-
$commit_close_xaction->setMetadataValue(
257
-
'committerName',
258
-
$data->getCommitDetail('committer'));
259
-
$commit_close_xaction->setMetadataValue(
260
-
'authorPHID',
261
-
$author_phid);
262
-
$commit_close_xaction->setMetadataValue(
263
-
'authorName',
264
-
$data->getAuthorName());
265
252
266
253
if ($low_level_query) {
267
254
$commit_close_xaction->setMetadataValue(