Page MenuHomePhabricator

D18978.diff
No OneTemporary

D18978.diff

diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -358,9 +358,17 @@
array $changes,
PhutilMarkupEngine $engine) {
- // we are only really trying to find unmentionable phids here...
- // don't bother with this outside initial commit (i.e. create)
- // transaction
+ $actor = $this->getActor();
+ $result = array();
+
+ // Some interactions (like "Fixes Txxx" interacting with Maniphest) have
+ // already been processed, so we're only re-parsing them here to avoid
+ // generating an extra redundant mention. Other interactions are being
+ // processed for the first time.
+
+ // We're only recognizing magic in the commit message itself, not in
+ // audit comments.
+
$is_commit = false;
foreach ($xactions as $xaction) {
switch ($xaction->getTransactionType()) {
@@ -370,8 +378,6 @@
}
}
- // "result" is always an array....
- $result = array();
if (!$is_commit) {
return $result;
}
@@ -403,6 +409,46 @@
->withNames($monograms)
->execute();
$phid_map[] = mpull($objects, 'getPHID', 'getPHID');
+
+
+ $reverts_refs = id(new DifferentialCustomFieldRevertsParser())
+ ->parseCorpus($huge_block);
+ $reverts = array_mergev(ipull($reverts_refs, 'monograms'));
+ if ($reverts) {
+ // Only allow commits to revert other commits in the same repository.
+ $reverted_commits = id(new DiffusionCommitQuery())
+ ->setViewer($actor)
+ ->withRepository($object->getRepository())
+ ->withIdentifiers($reverts)
+ ->execute();
+
+ $reverted_revisions = id(new PhabricatorObjectQuery())
+ ->setViewer($actor)
+ ->withNames($reverts)
+ ->withTypes(
+ array(
+ DifferentialRevisionPHIDType::TYPECONST,
+ ))
+ ->execute();
+
+ $reverted_phids =
+ mpull($reverted_commits, 'getPHID', 'getPHID') +
+ mpull($reverted_revisions, 'getPHID', 'getPHID');
+
+ // NOTE: Skip any write attempts if a user cleverly implies a commit
+ // reverts itself, although this would be exceptionally clever in Git
+ // or Mercurial.
+ unset($reverted_phids[$object->getPHID()]);
+
+ $reverts_edge = DiffusionCommitRevertsCommitEdgeType::EDGECONST;
+ $result[] = id(new PhabricatorAuditTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue('edge:type', $reverts_edge)
+ ->setNewValue(array('+' => $reverted_phids));
+
+ $phid_map[] = $reverted_phids;
+ }
+
$phid_map = array_mergev($phid_map);
$this->setUnmentionablePHIDMap($phid_map);
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
@@ -919,7 +919,44 @@
}
}
- $this->setUnmentionablePHIDMap(array_merge($task_phids, $rev_phids));
+ $revert_refs = id(new DifferentialCustomFieldRevertsParser())
+ ->parseCorpus($content_block);
+
+ $revert_monograms = array();
+ foreach ($revert_refs as $match) {
+ foreach ($match['monograms'] as $monogram) {
+ $revert_monograms[] = $monogram;
+ }
+ }
+
+ if ($revert_monograms) {
+ $revert_objects = id(new PhabricatorObjectQuery())
+ ->setViewer($this->getActor())
+ ->withNames($revert_monograms)
+ ->withTypes(
+ array(
+ DifferentialRevisionPHIDType::TYPECONST,
+ PhabricatorRepositoryCommitPHIDType::TYPECONST,
+ ))
+ ->execute();
+
+ $revert_phids = mpull($revert_objects, 'getPHID', 'getPHID');
+
+ // Don't let an object revert itself, although other silly stuff like
+ // cycles of objects reverting each other is not prevented.
+ unset($revert_phids[$object->getPHID()]);
+
+ $revert_type = DiffusionCommitRevertsCommitEdgeType::EDGECONST;
+ $edges[$revert_type] = $revert_phids;
+ } else {
+ $revert_phids = array();
+ }
+
+ $this->setUnmentionablePHIDMap(
+ array_merge(
+ $task_phids,
+ $rev_phids,
+ $revert_phids));
$result = array();
foreach ($edges as $type => $specs) {
diff --git a/src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php b/src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php
--- a/src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php
+++ b/src/applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php
@@ -19,7 +19,7 @@
$add_edges) {
return pht(
- '%s added %s reverting commit(s): %s.',
+ '%s added %s reverting change(s): %s.',
$actor,
$add_count,
$add_edges);
@@ -31,7 +31,7 @@
$rem_edges) {
return pht(
- '%s removed %s reverting commit(s): %s.',
+ '%s removed %s reverting change(s): %s.',
$actor,
$rem_count,
$rem_edges);
@@ -46,7 +46,7 @@
$rem_edges) {
return pht(
- '%s edited reverting commit(s), added %s: %s; removed %s: %s.',
+ '%s edited reverting change(s), added %s: %s; removed %s: %s.',
$actor,
$add_count,
$add_edges,
@@ -61,7 +61,7 @@
$add_edges) {
return pht(
- '%s added %s reverting commit(s) for %s: %s.',
+ '%s added %s reverting change(s) for %s: %s.',
$actor,
$add_count,
$object,
@@ -75,7 +75,7 @@
$rem_edges) {
return pht(
- '%s removed %s reverting commit(s) for %s: %s.',
+ '%s removed %s reverting change(s) for %s: %s.',
$actor,
$rem_count,
$object,
@@ -92,7 +92,7 @@
$rem_edges) {
return pht(
- '%s edited reverting commit(s) for %s, added %s: %s; removed %s: %s.',
+ '%s edited reverting change(s) for %s, added %s: %s; removed %s: %s.',
$actor,
$object,
$add_count,
diff --git a/src/applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php b/src/applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php
--- a/src/applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php
+++ b/src/applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php
@@ -22,7 +22,7 @@
$add_edges) {
return pht(
- '%s added %s reverted commit(s): %s.',
+ '%s added %s reverted change(s): %s.',
$actor,
$add_count,
$add_edges);
@@ -34,7 +34,7 @@
$rem_edges) {
return pht(
- '%s removed %s reverted commit(s): %s.',
+ '%s removed %s reverted change(s): %s.',
$actor,
$rem_count,
$rem_edges);
@@ -49,7 +49,7 @@
$rem_edges) {
return pht(
- '%s edited reverted commit(s), added %s: %s; removed %s: %s.',
+ '%s edited reverted change(s), added %s: %s; removed %s: %s.',
$actor,
$add_count,
$add_edges,
@@ -64,7 +64,7 @@
$add_edges) {
return pht(
- '%s added %s reverted commit(s) for %s: %s.',
+ '%s added %s reverted change(s) for %s: %s.',
$actor,
$add_count,
$object,
@@ -78,7 +78,7 @@
$rem_edges) {
return pht(
- '%s removed %s reverted commit(s) for %s: %s.',
+ '%s removed %s reverted change(s) for %s: %s.',
$actor,
$rem_count,
$object,
@@ -95,7 +95,7 @@
$rem_edges) {
return pht(
- '%s edited reverted commit(s) for %s, added %s: %s; removed %s: %s.',
+ '%s edited reverted change(s) for %s, added %s: %s; removed %s: %s.',
$actor,
$object,
$add_count,
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
@@ -15,6 +15,7 @@
protected function parseCommit(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
+ $viewer = PhabricatorUser::getOmnipotentUser();
if ($this->shouldSkipImportStep()) {
// This worker has no followup tasks, so we can just bail out
@@ -50,7 +51,7 @@
id(new PhabricatorDiffusionApplication())->getPHID());
$editor = id(new PhabricatorAuditEditor())
- ->setActor(PhabricatorUser::getOmnipotentUser())
+ ->setActor($viewer)
->setActingAsPHID($acting_as_phid)
->setContinueOnMissingFields(true)
->setContinueOnNoEffect(true)
@@ -69,29 +70,6 @@
'committerPHID' => $data->getCommitDetail('committerPHID'),
));
- $reverts_refs = id(new DifferentialCustomFieldRevertsParser())
- ->parseCorpus($data->getCommitMessage());
- $reverts = array_mergev(ipull($reverts_refs, 'monograms'));
-
- if ($reverts) {
- $reverted_commits = id(new DiffusionCommitQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withRepository($repository)
- ->withIdentifiers($reverts)
- ->execute();
- $reverted_commit_phids = mpull($reverted_commits, 'getPHID', 'getPHID');
-
- // NOTE: Skip any write attempts if a user cleverly implies a commit
- // reverts itself.
- unset($reverted_commit_phids[$commit->getPHID()]);
-
- $reverts_edge = DiffusionCommitRevertsCommitEdgeType::EDGECONST;
- $xactions[] = id(new PhabricatorAuditTransaction())
- ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
- ->setMetadataValue('edge:type', $reverts_edge)
- ->setNewValue(array('+' => array_fuse($reverted_commit_phids)));
- }
-
try {
$raw_patch = $this->loadRawPatchText($repository, $commit);
} catch (Exception $ex) {
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -458,6 +458,12 @@
case PhabricatorTransactions::TYPE_JOIN_POLICY:
return 'fa-lock';
case PhabricatorTransactions::TYPE_EDGE:
+ switch ($this->getMetadataValue('edge:type')) {
+ case DiffusionCommitRevertedByCommitEdgeType::EDGECONST:
+ return 'fa-undo';
+ case DiffusionCommitRevertsCommitEdgeType::EDGECONST:
+ return 'fa-ambulance';
+ }
return 'fa-link';
case PhabricatorTransactions::TYPE_BUILDABLE:
return 'fa-wrench';
@@ -496,6 +502,14 @@
return 'black';
}
break;
+ case PhabricatorTransactions::TYPE_EDGE:
+ switch ($this->getMetadataValue('edge:type')) {
+ case DiffusionCommitRevertedByCommitEdgeType::EDGECONST:
+ return 'pink';
+ case DiffusionCommitRevertsCommitEdgeType::EDGECONST:
+ return 'sky';
+ }
+ break;
case PhabricatorTransactions::TYPE_BUILDABLE:
switch ($this->getNewValue()) {
case HarbormasterBuildable::STATUS_PASSED:
diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
--- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
+++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
@@ -676,73 +676,73 @@
'%s edited commit(s), added %s: %s; removed %s: %s.' =>
'%s edited commits, added %3$s; removed %5$s.',
- '%s added %s reverted commit(s): %s.' => array(
+ '%s added %s reverted change(s): %s.' => array(
array(
- '%s added a reverted commit: %3$s.',
- '%s added reverted commits: %3$s.',
+ '%s added a reverted change: %3$s.',
+ '%s added reverted changes: %3$s.',
),
),
- '%s removed %s reverted commit(s): %s.' => array(
+ '%s removed %s reverted change(s): %s.' => array(
array(
- '%s removed a reverted commit: %3$s.',
- '%s removed reverted commits: %3$s.',
+ '%s removed a reverted change: %3$s.',
+ '%s removed reverted changes: %3$s.',
),
),
- '%s edited reverted commit(s), added %s: %s; removed %s: %s.' =>
- '%s edited reverted commits, added %3$s; removed %5$s.',
+ '%s edited reverted change(s), added %s: %s; removed %s: %s.' =>
+ '%s edited reverted changes, added %3$s; removed %5$s.',
- '%s added %s reverted commit(s) for %s: %s.' => array(
+ '%s added %s reverted change(s) for %s: %s.' => array(
array(
- '%s added a reverted commit for %3$s: %4$s.',
- '%s added reverted commits for %3$s: %4$s.',
+ '%s added a reverted change for %3$s: %4$s.',
+ '%s added reverted changes for %3$s: %4$s.',
),
),
- '%s removed %s reverted commit(s) for %s: %s.' => array(
+ '%s removed %s reverted change(s) for %s: %s.' => array(
array(
- '%s removed a reverted commit for %3$s: %4$s.',
- '%s removed reverted commits for %3$s: %4$s.',
+ '%s removed a reverted change for %3$s: %4$s.',
+ '%s removed reverted changes for %3$s: %4$s.',
),
),
- '%s edited reverted commit(s) for %s, added %s: %s; removed %s: %s.' =>
- '%s edited reverted commits for %2$s, added %4$s; removed %6$s.',
+ '%s edited reverted change(s) for %s, added %s: %s; removed %s: %s.' =>
+ '%s edited reverted changes for %2$s, added %4$s; removed %6$s.',
- '%s added %s reverting commit(s): %s.' => array(
+ '%s added %s reverting change(s): %s.' => array(
array(
- '%s added a reverting commit: %3$s.',
- '%s added reverting commits: %3$s.',
+ '%s added a reverting change: %3$s.',
+ '%s added reverting changes: %3$s.',
),
),
- '%s removed %s reverting commit(s): %s.' => array(
+ '%s removed %s reverting change(s): %s.' => array(
array(
- '%s removed a reverting commit: %3$s.',
- '%s removed reverting commits: %3$s.',
+ '%s removed a reverting change: %3$s.',
+ '%s removed reverting changes: %3$s.',
),
),
- '%s edited reverting commit(s), added %s: %s; removed %s: %s.' =>
- '%s edited reverting commits, added %3$s; removed %5$s.',
+ '%s edited reverting change(s), added %s: %s; removed %s: %s.' =>
+ '%s edited reverting changes, added %3$s; removed %5$s.',
- '%s added %s reverting commit(s) for %s: %s.' => array(
+ '%s added %s reverting change(s) for %s: %s.' => array(
array(
- '%s added a reverting commit for %3$s: %4$s.',
- '%s added reverting commits for %3$s: %4$s.',
+ '%s added a reverting change for %3$s: %4$s.',
+ '%s added reverting changes for %3$s: %4$s.',
),
),
- '%s removed %s reverting commit(s) for %s: %s.' => array(
+ '%s removed %s reverting change(s) for %s: %s.' => array(
array(
- '%s removed a reverting commit for %3$s: %4$s.',
- '%s removed reverting commits for %3$s: %4$s.',
+ '%s removed a reverting change for %3$s: %4$s.',
+ '%s removed reverting changes for %3$s: %4$s.',
),
),
- '%s edited reverting commit(s) for %s, added %s: %s; removed %s: %s.' =>
- '%s edited reverting commits for %s, added %4$s; removed %6$s.',
+ '%s edited reverting change(s) for %s, added %s: %s; removed %s: %s.' =>
+ '%s edited reverting changes for %s, added %4$s; removed %6$s.',
'%s changed project member(s), added %d: %s; removed %d: %s.' =>
'%s changed project members, added %3$s; removed %5$s.',

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 9, 5:55 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7390319
Default Alt Text
D18978.diff (15 KB)

Event Timeline