2014-02-19 01:32:55 +01:00
|
|
|
<?php
|
|
|
|
|
|
|
|
final class DifferentialTransactionEditor
|
|
|
|
extends PhabricatorApplicationTransactionEditor {
|
|
|
|
|
2014-03-05 21:07:13 +01:00
|
|
|
private $heraldEmailPHIDs;
|
2014-03-08 02:43:58 +01:00
|
|
|
private $changedPriorToCommitURI;
|
|
|
|
private $isCloseByCommit;
|
Differential - refine selecting a repository diff --> revision workflow
Summary: Fixes T6200. Ref T6237. When creating a diff from the web view, allow the user to select the repository at that time. When viewing a diff that has no associated revision and then creating a revision, pass along the repository phid to the create revision controller. Within the create revision controller, default the repository selector to this repository phid. Finally, in the editor, stop aggressively resetting the repository phid for every TYPE_UPDATE; rather, do so if its not a new object -- the diff should reign supreme in that case -- or if there's no repository -- let the diff be the guide.
Test Plan:
- made a diff with an associated repo, made a revision from the diff, saw the associated repo and it stuck on save!
- made a diff with an associated repo, made a revision from the diff but changed the repo and it stuck on save!
- made a diff with an associated repo, made a revision from the diff but changed the repo to nothing and it stuck on save!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6237, T6200
Differential Revision: https://secure.phabricator.com/D10872
2014-11-19 20:11:09 +01:00
|
|
|
private $repositoryPHIDOverride = false;
|
2014-03-08 02:43:58 +01:00
|
|
|
|
2014-08-12 21:28:41 +02:00
|
|
|
public function getEditorApplicationClass() {
|
|
|
|
return 'PhabricatorDifferentialApplication';
|
|
|
|
}
|
|
|
|
|
|
|
|
public function getEditorObjectsDescription() {
|
|
|
|
return pht('Differential Revisions');
|
|
|
|
}
|
|
|
|
|
2014-04-01 17:23:34 +02:00
|
|
|
public function getDiffUpdateTransaction(array $xactions) {
|
|
|
|
$type_update = DifferentialTransaction::TYPE_UPDATE;
|
|
|
|
|
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
if ($xaction->getTransactionType() == $type_update) {
|
|
|
|
return $xaction;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2014-03-08 02:43:58 +01:00
|
|
|
public function setIsCloseByCommit($is_close_by_commit) {
|
|
|
|
$this->isCloseByCommit = $is_close_by_commit;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
public function getIsCloseByCommit() {
|
|
|
|
return $this->isCloseByCommit;
|
|
|
|
}
|
|
|
|
|
|
|
|
public function setChangedPriorToCommitURI($uri) {
|
|
|
|
$this->changedPriorToCommitURI = $uri;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
public function getChangedPriorToCommitURI() {
|
|
|
|
return $this->changedPriorToCommitURI;
|
|
|
|
}
|
2014-03-05 21:07:13 +01:00
|
|
|
|
Differential - refine selecting a repository diff --> revision workflow
Summary: Fixes T6200. Ref T6237. When creating a diff from the web view, allow the user to select the repository at that time. When viewing a diff that has no associated revision and then creating a revision, pass along the repository phid to the create revision controller. Within the create revision controller, default the repository selector to this repository phid. Finally, in the editor, stop aggressively resetting the repository phid for every TYPE_UPDATE; rather, do so if its not a new object -- the diff should reign supreme in that case -- or if there's no repository -- let the diff be the guide.
Test Plan:
- made a diff with an associated repo, made a revision from the diff, saw the associated repo and it stuck on save!
- made a diff with an associated repo, made a revision from the diff but changed the repo and it stuck on save!
- made a diff with an associated repo, made a revision from the diff but changed the repo to nothing and it stuck on save!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6237, T6200
Differential Revision: https://secure.phabricator.com/D10872
2014-11-19 20:11:09 +01:00
|
|
|
public function setRepositoryPHIDOverride($phid_or_null) {
|
|
|
|
$this->repositoryPHIDOverride = $phid_or_null;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
public function getTransactionTypes() {
|
|
|
|
$types = parent::getTransactionTypes();
|
|
|
|
|
2014-02-25 00:57:26 +01:00
|
|
|
$types[] = PhabricatorTransactions::TYPE_COMMENT;
|
2014-02-19 01:32:55 +01:00
|
|
|
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
|
|
|
|
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
|
|
|
|
|
2014-02-25 00:57:26 +01:00
|
|
|
$types[] = DifferentialTransaction::TYPE_ACTION;
|
2014-02-25 00:57:35 +01:00
|
|
|
$types[] = DifferentialTransaction::TYPE_INLINE;
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
$types[] = DifferentialTransaction::TYPE_STATUS;
|
2014-02-19 01:32:55 +01:00
|
|
|
$types[] = DifferentialTransaction::TYPE_UPDATE;
|
|
|
|
|
|
|
|
return $types;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function getCustomTransactionOldValue(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-21 20:53:48 +01:00
|
|
|
case PhabricatorTransactions::TYPE_VIEW_POLICY:
|
|
|
|
return $object->getViewPolicy();
|
|
|
|
case PhabricatorTransactions::TYPE_EDIT_POLICY:
|
|
|
|
return $object->getEditPolicy();
|
2014-02-25 00:57:26 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
return null;
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
|
|
|
return null;
|
2014-03-01 01:49:22 +01:00
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
|
|
|
if ($this->getIsNewObject()) {
|
|
|
|
return null;
|
|
|
|
} else {
|
|
|
|
return $object->getActiveDiff()->getPHID();
|
|
|
|
}
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getCustomTransactionOldValue($object, $xaction);
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function getCustomTransactionNewValue(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-21 20:53:48 +01:00
|
|
|
case PhabricatorTransactions::TYPE_VIEW_POLICY:
|
|
|
|
case PhabricatorTransactions::TYPE_EDIT_POLICY:
|
2014-02-25 00:57:26 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
2014-03-01 01:49:22 +01:00
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
2014-02-21 20:53:48 +01:00
|
|
|
return $xaction->getNewValue();
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
|
|
|
return null;
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getCustomTransactionNewValue($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:26 +01:00
|
|
|
protected function transactionHasEffect(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
Fix several issues with application interactions while importing commits
Summary:
- Fixes T5851. Currently, if a commit has `Fixes T123`, we generate an email with just that before generating the commit email. Don't send/publish transactions about a commit before it imports (this is a tiny bit hacky, but well-contained and I don't think it causes any problems).
- Fixes T4864. Currently, we try to parse Differential information even if Differential is not installed. Instead, do this only if Differential is installed.
- Fixes T5771. Currently, if we can't figure out who the committer/author of a commit is, we don't publish a `Fixes T123` transaction. Instead, fall back to acting as "Diffusion" if we can't find a better actor. Most of this diff expands the role of application actors. The existing application actors (Herald and Harbormaster) seem to be working well.
Test Plan:
- Pushed a commit with `Fixes T123` and verified it did not generate email directly. (The task half of the transaction still does, correctly.)
- Uninstalled Differential and pushed a commit, got a clean import instead of an exception.
- Commented out author/committer PHIDs and pushed stuff, saw a "Diffusion" actor.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5771, T4864, T5851
Differential Revision: https://secure.phabricator.com/D10221
2014-08-11 21:08:24 +02:00
|
|
|
$actor_phid = $this->getActingAsPHID();
|
|
|
|
|
2014-02-25 00:57:26 +01:00
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
|
|
|
return $xaction->hasComment();
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
|
|
|
|
$status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED;
|
|
|
|
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
|
|
|
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
|
2014-03-05 19:47:47 +01:00
|
|
|
$status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED;
|
2014-02-25 00:57:49 +01:00
|
|
|
|
2014-02-25 21:36:39 +01:00
|
|
|
$action_type = $xaction->getNewValue();
|
|
|
|
switch ($action_type) {
|
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
if ($action_type == DifferentialAction::ACTION_ACCEPT) {
|
|
|
|
$new_status = DifferentialReviewerStatus::STATUS_ACCEPTED;
|
|
|
|
} else {
|
|
|
|
$new_status = DifferentialReviewerStatus::STATUS_REJECTED;
|
|
|
|
}
|
|
|
|
|
|
|
|
$actor = $this->getActor();
|
|
|
|
|
2014-03-05 19:47:47 +01:00
|
|
|
// These transactions can cause effects in two ways: by altering the
|
2014-02-25 21:36:39 +01:00
|
|
|
// status of an existing reviewer; or by adding the actor as a new
|
|
|
|
// reviewer.
|
|
|
|
|
|
|
|
$will_add_reviewer = true;
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
if ($reviewer->hasAuthority($actor)) {
|
|
|
|
if ($reviewer->getStatus() != $new_status) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
if ($reviewer->getReviewerPHID() == $actor_phid) {
|
|
|
|
$will_add_reviwer = false;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return $will_add_reviewer;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
return ($object->getStatus() != $status_closed);
|
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
|
|
|
return ($object->getStatus() != $status_abandoned);
|
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
return ($object->getStatus() == $status_abandoned);
|
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
return ($object->getStatus() == $status_closed);
|
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
2014-03-05 19:47:47 +01:00
|
|
|
return ($object->getStatus() != $status_plan);
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
return ($object->getStatus() != $status_review);
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
if ($reviewer->getReviewerPHID() == $actor_phid) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
return false;
|
2014-02-25 21:36:24 +01:00
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
return ($actor_phid != $object->getAuthorPHID());
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
2014-02-25 00:57:26 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::transactionHasEffect($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function applyCustomInternalTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
2014-03-05 19:47:29 +01:00
|
|
|
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
|
|
|
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
|
2014-03-05 19:47:47 +01:00
|
|
|
$status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED;
|
2014-03-05 19:47:29 +01:00
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-21 20:53:48 +01:00
|
|
|
case PhabricatorTransactions::TYPE_VIEW_POLICY:
|
|
|
|
$object->setViewPolicy($xaction->getNewValue());
|
|
|
|
return;
|
|
|
|
case PhabricatorTransactions::TYPE_EDIT_POLICY:
|
|
|
|
$object->setEditPolicy($xaction->getNewValue());
|
|
|
|
return;
|
2014-02-21 20:54:08 +01:00
|
|
|
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
|
2014-02-25 00:57:26 +01:00
|
|
|
case PhabricatorTransactions::TYPE_COMMENT:
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
|
|
|
return;
|
2014-02-21 20:54:32 +01:00
|
|
|
case PhabricatorTransactions::TYPE_EDGE:
|
2014-02-21 20:54:08 +01:00
|
|
|
return;
|
2014-03-01 01:49:22 +01:00
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
2014-03-12 23:24:43 +01:00
|
|
|
if (!$this->getIsCloseByCommit() &&
|
|
|
|
(($object->getStatus() == $status_revision) ||
|
|
|
|
($object->getStatus() == $status_plan))) {
|
2014-03-08 02:43:58 +01:00
|
|
|
$object->setStatus($status_review);
|
|
|
|
}
|
2014-03-25 22:01:38 +01:00
|
|
|
|
|
|
|
$diff = $this->requireDiff($xaction->getNewValue());
|
|
|
|
|
|
|
|
$object->setLineCount($diff->getLineCount());
|
Differential - refine selecting a repository diff --> revision workflow
Summary: Fixes T6200. Ref T6237. When creating a diff from the web view, allow the user to select the repository at that time. When viewing a diff that has no associated revision and then creating a revision, pass along the repository phid to the create revision controller. Within the create revision controller, default the repository selector to this repository phid. Finally, in the editor, stop aggressively resetting the repository phid for every TYPE_UPDATE; rather, do so if its not a new object -- the diff should reign supreme in that case -- or if there's no repository -- let the diff be the guide.
Test Plan:
- made a diff with an associated repo, made a revision from the diff, saw the associated repo and it stuck on save!
- made a diff with an associated repo, made a revision from the diff but changed the repo and it stuck on save!
- made a diff with an associated repo, made a revision from the diff but changed the repo to nothing and it stuck on save!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6237, T6200
Differential Revision: https://secure.phabricator.com/D10872
2014-11-19 20:11:09 +01:00
|
|
|
if ($this->repositoryPHIDOverride !== false) {
|
|
|
|
$object->setRepositoryPHID($this->repositoryPHIDOverride);
|
|
|
|
} else {
|
|
|
|
$object->setRepositoryPHID($diff->getRepositoryPHID());
|
|
|
|
}
|
2014-03-25 22:01:38 +01:00
|
|
|
$object->setArcanistProjectPHID($diff->getArcanistProjectPHID());
|
2014-04-01 17:23:34 +02:00
|
|
|
$object->attachActiveDiff($diff);
|
2014-03-25 22:01:38 +01:00
|
|
|
|
2014-03-01 01:49:22 +01:00
|
|
|
// TODO: Update the `diffPHID` once we add that.
|
|
|
|
return;
|
2014-02-25 00:57:26 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
2014-02-25 00:57:49 +01:00
|
|
|
switch ($xaction->getNewValue()) {
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
2014-02-25 21:36:39 +01:00
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
// These have no direct effects, and affect review status only
|
|
|
|
// indirectly by altering reviewers with TYPE_EDGE transactions.
|
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
|
|
|
$object->setStatus(ArcanistDifferentialRevisionStatus::ABANDONED);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
2014-03-05 19:47:47 +01:00
|
|
|
$object->setStatus($status_plan);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
$object->setStatus($status_review);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
$object->setStatus($status_review);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
$object->setStatus($status_review);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
$object->setStatus(ArcanistDifferentialRevisionStatus::CLOSED);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 21:36:24 +01:00
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
Fix several issues with application interactions while importing commits
Summary:
- Fixes T5851. Currently, if a commit has `Fixes T123`, we generate an email with just that before generating the commit email. Don't send/publish transactions about a commit before it imports (this is a tiny bit hacky, but well-contained and I don't think it causes any problems).
- Fixes T4864. Currently, we try to parse Differential information even if Differential is not installed. Instead, do this only if Differential is installed.
- Fixes T5771. Currently, if we can't figure out who the committer/author of a commit is, we don't publish a `Fixes T123` transaction. Instead, fall back to acting as "Diffusion" if we can't find a better actor. Most of this diff expands the role of application actors. The existing application actors (Herald and Harbormaster) seem to be working well.
Test Plan:
- Pushed a commit with `Fixes T123` and verified it did not generate email directly. (The task half of the transaction still does, correctly.)
- Uninstalled Differential and pushed a commit, got a clean import instead of an exception.
- Commented out author/committer PHIDs and pushed stuff, saw a "Diffusion" actor.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5771, T4864, T5851
Differential Revision: https://secure.phabricator.com/D10221
2014-08-11 21:08:24 +02:00
|
|
|
$object->setAuthorPHID($this->getActingAsPHID());
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
2014-02-25 21:36:39 +01:00
|
|
|
break;
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::applyCustomInternalTransaction($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-25 21:36:02 +01:00
|
|
|
protected function expandTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
2014-03-18 02:19:42 +01:00
|
|
|
$results = parent::expandTransaction($object, $xaction);
|
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
$actor = $this->getActor();
|
Fix several issues with application interactions while importing commits
Summary:
- Fixes T5851. Currently, if a commit has `Fixes T123`, we generate an email with just that before generating the commit email. Don't send/publish transactions about a commit before it imports (this is a tiny bit hacky, but well-contained and I don't think it causes any problems).
- Fixes T4864. Currently, we try to parse Differential information even if Differential is not installed. Instead, do this only if Differential is installed.
- Fixes T5771. Currently, if we can't figure out who the committer/author of a commit is, we don't publish a `Fixes T123` transaction. Instead, fall back to acting as "Diffusion" if we can't find a better actor. Most of this diff expands the role of application actors. The existing application actors (Herald and Harbormaster) seem to be working well.
Test Plan:
- Pushed a commit with `Fixes T123` and verified it did not generate email directly. (The task half of the transaction still does, correctly.)
- Uninstalled Differential and pushed a commit, got a clean import instead of an exception.
- Commented out author/committer PHIDs and pushed stuff, saw a "Diffusion" actor.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5771, T4864, T5851
Differential Revision: https://secure.phabricator.com/D10221
2014-08-11 21:08:24 +02:00
|
|
|
$actor_phid = $this->getActingAsPHID();
|
2014-02-25 21:37:11 +01:00
|
|
|
$type_edge = PhabricatorTransactions::TYPE_EDGE;
|
2014-03-08 21:03:15 +01:00
|
|
|
|
2014-03-25 23:41:07 +01:00
|
|
|
$status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED;
|
|
|
|
|
2015-01-01 04:43:26 +01:00
|
|
|
$edge_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
|
2014-07-18 00:41:08 +02:00
|
|
|
$edge_ref_task = DifferentialRevisionHasTaskEdgeType::EDGECONST;
|
2014-02-25 21:37:11 +01:00
|
|
|
|
2014-03-25 23:41:07 +01:00
|
|
|
$is_sticky_accept = PhabricatorEnv::getEnvConfig(
|
|
|
|
'differential.sticky-accept');
|
|
|
|
|
2014-03-18 02:19:42 +01:00
|
|
|
$downgrade_rejects = false;
|
2014-03-25 23:41:07 +01:00
|
|
|
$downgrade_accepts = false;
|
2014-03-18 02:19:42 +01:00
|
|
|
if ($this->getIsCloseByCommit()) {
|
|
|
|
// Never downgrade reviewers when we're closing a revision after a
|
|
|
|
// commit.
|
|
|
|
} else {
|
|
|
|
switch ($xaction->getTransactionType()) {
|
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
|
|
|
$downgrade_rejects = true;
|
2014-03-25 23:41:07 +01:00
|
|
|
if (!$is_sticky_accept) {
|
|
|
|
// If "sticky accept" is disabled, also downgrade the accepts.
|
|
|
|
$downgrade_accepts = true;
|
|
|
|
}
|
2014-03-18 02:19:42 +01:00
|
|
|
break;
|
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
switch ($xaction->getNewValue()) {
|
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
$downgrade_rejects = true;
|
2014-03-25 23:41:07 +01:00
|
|
|
if ((!$is_sticky_accept) ||
|
|
|
|
($object->getStatus() != $status_plan)) {
|
|
|
|
// If the old state isn't "changes planned", downgrade the
|
|
|
|
// accepts. This exception allows an accepted revision to
|
|
|
|
// go through Plan Changes -> Request Review to return to
|
|
|
|
// "accepted" if the author didn't update the revision.
|
|
|
|
$downgrade_accepts = true;
|
|
|
|
}
|
2014-03-18 02:19:42 +01:00
|
|
|
break;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$new_accept = DifferentialReviewerStatus::STATUS_ACCEPTED;
|
|
|
|
$new_reject = DifferentialReviewerStatus::STATUS_REJECTED;
|
|
|
|
$old_accept = DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER;
|
|
|
|
$old_reject = DifferentialReviewerStatus::STATUS_REJECTED_OLDER;
|
|
|
|
|
2014-03-25 23:41:07 +01:00
|
|
|
if ($downgrade_rejects || $downgrade_accepts) {
|
2014-03-18 02:19:42 +01:00
|
|
|
// When a revision is updated, change all "reject" to "rejected older
|
|
|
|
// revision". This means we won't immediately push the update back into
|
|
|
|
// "needs review", but outstanding rejects will still block it from
|
|
|
|
// moving to "accepted".
|
|
|
|
|
|
|
|
// We also do this for "Request Review", even though the diff is not
|
|
|
|
// updated directly. Essentially, this acts like an update which doesn't
|
|
|
|
// actually change the diff text.
|
|
|
|
|
|
|
|
$edits = array();
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
2014-03-25 23:41:07 +01:00
|
|
|
if ($downgrade_rejects) {
|
|
|
|
if ($reviewer->getStatus() == $new_reject) {
|
|
|
|
$edits[$reviewer->getReviewerPHID()] = array(
|
|
|
|
'data' => array(
|
|
|
|
'status' => $old_reject,
|
|
|
|
),
|
|
|
|
);
|
|
|
|
}
|
2014-03-18 02:19:42 +01:00
|
|
|
}
|
|
|
|
|
2014-03-25 23:41:07 +01:00
|
|
|
if ($downgrade_accepts) {
|
|
|
|
if ($reviewer->getStatus() == $new_accept) {
|
|
|
|
$edits[$reviewer->getReviewerPHID()] = array(
|
|
|
|
'data' => array(
|
|
|
|
'status' => $old_accept,
|
|
|
|
),
|
|
|
|
);
|
|
|
|
}
|
|
|
|
}
|
2014-03-18 02:19:42 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
if ($edits) {
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue(array('+' => $edits));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-02-25 21:36:02 +01:00
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-03-05 19:47:29 +01:00
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
2014-03-08 02:43:58 +01:00
|
|
|
if ($this->getIsCloseByCommit()) {
|
|
|
|
// Don't bother with any of this if this update is a side effect of
|
|
|
|
// commit detection.
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
2014-03-08 21:03:15 +01:00
|
|
|
// When a revision is updated and the diff comes from a branch named
|
|
|
|
// "T123" or similar, automatically associate the commit with the
|
|
|
|
// task that the branch names.
|
|
|
|
|
2014-07-23 02:03:09 +02:00
|
|
|
$maniphest = 'PhabricatorManiphestApplication';
|
2014-03-08 21:03:15 +01:00
|
|
|
if (PhabricatorApplication::isClassInstalled($maniphest)) {
|
2014-03-25 22:01:38 +01:00
|
|
|
$diff = $this->requireDiff($xaction->getNewValue());
|
2014-03-08 21:12:11 +01:00
|
|
|
$branch = $diff->getBranch();
|
|
|
|
|
|
|
|
// No "$", to allow for branches like T123_demo.
|
|
|
|
$match = null;
|
|
|
|
if (preg_match('/^T(\d+)/i', $branch, $match)) {
|
|
|
|
$task_id = $match[1];
|
|
|
|
$tasks = id(new ManiphestTaskQuery())
|
|
|
|
->setViewer($this->getActor())
|
|
|
|
->withIDs(array($task_id))
|
|
|
|
->execute();
|
|
|
|
if ($tasks) {
|
|
|
|
$task = head($tasks);
|
|
|
|
$task_phid = $task->getPHID();
|
|
|
|
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_ref_task)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue(array('+' => array($task_phid => $task_phid)));
|
2014-03-08 21:03:15 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2014-03-05 19:47:29 +01:00
|
|
|
break;
|
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
case PhabricatorTransactions::TYPE_COMMENT:
|
|
|
|
// When a user leaves a comment, upgrade their reviewer status from
|
|
|
|
// "added" to "commented" if they're also a reviewer. We may further
|
|
|
|
// upgrade this based on other actions in the transaction group.
|
|
|
|
|
|
|
|
$status_added = DifferentialReviewerStatus::STATUS_ADDED;
|
|
|
|
$status_commented = DifferentialReviewerStatus::STATUS_COMMENTED;
|
|
|
|
|
|
|
|
$data = array(
|
|
|
|
'status' => $status_commented,
|
|
|
|
);
|
|
|
|
|
|
|
|
$edits = array();
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
if ($reviewer->getReviewerPHID() == $actor_phid) {
|
|
|
|
if ($reviewer->getStatus() == $status_added) {
|
|
|
|
$edits[$actor_phid] = array(
|
|
|
|
'data' => $data,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2014-02-25 21:36:39 +01:00
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
if ($edits) {
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue(array('+' => $edits));
|
|
|
|
}
|
|
|
|
break;
|
2014-02-25 21:36:24 +01:00
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
2014-02-25 21:36:39 +01:00
|
|
|
$action_type = $xaction->getNewValue();
|
|
|
|
|
|
|
|
switch ($action_type) {
|
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
if ($action_type == DifferentialAction::ACTION_ACCEPT) {
|
|
|
|
$data = array(
|
|
|
|
'status' => DifferentialReviewerStatus::STATUS_ACCEPTED,
|
|
|
|
);
|
|
|
|
} else {
|
|
|
|
$data = array(
|
|
|
|
'status' => DifferentialReviewerStatus::STATUS_REJECTED,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
$edits = array();
|
|
|
|
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
if ($reviewer->hasAuthority($actor)) {
|
|
|
|
$edits[$reviewer->getReviewerPHID()] = array(
|
|
|
|
'data' => $data,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Also either update or add the actor themselves as a reviewer.
|
|
|
|
$edits[$actor_phid] = array(
|
|
|
|
'data' => $data,
|
|
|
|
);
|
|
|
|
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue(array('+' => $edits));
|
|
|
|
break;
|
2014-02-25 21:36:24 +01:00
|
|
|
|
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
// If the user is commandeering, add the previous owner as a
|
|
|
|
// reviewer and remove the actor.
|
|
|
|
|
|
|
|
$edits = array(
|
|
|
|
'-' => array(
|
|
|
|
$actor_phid => $actor_phid,
|
|
|
|
),
|
|
|
|
);
|
|
|
|
|
|
|
|
$owner_phid = $object->getAuthorPHID();
|
|
|
|
if ($owner_phid) {
|
|
|
|
$reviewer = new DifferentialReviewer(
|
|
|
|
$owner_phid,
|
|
|
|
array(
|
|
|
|
'status' => DifferentialReviewerStatus::STATUS_ADDED,
|
|
|
|
));
|
|
|
|
|
|
|
|
$edits['+'] = array(
|
|
|
|
$owner_phid => array(
|
|
|
|
'data' => $reviewer->getEdgeData(),
|
|
|
|
),
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
2014-03-12 14:04:30 +01:00
|
|
|
// NOTE: We're setting setIsCommandeerSideEffect() on this because
|
|
|
|
// normally you can't add a revision's author as a reviewer, but
|
|
|
|
// this action swaps them after validation executes.
|
|
|
|
|
2014-02-25 21:36:24 +01:00
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
2014-03-12 14:04:30 +01:00
|
|
|
->setIsCommandeerSideEffect(true)
|
2014-02-25 21:36:24 +01:00
|
|
|
->setNewValue($edits);
|
|
|
|
|
|
|
|
break;
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
// If the user is resigning, add a separate reviewer edit
|
|
|
|
// transaction which removes them as a reviewer.
|
|
|
|
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue(
|
|
|
|
array(
|
|
|
|
'-' => array(
|
|
|
|
$actor_phid => $actor_phid,
|
|
|
|
),
|
|
|
|
));
|
|
|
|
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function applyCustomExternalTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-21 20:53:48 +01:00
|
|
|
case PhabricatorTransactions::TYPE_VIEW_POLICY:
|
|
|
|
case PhabricatorTransactions::TYPE_EDIT_POLICY:
|
|
|
|
return;
|
2014-02-21 20:54:08 +01:00
|
|
|
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
|
2014-02-21 20:54:32 +01:00
|
|
|
case PhabricatorTransactions::TYPE_EDGE:
|
2014-02-25 00:57:26 +01:00
|
|
|
case PhabricatorTransactions::TYPE_COMMENT:
|
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
2014-02-21 20:54:08 +01:00
|
|
|
return;
|
2014-03-01 01:49:22 +01:00
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
|
|
|
// Now that we're inside the transaction, do a final check.
|
2014-03-25 22:01:38 +01:00
|
|
|
$diff = $this->requireDiff($xaction->getNewValue());
|
2014-03-01 01:49:22 +01:00
|
|
|
|
|
|
|
// TODO: It would be slightly cleaner to just revalidate this
|
|
|
|
// transaction somehow using the same validation code, but that's
|
|
|
|
// not easy to do at the moment.
|
|
|
|
|
2014-03-25 22:01:38 +01:00
|
|
|
$revision_id = $diff->getRevisionID();
|
|
|
|
if ($revision_id && ($revision_id != $object->getID())) {
|
|
|
|
throw new Exception(
|
|
|
|
pht(
|
|
|
|
'Diff is already attached to another revision. You lost '.
|
|
|
|
'a race?'));
|
2014-03-01 01:49:22 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
$diff->setRevisionID($object->getID());
|
|
|
|
$diff->save();
|
|
|
|
return;
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::applyCustomExternalTransaction($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
protected function mergeEdgeData($type, array $u, array $v) {
|
|
|
|
$result = parent::mergeEdgeData($type, $u, $v);
|
|
|
|
|
|
|
|
switch ($type) {
|
2015-01-01 04:43:26 +01:00
|
|
|
case DifferentialRevisionHasReviewerEdgeType::EDGECONST:
|
2014-02-25 21:37:11 +01:00
|
|
|
// When the same reviewer has their status updated by multiple
|
|
|
|
// transactions, we want the strongest status to win. An example of
|
|
|
|
// this is when a user adds a comment and also accepts a revision which
|
|
|
|
// they are a reviewer on. The comment creates a "commented" status,
|
|
|
|
// while the accept creates an "accepted" status. Since accept is
|
|
|
|
// stronger, it should win and persist.
|
|
|
|
|
|
|
|
$u_status = idx($u, 'status');
|
|
|
|
$v_status = idx($v, 'status');
|
|
|
|
$u_str = DifferentialReviewerStatus::getStatusStrength($u_status);
|
|
|
|
$v_str = DifferentialReviewerStatus::getStatusStrength($v_status);
|
|
|
|
if ($u_str > $v_str) {
|
|
|
|
$result['status'] = $u_status;
|
|
|
|
} else {
|
|
|
|
$result['status'] = $v_status;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $result;
|
|
|
|
}
|
|
|
|
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
protected function applyFinalEffects(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
|
2014-03-11 21:47:30 +01:00
|
|
|
// Load the most up-to-date version of the revision and its reviewers,
|
|
|
|
// so we don't need to try to deduce the state of reviewers by examining
|
|
|
|
// all the changes made by the transactions. Then, update the reviewers
|
|
|
|
// on the object to make sure we're acting on the current reviewer set
|
|
|
|
// (and, for example, sending mail to the right people).
|
|
|
|
|
|
|
|
$new_revision = id(new DifferentialRevisionQuery())
|
|
|
|
->setViewer($this->getActor())
|
|
|
|
->needReviewerStatus(true)
|
2014-04-01 22:34:05 +02:00
|
|
|
->needActiveDiffs(true)
|
2014-03-11 21:47:30 +01:00
|
|
|
->withIDs(array($object->getID()))
|
|
|
|
->executeOne();
|
|
|
|
if (!$new_revision) {
|
|
|
|
throw new Exception(
|
|
|
|
pht('Failed to load revision from transaction finalization.'));
|
|
|
|
}
|
|
|
|
|
|
|
|
$object->attachReviewerStatus($new_revision->getReviewerStatus());
|
2014-04-01 22:34:05 +02:00
|
|
|
$object->attachActiveDiff($new_revision->getActiveDiff());
|
2014-04-11 21:54:21 +02:00
|
|
|
$object->attachRepository($new_revision->getRepository());
|
2014-03-11 21:47:30 +01:00
|
|
|
|
2014-03-08 21:12:11 +01:00
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
switch ($xaction->getTransactionType()) {
|
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
2014-03-25 22:01:38 +01:00
|
|
|
$diff = $this->requireDiff($xaction->getNewValue(), true);
|
2014-03-08 21:12:11 +01:00
|
|
|
|
|
|
|
// Update these denormalized index tables when we attach a new
|
|
|
|
// diff to a revision.
|
|
|
|
|
|
|
|
$this->updateRevisionHashTable($object, $diff);
|
|
|
|
$this->updateAffectedPathTable($object, $diff);
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
$status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
|
|
|
|
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
|
|
|
|
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
|
|
|
|
|
|
|
$old_status = $object->getStatus();
|
|
|
|
switch ($old_status) {
|
|
|
|
case $status_accepted:
|
|
|
|
case $status_revision:
|
|
|
|
case $status_review:
|
|
|
|
// Try to move a revision to "accepted". We look for:
|
|
|
|
//
|
|
|
|
// - at least one accepting reviewer who is a user; and
|
|
|
|
// - no rejects; and
|
2014-03-05 19:47:29 +01:00
|
|
|
// - no rejects of older diffs; and
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
// - no blocking reviewers.
|
|
|
|
|
|
|
|
$has_accepting_user = false;
|
|
|
|
$has_rejecting_reviewer = false;
|
2014-03-05 19:47:29 +01:00
|
|
|
$has_rejecting_older_reviewer = false;
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
$has_blocking_reviewer = false;
|
2014-03-11 21:47:30 +01:00
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
$reviewer_status = $reviewer->getStatus();
|
|
|
|
switch ($reviewer_status) {
|
|
|
|
case DifferentialReviewerStatus::STATUS_REJECTED:
|
|
|
|
$has_rejecting_reviewer = true;
|
|
|
|
break;
|
2014-03-05 19:47:29 +01:00
|
|
|
case DifferentialReviewerStatus::STATUS_REJECTED_OLDER:
|
|
|
|
$has_rejecting_older_reviewer = true;
|
|
|
|
break;
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
case DifferentialReviewerStatus::STATUS_BLOCKING:
|
|
|
|
$has_blocking_reviewer = true;
|
|
|
|
break;
|
|
|
|
case DifferentialReviewerStatus::STATUS_ACCEPTED:
|
|
|
|
if ($reviewer->isUser()) {
|
|
|
|
$has_accepting_user = true;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$new_status = null;
|
|
|
|
if ($has_accepting_user &&
|
|
|
|
!$has_rejecting_reviewer &&
|
2014-03-05 19:47:29 +01:00
|
|
|
!$has_rejecting_older_reviewer &&
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
!$has_blocking_reviewer) {
|
|
|
|
$new_status = $status_accepted;
|
|
|
|
} else if ($has_rejecting_reviewer) {
|
|
|
|
// This isn't accepted, and there's at least one rejecting reviewer,
|
|
|
|
// so the revision needs changes. This usually happens after a
|
|
|
|
// "reject".
|
|
|
|
$new_status = $status_revision;
|
|
|
|
} else if ($old_status == $status_accepted) {
|
|
|
|
// This revision was accepted, but it no longer satisfies the
|
|
|
|
// conditions for acceptance. This usually happens after an accepting
|
|
|
|
// reviewer resigns or is removed.
|
|
|
|
$new_status = $status_review;
|
|
|
|
}
|
|
|
|
|
2014-03-12 23:24:43 +01:00
|
|
|
if ($new_status !== null && ($new_status != $old_status)) {
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
$xaction = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType(DifferentialTransaction::TYPE_STATUS)
|
|
|
|
->setOldValue($old_status)
|
|
|
|
->setNewValue($new_status);
|
|
|
|
|
|
|
|
$xaction = $this->populateTransaction($object, $xaction)->save();
|
|
|
|
|
|
|
|
$xactions[] = $xaction;
|
|
|
|
|
|
|
|
$object->setStatus($new_status)->save();
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
default:
|
|
|
|
// Revisions can't transition out of other statuses (like closed or
|
|
|
|
// abandoned) as a side effect of reviewer status changes.
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $xactions;
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function validateTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
$type,
|
|
|
|
array $xactions) {
|
|
|
|
|
|
|
|
$errors = parent::validateTransaction($object, $type, $xactions);
|
|
|
|
|
2014-03-12 14:04:30 +01:00
|
|
|
$config_self_accept_key = 'differential.allow-self-accept';
|
|
|
|
$allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
switch ($type) {
|
2014-03-12 14:04:30 +01:00
|
|
|
case PhabricatorTransactions::TYPE_EDGE:
|
|
|
|
switch ($xaction->getMetadataValue('edge:type')) {
|
2015-01-01 04:43:26 +01:00
|
|
|
case DifferentialRevisionHasReviewerEdgeType::EDGECONST:
|
2014-03-12 14:04:30 +01:00
|
|
|
|
|
|
|
// Prevent the author from becoming a reviewer.
|
|
|
|
|
|
|
|
// NOTE: This is pretty gross, but this restriction is unusual.
|
|
|
|
// If we end up with too much more of this, we should try to clean
|
|
|
|
// this up -- maybe by moving validation to after transactions
|
|
|
|
// are adjusted (so we can just examine the final value) or adding
|
|
|
|
// a second phase there?
|
|
|
|
|
|
|
|
$author_phid = $object->getAuthorPHID();
|
|
|
|
$new = $xaction->getNewValue();
|
|
|
|
|
|
|
|
$add = idx($new, '+', array());
|
|
|
|
$eq = idx($new, '=', array());
|
|
|
|
$phids = array_keys($add + $eq);
|
|
|
|
|
|
|
|
foreach ($phids as $phid) {
|
|
|
|
if (($phid == $author_phid) &&
|
|
|
|
!$allow_self_accept &&
|
|
|
|
!$xaction->getIsCommandeerSideEffect()) {
|
|
|
|
$errors[] =
|
|
|
|
new PhabricatorApplicationTransactionValidationError(
|
|
|
|
$type,
|
|
|
|
pht('Invalid'),
|
|
|
|
pht('The author of a revision can not be a reviewer.'),
|
|
|
|
$xaction);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
break;
|
2014-03-01 01:49:22 +01:00
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
|
|
|
$diff = $this->loadDiff($xaction->getNewValue());
|
|
|
|
if (!$diff) {
|
|
|
|
$errors[] = new PhabricatorApplicationTransactionValidationError(
|
|
|
|
$type,
|
|
|
|
pht('Invalid'),
|
|
|
|
pht('The specified diff does not exist.'),
|
|
|
|
$xaction);
|
|
|
|
} else if (($diff->getRevisionID()) &&
|
|
|
|
($diff->getRevisionID() != $object->getID())) {
|
|
|
|
$errors[] = new PhabricatorApplicationTransactionValidationError(
|
|
|
|
$type,
|
|
|
|
pht('Invalid'),
|
|
|
|
pht(
|
|
|
|
'You can not update this revision to the specified diff, '.
|
|
|
|
'because the diff is already attached to another revision.'),
|
|
|
|
$xaction);
|
|
|
|
}
|
|
|
|
break;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
$error = $this->validateDifferentialAction(
|
|
|
|
$object,
|
|
|
|
$type,
|
|
|
|
$xaction,
|
|
|
|
$xaction->getNewValue());
|
|
|
|
if ($error) {
|
|
|
|
$errors[] = new PhabricatorApplicationTransactionValidationError(
|
|
|
|
$type,
|
|
|
|
pht('Invalid'),
|
|
|
|
$error,
|
|
|
|
$xaction);
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return $errors;
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
private function validateDifferentialAction(
|
|
|
|
DifferentialRevision $revision,
|
|
|
|
$type,
|
|
|
|
DifferentialTransaction $xaction,
|
|
|
|
$action) {
|
|
|
|
|
|
|
|
$author_phid = $revision->getAuthorPHID();
|
Fix several issues with application interactions while importing commits
Summary:
- Fixes T5851. Currently, if a commit has `Fixes T123`, we generate an email with just that before generating the commit email. Don't send/publish transactions about a commit before it imports (this is a tiny bit hacky, but well-contained and I don't think it causes any problems).
- Fixes T4864. Currently, we try to parse Differential information even if Differential is not installed. Instead, do this only if Differential is installed.
- Fixes T5771. Currently, if we can't figure out who the committer/author of a commit is, we don't publish a `Fixes T123` transaction. Instead, fall back to acting as "Diffusion" if we can't find a better actor. Most of this diff expands the role of application actors. The existing application actors (Herald and Harbormaster) seem to be working well.
Test Plan:
- Pushed a commit with `Fixes T123` and verified it did not generate email directly. (The task half of the transaction still does, correctly.)
- Uninstalled Differential and pushed a commit, got a clean import instead of an exception.
- Commented out author/committer PHIDs and pushed stuff, saw a "Diffusion" actor.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5771, T4864, T5851
Differential Revision: https://secure.phabricator.com/D10221
2014-08-11 21:08:24 +02:00
|
|
|
$actor_phid = $this->getActingAsPHID();
|
2014-02-25 00:57:49 +01:00
|
|
|
$actor_is_author = ($author_phid == $actor_phid);
|
|
|
|
|
2014-06-03 01:58:48 +02:00
|
|
|
$config_abandon_key = 'differential.always-allow-abandon';
|
|
|
|
$always_allow_abandon = PhabricatorEnv::getEnvConfig($config_abandon_key);
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
$config_close_key = 'differential.always-allow-close';
|
|
|
|
$always_allow_close = PhabricatorEnv::getEnvConfig($config_close_key);
|
|
|
|
|
|
|
|
$config_reopen_key = 'differential.allow-reopen';
|
|
|
|
$allow_reopen = PhabricatorEnv::getEnvConfig($config_reopen_key);
|
|
|
|
|
2014-02-25 21:36:39 +01:00
|
|
|
$config_self_accept_key = 'differential.allow-self-accept';
|
|
|
|
$allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
$revision_status = $revision->getStatus();
|
|
|
|
|
|
|
|
$status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
|
|
|
|
$status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED;
|
|
|
|
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
|
|
|
|
|
|
|
|
switch ($action) {
|
2014-02-25 21:36:39 +01:00
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
if ($actor_is_author && !$allow_self_accept) {
|
|
|
|
return pht(
|
|
|
|
'You can not accept this revision because you are the owner.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_abandoned) {
|
|
|
|
return pht(
|
|
|
|
'You can not accept this revision because it has been '.
|
|
|
|
'abandoned.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
|
|
|
'You can not accept this revision because it has already been '.
|
|
|
|
'closed.');
|
|
|
|
}
|
Allow Herald to "Require legal signatures" for reviews
Summary:
Ref T3116. Add a Herald action "Require legal signatures" which requires revision authors to accept legal agreements before their revisions can be accepted.
- Herald will check which documents the author has signed, and trigger a "you have to sign X, Y, Z" for other documents.
- If the author has already signed everything, we don't spam the revision -- basically, this only triggers when signatures are missing.
- The UI will show which documents must be signed and warn that the revision can't be accepted until they're completed.
- Users aren't allowed to "Accept" the revision until documents are cleared.
Fixes T1157. The original install making the request (Hive) no longer uses Phabricator, and this satisfies our requirements.
Test Plan:
- Added a Herald rule.
- Created a revision, saw the rule trigger.
- Viewed as author and non-author, saw field UI (generic for non-author, specific for author), transaction UI, and accept-warning UI.
- Tried to accept revision.
- Signed document, saw UI update. Note that signatures don't currently //push// an update to the revision, but could eventually (like blocking tasks work).
- Accepted revision.
- Created another revision, saw rules not add the document (since it's already signed, this is the "no spam" case).
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: asherkin, epriestley
Maniphest Tasks: T1157, T3116
Differential Revision: https://secure.phabricator.com/D9771
2014-06-29 16:53:53 +02:00
|
|
|
|
|
|
|
// TODO: It would be nice to make this generic at some point.
|
|
|
|
$signatures = DifferentialRequiredSignaturesField::loadForRevision(
|
|
|
|
$revision);
|
|
|
|
foreach ($signatures as $phid => $signed) {
|
|
|
|
if (!$signed) {
|
|
|
|
return pht(
|
|
|
|
'You can not accept this revision because the author has '.
|
|
|
|
'not signed all of the required legal documents.');
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-02-25 21:36:39 +01:00
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
if ($actor_is_author) {
|
|
|
|
return pht(
|
|
|
|
'You can not request changes to your own revision.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_abandoned) {
|
|
|
|
return pht(
|
|
|
|
'You can not request changes to this revision because it has been '.
|
|
|
|
'abandoned.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
|
|
|
'You can not request changes to this revision because it has '.
|
|
|
|
'already been closed.');
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
// You can always resign from a revision if you're a reviewer. If you
|
|
|
|
// aren't, this is a no-op rather than invalid.
|
|
|
|
break;
|
|
|
|
|
2014-02-25 21:36:24 +01:00
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
// You can claim a revision if you're not the owner. If you are, this
|
|
|
|
// is a no-op rather than invalid.
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not commandeer this revision because it has already been '.
|
|
|
|
'closed.');
|
2014-02-25 21:36:24 +01:00
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
2014-06-03 01:58:48 +02:00
|
|
|
if (!$actor_is_author && !$always_allow_abandon) {
|
2014-02-25 00:57:49 +01:00
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not abandon this revision because you do not own it. '.
|
|
|
|
'You can only abandon revisions you own.');
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not abandon this revision because it has already been '.
|
|
|
|
'closed.');
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
// NOTE: Abandons of already-abandoned revisions are treated as no-op
|
|
|
|
// instead of invalid. Other abandons are OK.
|
|
|
|
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
if (!$actor_is_author) {
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not reclaim this revision because you do not own '.
|
|
|
|
'it. You can only reclaim revisions you own.');
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not reclaim this revision because it has already been '.
|
|
|
|
'closed.');
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
// NOTE: Reclaims of other non-abandoned revisions are treated as no-op
|
|
|
|
// instead of invalid.
|
|
|
|
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
if (!$allow_reopen) {
|
|
|
|
return pht(
|
|
|
|
'The reopen action is not enabled on this Phabricator install. '.
|
|
|
|
'Adjust your configuration to enable it.');
|
|
|
|
}
|
|
|
|
|
|
|
|
// NOTE: If the revision is not closed, this is caught as a no-op
|
|
|
|
// instead of an invalid transaction.
|
|
|
|
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
|
|
|
if (!$actor_is_author) {
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not plan changes to this revision because you do not '.
|
|
|
|
'own it. To plan changes to a revision, you must be its owner.');
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
switch ($revision_status) {
|
|
|
|
case ArcanistDifferentialRevisionStatus::ACCEPTED:
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
|
|
|
|
// These are OK.
|
|
|
|
break;
|
2014-03-05 19:47:47 +01:00
|
|
|
case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED:
|
|
|
|
// Let this through, it's a no-op.
|
|
|
|
break;
|
2014-02-25 00:57:49 +01:00
|
|
|
case ArcanistDifferentialRevisionStatus::ABANDONED:
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not plan changes to this revision because it has '.
|
|
|
|
'been abandoned.');
|
2014-02-25 00:57:49 +01:00
|
|
|
case ArcanistDifferentialRevisionStatus::CLOSED:
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not plan changes to this revision because it has '.
|
|
|
|
'already been closed.');
|
2014-02-25 00:57:49 +01:00
|
|
|
default:
|
|
|
|
throw new Exception(
|
|
|
|
pht(
|
|
|
|
'Encountered unexpected revision status ("%s") when '.
|
|
|
|
'validating "%s" action.',
|
|
|
|
$revision_status,
|
|
|
|
$action));
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
if (!$actor_is_author) {
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not request review of this revision because you do '.
|
|
|
|
'not own it. To request review of a revision, you must be its '.
|
|
|
|
'owner.');
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
switch ($revision_status) {
|
|
|
|
case ArcanistDifferentialRevisionStatus::ACCEPTED:
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
|
2014-03-05 19:47:47 +01:00
|
|
|
case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED:
|
2014-02-25 00:57:49 +01:00
|
|
|
// These are OK.
|
|
|
|
break;
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
|
|
|
|
// This will be caught as "no effect" later on.
|
|
|
|
break;
|
|
|
|
case ArcanistDifferentialRevisionStatus::ABANDONED:
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not request review of this revision because it has '.
|
|
|
|
'been abandoned. Instead, reclaim it.');
|
2014-02-25 00:57:49 +01:00
|
|
|
case ArcanistDifferentialRevisionStatus::CLOSED:
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not request review of this revision because it has '.
|
|
|
|
'already been closed.');
|
2014-02-25 00:57:49 +01:00
|
|
|
default:
|
|
|
|
throw new Exception(
|
|
|
|
pht(
|
|
|
|
'Encountered unexpected revision status ("%s") when '.
|
|
|
|
'validating "%s" action.',
|
|
|
|
$revision_status,
|
|
|
|
$action));
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
2014-03-08 02:43:58 +01:00
|
|
|
// We force revisions closed when we discover a corresponding commit.
|
|
|
|
// In this case, revisions are allowed to transition to closed from
|
|
|
|
// any state. This is an automated action taken by the daemons.
|
2014-02-25 00:57:49 +01:00
|
|
|
|
2014-03-08 02:43:58 +01:00
|
|
|
if (!$this->getIsCloseByCommit()) {
|
|
|
|
if (!$actor_is_author && !$always_allow_close) {
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not close this revision because you do not own it. To '.
|
|
|
|
'close a revision, you must be its owner.');
|
2014-03-08 02:43:58 +01:00
|
|
|
}
|
2014-02-25 00:57:49 +01:00
|
|
|
|
2014-03-08 02:43:58 +01:00
|
|
|
if ($revision_status != $status_accepted) {
|
|
|
|
return pht(
|
2014-06-09 20:36:49 +02:00
|
|
|
'You can not close this revision because it has not been '.
|
|
|
|
'accepted. You can only close accepted revisions.');
|
2014-03-08 02:43:58 +01:00
|
|
|
}
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:35 +01:00
|
|
|
protected function sortTransactions(array $xactions) {
|
2014-03-01 00:25:49 +01:00
|
|
|
$xactions = parent::sortTransactions($xactions);
|
2014-03-01 00:20:45 +01:00
|
|
|
|
2014-02-25 00:57:35 +01:00
|
|
|
$head = array();
|
|
|
|
$tail = array();
|
|
|
|
|
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
$type = $xaction->getTransactionType();
|
|
|
|
if ($type == DifferentialTransaction::TYPE_INLINE) {
|
|
|
|
$tail[] = $xaction;
|
|
|
|
} else {
|
|
|
|
$head[] = $xaction;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return array_values(array_merge($head, $tail));
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function requireCapabilities(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
2014-09-09 22:49:56 +02:00
|
|
|
switch ($xaction->getTransactionType()) {}
|
2014-02-19 01:32:55 +01:00
|
|
|
|
|
|
|
return parent::requireCapabilities($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-03-05 02:01:33 +01:00
|
|
|
protected function shouldPublishFeedStory(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
2014-02-27 20:06:55 +01:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:16 +01:00
|
|
|
protected function shouldSendMail(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function getMailTo(PhabricatorLiskDAO $object) {
|
|
|
|
$phids = array();
|
|
|
|
$phids[] = $object->getAuthorPHID();
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
$phids[] = $reviewer->getReviewerPHID();
|
|
|
|
}
|
|
|
|
return $phids;
|
|
|
|
}
|
|
|
|
|
2014-03-05 21:07:13 +01:00
|
|
|
protected function getMailCC(PhabricatorLiskDAO $object) {
|
|
|
|
$phids = parent::getMailCC($object);
|
|
|
|
|
|
|
|
if ($this->heraldEmailPHIDs) {
|
|
|
|
foreach ($this->heraldEmailPHIDs as $phid) {
|
|
|
|
$phids[] = $phid;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return $phids;
|
|
|
|
}
|
|
|
|
|
2014-03-08 20:43:50 +01:00
|
|
|
protected function getMailAction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
$action = parent::getMailAction($object, $xactions);
|
|
|
|
|
|
|
|
$strongest = $this->getStrongestAction($object, $xactions);
|
|
|
|
switch ($strongest->getTransactionType()) {
|
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
|
|
|
$count = new PhutilNumber($object->getLineCount());
|
2014-04-19 02:52:11 +02:00
|
|
|
$action = pht('%s, %s line(s)', $action, $count);
|
2014-03-08 20:43:50 +01:00
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $action;
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:16 +01:00
|
|
|
protected function getMailSubjectPrefix() {
|
|
|
|
return PhabricatorEnv::getEnvConfig('metamta.differential.subject-prefix');
|
|
|
|
}
|
|
|
|
|
2014-02-28 00:16:22 +01:00
|
|
|
protected function getMailThreadID(PhabricatorLiskDAO $object) {
|
|
|
|
// This is nonstandard, but retains threading with older messages.
|
|
|
|
$phid = $object->getPHID();
|
|
|
|
return "differential-rev-{$phid}-req";
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:16 +01:00
|
|
|
protected function buildReplyHandler(PhabricatorLiskDAO $object) {
|
|
|
|
return id(new DifferentialReplyHandler())
|
|
|
|
->setMailReceiver($object);
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function buildMailTemplate(PhabricatorLiskDAO $object) {
|
|
|
|
$id = $object->getID();
|
|
|
|
$title = $object->getTitle();
|
|
|
|
|
|
|
|
$original_title = $object->getOriginalTitle();
|
|
|
|
|
|
|
|
$subject = "D{$id}: {$title}";
|
|
|
|
$thread_topic = "D{$id}: {$original_title}";
|
|
|
|
|
|
|
|
return id(new PhabricatorMetaMTAMail())
|
|
|
|
->setSubject($subject)
|
|
|
|
->addHeader('Thread-Topic', $thread_topic);
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function buildMailBody(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
|
2015-01-15 02:23:18 +01:00
|
|
|
$body = new PhabricatorMetaMTAMailBody();
|
|
|
|
$body->setViewer($this->requireActor());
|
|
|
|
|
|
|
|
$this->addHeadersAndCommentsToMailBody($body, $xactions);
|
2014-02-25 00:57:16 +01:00
|
|
|
|
2014-02-26 00:29:10 +01:00
|
|
|
$type_inline = DifferentialTransaction::TYPE_INLINE;
|
|
|
|
|
|
|
|
$inlines = array();
|
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
if ($xaction->getTransactionType() == $type_inline) {
|
|
|
|
$inlines[] = $xaction;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2015-01-15 02:23:18 +01:00
|
|
|
if ($inlines) {
|
|
|
|
$body->addTextSection(
|
|
|
|
pht('INLINE COMMENTS'),
|
|
|
|
$this->renderInlineCommentsForMail($object, $inlines));
|
|
|
|
}
|
|
|
|
|
2014-03-08 02:43:58 +01:00
|
|
|
$changed_uri = $this->getChangedPriorToCommitURI();
|
|
|
|
if ($changed_uri) {
|
2014-10-30 23:24:10 +01:00
|
|
|
$body->addLinkSection(
|
2014-03-08 02:43:58 +01:00
|
|
|
pht('CHANGED PRIOR TO COMMIT'),
|
|
|
|
$changed_uri);
|
|
|
|
}
|
|
|
|
|
2015-01-15 02:23:18 +01:00
|
|
|
$this->addCustomFieldsToMailBody($body, $object, $xactions);
|
2014-02-26 00:29:10 +01:00
|
|
|
|
2014-10-30 23:24:10 +01:00
|
|
|
$body->addLinkSection(
|
2014-02-25 00:57:16 +01:00
|
|
|
pht('REVISION DETAIL'),
|
|
|
|
PhabricatorEnv::getProductionURI('/D'.$object->getID()));
|
|
|
|
|
2014-03-08 23:43:34 +01:00
|
|
|
$update_xaction = null;
|
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
switch ($xaction->getTransactionType()) {
|
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
|
|
|
$update_xaction = $xaction;
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($update_xaction) {
|
2014-03-25 22:01:38 +01:00
|
|
|
$diff = $this->requireDiff($update_xaction->getNewValue(), true);
|
2014-03-08 23:43:34 +01:00
|
|
|
|
|
|
|
$body->addTextSection(
|
|
|
|
pht('AFFECTED FILES'),
|
|
|
|
$this->renderAffectedFilesForMail($diff));
|
|
|
|
|
|
|
|
$config_key_inline = 'metamta.differential.inline-patches';
|
|
|
|
$config_inline = PhabricatorEnv::getEnvConfig($config_key_inline);
|
|
|
|
|
|
|
|
$config_key_attach = 'metamta.differential.attach-patches';
|
|
|
|
$config_attach = PhabricatorEnv::getEnvConfig($config_key_attach);
|
|
|
|
|
|
|
|
if ($config_inline || $config_attach) {
|
2014-08-15 17:04:10 +02:00
|
|
|
$patch_section = $this->renderPatchForMail($diff);
|
|
|
|
$lines = count(phutil_split_lines($patch_section->getPlaintext()));
|
2014-03-08 23:43:34 +01:00
|
|
|
|
|
|
|
if ($config_inline && ($lines <= $config_inline)) {
|
|
|
|
$body->addTextSection(
|
|
|
|
pht('CHANGE DETAILS'),
|
2014-08-15 17:04:10 +02:00
|
|
|
$patch_section);
|
2014-03-08 23:43:34 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
if ($config_attach) {
|
|
|
|
$name = pht('D%s.%s.patch', $object->getID(), $diff->getID());
|
|
|
|
$mime_type = 'text/x-patch; charset=utf-8';
|
|
|
|
$body->addAttachment(
|
2014-08-15 17:04:10 +02:00
|
|
|
new PhabricatorMetaMTAAttachment(
|
|
|
|
$patch_section->getPlaintext(), $name, $mime_type));
|
2014-03-08 23:43:34 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2014-02-26 00:29:10 +01:00
|
|
|
|
2014-02-25 00:57:16 +01:00
|
|
|
return $body;
|
|
|
|
}
|
|
|
|
|
2014-08-12 21:28:41 +02:00
|
|
|
public function getMailTagsMap() {
|
|
|
|
return array(
|
|
|
|
MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEW_REQUEST =>
|
|
|
|
pht('A revision is created.'),
|
|
|
|
MetaMTANotificationType::TYPE_DIFFERENTIAL_UPDATED =>
|
|
|
|
pht('A revision is updated.'),
|
|
|
|
MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMENT =>
|
|
|
|
pht('Someone comments on a revision.'),
|
|
|
|
MetaMTANotificationType::TYPE_DIFFERENTIAL_CLOSED =>
|
|
|
|
pht('A revision is closed.'),
|
|
|
|
MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEWERS =>
|
|
|
|
pht("A revision's reviewers change."),
|
|
|
|
MetaMTANotificationType::TYPE_DIFFERENTIAL_CC =>
|
|
|
|
pht("A revision's CCs change."),
|
|
|
|
MetaMTANotificationType::TYPE_DIFFERENTIAL_OTHER =>
|
|
|
|
pht('Other revision activity not listed above occurs.'),
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function supportsSearch() {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function extractFilePHIDsFromCustomTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
2014-07-23 02:03:09 +02:00
|
|
|
switch ($xaction->getTransactionType()) {}
|
2014-02-19 01:32:55 +01:00
|
|
|
|
|
|
|
return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-03-01 00:20:45 +01:00
|
|
|
protected function expandCustomRemarkupBlockTransactions(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions,
|
|
|
|
$blocks,
|
|
|
|
PhutilMarkupEngine $engine) {
|
|
|
|
|
|
|
|
$flat_blocks = array_mergev($blocks);
|
|
|
|
$huge_block = implode("\n\n", $flat_blocks);
|
|
|
|
|
|
|
|
$task_map = array();
|
|
|
|
$task_refs = id(new ManiphestCustomFieldStatusParser())
|
|
|
|
->parseCorpus($huge_block);
|
|
|
|
foreach ($task_refs as $match) {
|
|
|
|
foreach ($match['monograms'] as $monogram) {
|
|
|
|
$task_id = (int)trim($monogram, 'tT');
|
|
|
|
$task_map[$task_id] = true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$rev_map = array();
|
|
|
|
$rev_refs = id(new DifferentialCustomFieldDependsOnParser())
|
|
|
|
->parseCorpus($huge_block);
|
|
|
|
foreach ($rev_refs as $match) {
|
|
|
|
foreach ($match['monograms'] as $monogram) {
|
|
|
|
$rev_id = (int)trim($monogram, 'dD');
|
|
|
|
$rev_map[$rev_id] = true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$edges = array();
|
2015-01-22 21:12:05 +01:00
|
|
|
$task_phids = array();
|
|
|
|
$rev_phids = array();
|
2014-03-01 00:20:45 +01:00
|
|
|
|
|
|
|
if ($task_map) {
|
|
|
|
$tasks = id(new ManiphestTaskQuery())
|
|
|
|
->setViewer($this->getActor())
|
|
|
|
->withIDs(array_keys($task_map))
|
|
|
|
->execute();
|
|
|
|
|
|
|
|
if ($tasks) {
|
2015-01-22 21:12:05 +01:00
|
|
|
$task_phids = mpull($tasks, 'getPHID', 'getPHID');
|
2014-07-18 00:41:08 +02:00
|
|
|
$edge_related = DifferentialRevisionHasTaskEdgeType::EDGECONST;
|
2015-01-22 21:12:05 +01:00
|
|
|
$edges[$edge_related] = $task_phids;
|
2014-03-01 00:20:45 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($rev_map) {
|
|
|
|
$revs = id(new DifferentialRevisionQuery())
|
|
|
|
->setViewer($this->getActor())
|
|
|
|
->withIDs(array_keys($rev_map))
|
|
|
|
->execute();
|
|
|
|
$rev_phids = mpull($revs, 'getPHID', 'getPHID');
|
|
|
|
|
|
|
|
// NOTE: Skip any write attempts if a user cleverly implies a revision
|
|
|
|
// depends upon itself.
|
|
|
|
unset($rev_phids[$object->getPHID()]);
|
|
|
|
|
|
|
|
if ($revs) {
|
2015-01-01 04:43:26 +01:00
|
|
|
$depends = DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST;
|
|
|
|
$edges[$depends] = $rev_phids;
|
2014-03-01 00:20:45 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2015-01-22 21:12:05 +01:00
|
|
|
$this->setUnmentionablePHIDMap(array_merge($task_phids, $rev_phids));
|
|
|
|
|
2014-03-01 00:20:45 +01:00
|
|
|
$result = array();
|
|
|
|
foreach ($edges as $type => $specs) {
|
|
|
|
$result[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
|
|
|
|
->setMetadataValue('edge:type', $type)
|
|
|
|
->setNewValue(array('+' => $specs));
|
|
|
|
}
|
|
|
|
|
|
|
|
return $result;
|
|
|
|
}
|
|
|
|
|
2014-08-15 19:14:09 +02:00
|
|
|
protected function indentForMail(array $lines) {
|
|
|
|
$indented = array();
|
|
|
|
foreach ($lines as $line) {
|
|
|
|
$indented[] = '> '.$line;
|
|
|
|
}
|
|
|
|
return $indented;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function nestCommentHistory(
|
|
|
|
DifferentialTransactionComment $comment, array $comments_by_line_number,
|
|
|
|
array $users_by_phid) {
|
|
|
|
|
|
|
|
$nested = array();
|
|
|
|
$previous_comments = $comments_by_line_number[$comment->getChangesetID()]
|
|
|
|
[$comment->getLineNumber()];
|
|
|
|
foreach ($previous_comments as $previous_comment) {
|
2014-12-30 11:48:03 +01:00
|
|
|
if ($previous_comment->getID() >= $comment->getID()) {
|
2014-08-15 19:14:09 +02:00
|
|
|
break;
|
2014-12-30 11:48:03 +01:00
|
|
|
}
|
2014-08-15 19:14:09 +02:00
|
|
|
$nested = $this->indentForMail(
|
|
|
|
array_merge(
|
|
|
|
$nested,
|
|
|
|
explode("\n", $previous_comment->getContent())));
|
|
|
|
$user = idx($users_by_phid, $previous_comment->getAuthorPHID(), null);
|
|
|
|
if ($user) {
|
|
|
|
array_unshift($nested, pht('%s wrote:', $user->getUserName()));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$nested = array_merge($nested, explode("\n", $comment->getContent()));
|
|
|
|
return implode("\n", $nested);
|
|
|
|
}
|
|
|
|
|
2014-02-26 00:29:10 +01:00
|
|
|
private function renderInlineCommentsForMail(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $inlines) {
|
|
|
|
|
|
|
|
$context_key = 'metamta.differential.unified-comment-context';
|
|
|
|
$show_context = PhabricatorEnv::getEnvConfig($context_key);
|
|
|
|
|
|
|
|
$changeset_ids = array();
|
2014-08-15 19:14:09 +02:00
|
|
|
$line_numbers_by_changeset = array();
|
2014-02-26 00:29:10 +01:00
|
|
|
foreach ($inlines as $inline) {
|
|
|
|
$id = $inline->getComment()->getChangesetID();
|
|
|
|
$changeset_ids[$id] = $id;
|
2014-08-15 19:14:09 +02:00
|
|
|
$line_numbers_by_changeset[$id][] =
|
|
|
|
$inline->getComment()->getLineNumber();
|
2014-02-26 00:29:10 +01:00
|
|
|
}
|
|
|
|
|
2014-06-05 01:14:54 +02:00
|
|
|
$changesets = id(new DifferentialChangesetQuery())
|
|
|
|
->setViewer($this->getActor())
|
|
|
|
->withIDs($changeset_ids)
|
|
|
|
->needHunks(true)
|
|
|
|
->execute();
|
2014-02-26 00:29:10 +01:00
|
|
|
|
|
|
|
$inline_groups = DifferentialTransactionComment::sortAndGroupInlines(
|
|
|
|
$inlines,
|
|
|
|
$changesets);
|
|
|
|
|
2014-06-05 01:14:54 +02:00
|
|
|
if ($show_context) {
|
|
|
|
$hunk_parser = new DifferentialHunkParser();
|
2014-08-15 19:14:09 +02:00
|
|
|
$table = new DifferentialTransactionComment();
|
|
|
|
$conn_r = $table->establishConnection('r');
|
|
|
|
$queries = array();
|
|
|
|
foreach ($line_numbers_by_changeset as $id => $line_numbers) {
|
|
|
|
$queries[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'(changesetID = %d AND lineNumber IN (%Ld))',
|
|
|
|
$id, $line_numbers);
|
|
|
|
}
|
|
|
|
$all_comments = id(new DifferentialTransactionComment())->loadAllWhere(
|
|
|
|
'transactionPHID IS NOT NULL AND (%Q)', implode(' OR ', $queries));
|
|
|
|
$comments_by_line_number = array();
|
|
|
|
foreach ($all_comments as $comment) {
|
|
|
|
$comments_by_line_number
|
|
|
|
[$comment->getChangesetID()]
|
|
|
|
[$comment->getLineNumber()]
|
|
|
|
[$comment->getID()] = $comment;
|
|
|
|
}
|
|
|
|
$author_phids = mpull($all_comments, 'getAuthorPHID');
|
|
|
|
$authors = id(new PhabricatorPeopleQuery())
|
|
|
|
->setViewer($this->getActor())
|
|
|
|
->withPHIDs($author_phids)
|
|
|
|
->execute();
|
|
|
|
$authors_by_phid = mpull($authors, null, 'getPHID');
|
2014-06-05 01:14:54 +02:00
|
|
|
}
|
|
|
|
|
2014-08-15 17:04:10 +02:00
|
|
|
$section = new PhabricatorMetaMTAMailSection();
|
2014-02-26 00:29:10 +01:00
|
|
|
foreach ($inline_groups as $changeset_id => $group) {
|
|
|
|
$changeset = idx($changesets, $changeset_id);
|
|
|
|
if (!$changeset) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
foreach ($group as $inline) {
|
|
|
|
$comment = $inline->getComment();
|
|
|
|
$file = $changeset->getFilename();
|
|
|
|
$start = $comment->getLineNumber();
|
|
|
|
$len = $comment->getLineLength();
|
|
|
|
if ($len) {
|
|
|
|
$range = $start.'-'.($start + $len);
|
|
|
|
} else {
|
|
|
|
$range = $start;
|
|
|
|
}
|
|
|
|
|
|
|
|
$inline_content = $comment->getContent();
|
|
|
|
|
|
|
|
if (!$show_context) {
|
2014-08-15 17:04:10 +02:00
|
|
|
$section->addFragment("{$file}:{$range} {$inline_content}");
|
2014-02-26 00:29:10 +01:00
|
|
|
} else {
|
2014-08-15 17:04:10 +02:00
|
|
|
$patch = $hunk_parser->makeContextDiff(
|
2014-02-26 00:29:10 +01:00
|
|
|
$changeset->getHunks(),
|
|
|
|
$comment->getIsNewFile(),
|
|
|
|
$comment->getLineNumber(),
|
|
|
|
$comment->getLineLength(),
|
|
|
|
1);
|
2014-08-15 19:14:09 +02:00
|
|
|
$nested_comments = $this->nestCommentHistory(
|
|
|
|
$inline->getComment(), $comments_by_line_number, $authors_by_phid);
|
2014-02-26 00:29:10 +01:00
|
|
|
|
2014-08-15 17:04:10 +02:00
|
|
|
$section->addFragment('================')
|
|
|
|
->addFragment('Comment at: '.$file.':'.$range)
|
|
|
|
->addPlaintextFragment($patch)
|
|
|
|
->addHTMLFragment($this->renderPatchHTMLForMail($patch))
|
|
|
|
->addFragment('----------------')
|
2014-08-15 19:14:09 +02:00
|
|
|
->addFragment($nested_comments)
|
2014-08-15 17:04:10 +02:00
|
|
|
->addFragment(null);
|
2014-02-26 00:29:10 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-08-15 17:04:10 +02:00
|
|
|
return $section;
|
2014-02-26 00:29:10 +01:00
|
|
|
}
|
|
|
|
|
2014-03-08 21:12:11 +01:00
|
|
|
private function loadDiff($phid, $need_changesets = false) {
|
|
|
|
$query = id(new DifferentialDiffQuery())
|
2014-03-01 01:49:22 +01:00
|
|
|
->withPHIDs(array($phid))
|
2014-03-08 21:12:11 +01:00
|
|
|
->setViewer($this->getActor());
|
|
|
|
|
|
|
|
if ($need_changesets) {
|
|
|
|
$query->needChangesets(true);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $query->executeOne();
|
2014-03-01 01:49:22 +01:00
|
|
|
}
|
2014-02-26 00:29:10 +01:00
|
|
|
|
2014-03-25 22:01:38 +01:00
|
|
|
private function requireDiff($phid, $need_changesets = false) {
|
|
|
|
$diff = $this->loadDiff($phid, $need_changesets);
|
|
|
|
if (!$diff) {
|
|
|
|
throw new Exception(pht('Diff "%s" does not exist!', $phid));
|
|
|
|
}
|
|
|
|
|
|
|
|
return $diff;
|
|
|
|
}
|
|
|
|
|
2014-03-05 21:07:13 +01:00
|
|
|
/* -( Herald Integration )------------------------------------------------- */
|
|
|
|
|
|
|
|
protected function shouldApplyHeraldRules(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
|
|
|
|
if ($this->getIsNewObject()) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
switch ($xaction->getTransactionType()) {
|
|
|
|
case DifferentialTransaction::TYPE_UPDATE:
|
2014-03-08 02:43:58 +01:00
|
|
|
if (!$this->getIsCloseByCommit()) {
|
|
|
|
return true;
|
|
|
|
}
|
Allow Herald to "Require legal signatures" for reviews
Summary:
Ref T3116. Add a Herald action "Require legal signatures" which requires revision authors to accept legal agreements before their revisions can be accepted.
- Herald will check which documents the author has signed, and trigger a "you have to sign X, Y, Z" for other documents.
- If the author has already signed everything, we don't spam the revision -- basically, this only triggers when signatures are missing.
- The UI will show which documents must be signed and warn that the revision can't be accepted until they're completed.
- Users aren't allowed to "Accept" the revision until documents are cleared.
Fixes T1157. The original install making the request (Hive) no longer uses Phabricator, and this satisfies our requirements.
Test Plan:
- Added a Herald rule.
- Created a revision, saw the rule trigger.
- Viewed as author and non-author, saw field UI (generic for non-author, specific for author), transaction UI, and accept-warning UI.
- Tried to accept revision.
- Signed document, saw UI update. Note that signatures don't currently //push// an update to the revision, but could eventually (like blocking tasks work).
- Accepted revision.
- Created another revision, saw rules not add the document (since it's already signed, this is the "no spam" case).
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: asherkin, epriestley
Maniphest Tasks: T1157, T3116
Differential Revision: https://secure.phabricator.com/D9771
2014-06-29 16:53:53 +02:00
|
|
|
break;
|
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
switch ($xaction->getNewValue()) {
|
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
// When users commandeer revisions, we may need to trigger
|
|
|
|
// signatures or author-based rules.
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
break;
|
2014-03-05 21:07:13 +01:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return parent::shouldApplyHeraldRules($object, $xactions);
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function buildHeraldAdapter(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
|
|
|
|
$unsubscribed_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
|
|
|
|
$object->getPHID(),
|
2015-01-03 00:33:25 +01:00
|
|
|
PhabricatorObjectHasUnsubscriberEdgeType::EDGECONST);
|
2014-03-05 21:07:13 +01:00
|
|
|
|
|
|
|
$subscribed_phids = PhabricatorSubscribersQuery::loadSubscribersForPHID(
|
|
|
|
$object->getPHID());
|
|
|
|
|
|
|
|
$revision = id(new DifferentialRevisionQuery())
|
|
|
|
->setViewer($this->getActor())
|
|
|
|
->withPHIDs(array($object->getPHID()))
|
|
|
|
->needActiveDiffs(true)
|
|
|
|
->needReviewerStatus(true)
|
|
|
|
->executeOne();
|
|
|
|
if (!$revision) {
|
|
|
|
throw new Exception(
|
|
|
|
pht(
|
|
|
|
'Failed to load revision for Herald adapter construction!'));
|
|
|
|
}
|
|
|
|
|
|
|
|
$adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter(
|
2014-03-08 15:35:41 +01:00
|
|
|
$revision,
|
|
|
|
$revision->getActiveDiff());
|
2014-03-05 21:07:13 +01:00
|
|
|
|
|
|
|
$reviewers = $revision->getReviewerStatus();
|
|
|
|
$reviewer_phids = mpull($reviewers, 'getReviewerPHID');
|
|
|
|
|
|
|
|
$adapter->setExplicitCCs($subscribed_phids);
|
|
|
|
$adapter->setExplicitReviewers($reviewer_phids);
|
|
|
|
$adapter->setForbiddenCCs($unsubscribed_phids);
|
|
|
|
|
|
|
|
return $adapter;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function didApplyHeraldRules(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
HeraldAdapter $adapter,
|
|
|
|
HeraldTranscript $transcript) {
|
|
|
|
|
|
|
|
$xactions = array();
|
|
|
|
|
|
|
|
// Build a transaction to adjust CCs.
|
|
|
|
$ccs = array(
|
|
|
|
'+' => array_keys($adapter->getCCsAddedByHerald()),
|
|
|
|
'-' => array_keys($adapter->getCCsRemovedByHerald()),
|
|
|
|
);
|
|
|
|
$value = array();
|
|
|
|
foreach ($ccs as $type => $phids) {
|
|
|
|
foreach ($phids as $phid) {
|
|
|
|
$value[$type][$phid] = $phid;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($value) {
|
|
|
|
$xactions[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
|
|
|
|
->setNewValue($value);
|
|
|
|
}
|
|
|
|
|
|
|
|
// Build a transaction to adjust reviewers.
|
|
|
|
$reviewers = array(
|
|
|
|
DifferentialReviewerStatus::STATUS_ADDED =>
|
|
|
|
array_keys($adapter->getReviewersAddedByHerald()),
|
|
|
|
DifferentialReviewerStatus::STATUS_BLOCKING =>
|
|
|
|
array_keys($adapter->getBlockingReviewersAddedByHerald()),
|
|
|
|
);
|
|
|
|
|
2014-03-12 23:24:43 +01:00
|
|
|
$old_reviewers = $object->getReviewerStatus();
|
|
|
|
$old_reviewers = mpull($old_reviewers, null, 'getReviewerPHID');
|
|
|
|
|
2014-03-05 21:07:13 +01:00
|
|
|
$value = array();
|
|
|
|
foreach ($reviewers as $status => $phids) {
|
|
|
|
foreach ($phids as $phid) {
|
2014-03-12 14:04:30 +01:00
|
|
|
if ($phid == $object->getAuthorPHID()) {
|
|
|
|
// Don't try to add the revision's author as a reviewer, since this
|
|
|
|
// isn't valid and doesn't make sense.
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
2014-03-12 23:24:43 +01:00
|
|
|
// If the target is already a reviewer, don't try to change anything
|
|
|
|
// if their current status is at least as strong as the new status.
|
|
|
|
// For example, don't downgrade an "Accepted" to a "Blocking Reviewer".
|
|
|
|
$old_reviewer = idx($old_reviewers, $phid);
|
|
|
|
if ($old_reviewer) {
|
|
|
|
$old_status = $old_reviewer->getStatus();
|
|
|
|
|
|
|
|
$old_strength = DifferentialReviewerStatus::getStatusStrength(
|
|
|
|
$old_status);
|
|
|
|
$new_strength = DifferentialReviewerStatus::getStatusStrength(
|
|
|
|
$status);
|
|
|
|
|
|
|
|
if ($new_strength <= $old_strength) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-03-05 21:07:13 +01:00
|
|
|
$value['+'][$phid] = array(
|
|
|
|
'data' => array(
|
|
|
|
'status' => $status,
|
|
|
|
),
|
|
|
|
);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($value) {
|
2015-01-01 04:43:26 +01:00
|
|
|
$edge_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
|
2014-03-05 21:07:13 +01:00
|
|
|
|
|
|
|
$xactions[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setNewValue($value);
|
|
|
|
}
|
|
|
|
|
Allow Herald to "Require legal signatures" for reviews
Summary:
Ref T3116. Add a Herald action "Require legal signatures" which requires revision authors to accept legal agreements before their revisions can be accepted.
- Herald will check which documents the author has signed, and trigger a "you have to sign X, Y, Z" for other documents.
- If the author has already signed everything, we don't spam the revision -- basically, this only triggers when signatures are missing.
- The UI will show which documents must be signed and warn that the revision can't be accepted until they're completed.
- Users aren't allowed to "Accept" the revision until documents are cleared.
Fixes T1157. The original install making the request (Hive) no longer uses Phabricator, and this satisfies our requirements.
Test Plan:
- Added a Herald rule.
- Created a revision, saw the rule trigger.
- Viewed as author and non-author, saw field UI (generic for non-author, specific for author), transaction UI, and accept-warning UI.
- Tried to accept revision.
- Signed document, saw UI update. Note that signatures don't currently //push// an update to the revision, but could eventually (like blocking tasks work).
- Accepted revision.
- Created another revision, saw rules not add the document (since it's already signed, this is the "no spam" case).
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: asherkin, epriestley
Maniphest Tasks: T1157, T3116
Differential Revision: https://secure.phabricator.com/D9771
2014-06-29 16:53:53 +02:00
|
|
|
// Require legalpad document signatures.
|
|
|
|
$legal_phids = $adapter->getRequiredSignatureDocumentPHIDs();
|
|
|
|
if ($legal_phids) {
|
|
|
|
// We only require signatures of documents which have not already
|
|
|
|
// been signed. In general, this reduces the amount of churn that
|
|
|
|
// signature rules cause.
|
|
|
|
|
|
|
|
$signatures = id(new LegalpadDocumentSignatureQuery())
|
|
|
|
->setViewer(PhabricatorUser::getOmnipotentUser())
|
|
|
|
->withDocumentPHIDs($legal_phids)
|
|
|
|
->withSignerPHIDs(array($object->getAuthorPHID()))
|
|
|
|
->execute();
|
|
|
|
$signed_phids = mpull($signatures, 'getDocumentPHID');
|
|
|
|
$legal_phids = array_diff($legal_phids, $signed_phids);
|
|
|
|
|
|
|
|
// If we still have something to trigger, add the edges.
|
|
|
|
if ($legal_phids) {
|
2015-01-01 01:15:34 +01:00
|
|
|
$edge_legal = LegalpadObjectNeedsSignatureEdgeType::EDGECONST;
|
Allow Herald to "Require legal signatures" for reviews
Summary:
Ref T3116. Add a Herald action "Require legal signatures" which requires revision authors to accept legal agreements before their revisions can be accepted.
- Herald will check which documents the author has signed, and trigger a "you have to sign X, Y, Z" for other documents.
- If the author has already signed everything, we don't spam the revision -- basically, this only triggers when signatures are missing.
- The UI will show which documents must be signed and warn that the revision can't be accepted until they're completed.
- Users aren't allowed to "Accept" the revision until documents are cleared.
Fixes T1157. The original install making the request (Hive) no longer uses Phabricator, and this satisfies our requirements.
Test Plan:
- Added a Herald rule.
- Created a revision, saw the rule trigger.
- Viewed as author and non-author, saw field UI (generic for non-author, specific for author), transaction UI, and accept-warning UI.
- Tried to accept revision.
- Signed document, saw UI update. Note that signatures don't currently //push// an update to the revision, but could eventually (like blocking tasks work).
- Accepted revision.
- Created another revision, saw rules not add the document (since it's already signed, this is the "no spam" case).
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: asherkin, epriestley
Maniphest Tasks: T1157, T3116
Differential Revision: https://secure.phabricator.com/D9771
2014-06-29 16:53:53 +02:00
|
|
|
$xactions[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
|
|
|
|
->setMetadataValue('edge:type', $edge_legal)
|
|
|
|
->setNewValue(
|
|
|
|
array(
|
|
|
|
'+' => array_fuse($legal_phids),
|
|
|
|
));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-03-05 21:07:13 +01:00
|
|
|
// Save extra email PHIDs for later.
|
2014-03-14 19:52:16 +01:00
|
|
|
$email_phids = $adapter->getEmailPHIDsAddedByHerald();
|
|
|
|
$this->heraldEmailPHIDs = array_keys($email_phids);
|
2014-03-05 21:07:13 +01:00
|
|
|
|
|
|
|
// Apply build plans.
|
|
|
|
HarbormasterBuildable::applyBuildPlans(
|
2014-03-12 20:13:09 +01:00
|
|
|
$adapter->getDiff()->getPHID(),
|
2014-03-05 21:07:13 +01:00
|
|
|
$adapter->getPHID(),
|
|
|
|
$adapter->getBuildPlans());
|
|
|
|
|
|
|
|
return $xactions;
|
|
|
|
}
|
2014-02-26 00:29:10 +01:00
|
|
|
|
2014-03-08 21:12:11 +01:00
|
|
|
/**
|
|
|
|
* Update the table which links Differential revisions to paths they affect,
|
|
|
|
* so Diffusion can efficiently find pending revisions for a given file.
|
|
|
|
*/
|
|
|
|
private function updateAffectedPathTable(
|
|
|
|
DifferentialRevision $revision,
|
|
|
|
DifferentialDiff $diff) {
|
|
|
|
|
2014-04-11 21:54:21 +02:00
|
|
|
$repository = $revision->getRepository();
|
2014-03-08 21:12:11 +01:00
|
|
|
if (!$repository) {
|
2014-04-11 21:54:21 +02:00
|
|
|
// The repository where the code lives is untracked.
|
2014-03-08 21:12:11 +01:00
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$path_prefix = null;
|
|
|
|
|
|
|
|
$local_root = $diff->getSourceControlPath();
|
|
|
|
if ($local_root) {
|
|
|
|
// We're in a working copy which supports subdirectory checkouts (e.g.,
|
|
|
|
// SVN) so we need to figure out what prefix we should add to each path
|
|
|
|
// (e.g., trunk/projects/example/) to get the absolute path from the
|
|
|
|
// root of the repository. DVCS systems like Git and Mercurial are not
|
|
|
|
// affected.
|
|
|
|
|
|
|
|
// Normalize both paths and check if the repository root is a prefix of
|
|
|
|
// the local root. If so, throw it away. Note that this correctly handles
|
|
|
|
// the case where the remote path is "/".
|
|
|
|
$local_root = id(new PhutilURI($local_root))->getPath();
|
|
|
|
$local_root = rtrim($local_root, '/');
|
|
|
|
|
|
|
|
$repo_root = id(new PhutilURI($repository->getRemoteURI()))->getPath();
|
|
|
|
$repo_root = rtrim($repo_root, '/');
|
|
|
|
|
|
|
|
if (!strncmp($repo_root, $local_root, strlen($repo_root))) {
|
|
|
|
$path_prefix = substr($local_root, strlen($repo_root));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-04-11 21:54:21 +02:00
|
|
|
$changesets = $diff->getChangesets();
|
2014-03-08 21:12:11 +01:00
|
|
|
$paths = array();
|
|
|
|
foreach ($changesets as $changeset) {
|
|
|
|
$paths[] = $path_prefix.'/'.$changeset->getFilename();
|
|
|
|
}
|
|
|
|
|
|
|
|
// Mark this as also touching all parent paths, so you can see all pending
|
|
|
|
// changes to any file within a directory.
|
|
|
|
$all_paths = array();
|
|
|
|
foreach ($paths as $local) {
|
|
|
|
foreach (DiffusionPathIDQuery::expandPathToRoot($local) as $path) {
|
|
|
|
$all_paths[$path] = true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
$all_paths = array_keys($all_paths);
|
|
|
|
|
|
|
|
$path_ids =
|
|
|
|
PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths(
|
|
|
|
$all_paths);
|
|
|
|
|
|
|
|
$table = new DifferentialAffectedPath();
|
|
|
|
$conn_w = $table->establishConnection('w');
|
|
|
|
|
|
|
|
$sql = array();
|
|
|
|
foreach ($path_ids as $path_id) {
|
|
|
|
$sql[] = qsprintf(
|
|
|
|
$conn_w,
|
|
|
|
'(%d, %d, %d, %d)',
|
|
|
|
$repository->getID(),
|
|
|
|
$path_id,
|
|
|
|
time(),
|
|
|
|
$revision->getID());
|
|
|
|
}
|
|
|
|
|
|
|
|
queryfx(
|
|
|
|
$conn_w,
|
|
|
|
'DELETE FROM %T WHERE revisionID = %d',
|
|
|
|
$table->getTableName(),
|
|
|
|
$revision->getID());
|
|
|
|
foreach (array_chunk($sql, 256) as $chunk) {
|
|
|
|
queryfx(
|
|
|
|
$conn_w,
|
|
|
|
'INSERT INTO %T (repositoryID, pathID, epoch, revisionID) VALUES %Q',
|
|
|
|
$table->getTableName(),
|
|
|
|
implode(', ', $chunk));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Update the table connecting revisions to DVCS local hashes, so we can
|
|
|
|
* identify revisions by commit/tree hashes.
|
|
|
|
*/
|
|
|
|
private function updateRevisionHashTable(
|
|
|
|
DifferentialRevision $revision,
|
|
|
|
DifferentialDiff $diff) {
|
|
|
|
|
|
|
|
$vcs = $diff->getSourceControlSystem();
|
|
|
|
if ($vcs == DifferentialRevisionControlSystem::SVN) {
|
|
|
|
// Subversion has no local commit or tree hash information, so we don't
|
|
|
|
// have to do anything.
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$property = id(new DifferentialDiffProperty())->loadOneWhere(
|
|
|
|
'diffID = %d AND name = %s',
|
|
|
|
$diff->getID(),
|
|
|
|
'local:commits');
|
|
|
|
if (!$property) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$hashes = array();
|
|
|
|
|
|
|
|
$data = $property->getData();
|
|
|
|
switch ($vcs) {
|
|
|
|
case DifferentialRevisionControlSystem::GIT:
|
|
|
|
foreach ($data as $commit) {
|
|
|
|
$hashes[] = array(
|
|
|
|
ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT,
|
|
|
|
$commit['commit'],
|
|
|
|
);
|
|
|
|
$hashes[] = array(
|
|
|
|
ArcanistDifferentialRevisionHash::HASH_GIT_TREE,
|
|
|
|
$commit['tree'],
|
|
|
|
);
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
case DifferentialRevisionControlSystem::MERCURIAL:
|
|
|
|
foreach ($data as $commit) {
|
|
|
|
$hashes[] = array(
|
|
|
|
ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT,
|
|
|
|
$commit['rev'],
|
|
|
|
);
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
$conn_w = $revision->establishConnection('w');
|
|
|
|
|
|
|
|
$sql = array();
|
|
|
|
foreach ($hashes as $info) {
|
|
|
|
list($type, $hash) = $info;
|
|
|
|
$sql[] = qsprintf(
|
|
|
|
$conn_w,
|
|
|
|
'(%d, %s, %s)',
|
|
|
|
$revision->getID(),
|
|
|
|
$type,
|
|
|
|
$hash);
|
|
|
|
}
|
|
|
|
|
|
|
|
queryfx(
|
|
|
|
$conn_w,
|
|
|
|
'DELETE FROM %T WHERE revisionID = %d',
|
|
|
|
ArcanistDifferentialRevisionHash::TABLE_NAME,
|
|
|
|
$revision->getID());
|
|
|
|
|
|
|
|
if ($sql) {
|
|
|
|
queryfx(
|
|
|
|
$conn_w,
|
|
|
|
'INSERT INTO %T (revisionID, type, hash) VALUES %Q',
|
|
|
|
ArcanistDifferentialRevisionHash::TABLE_NAME,
|
|
|
|
implode(', ', $sql));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2014-03-08 23:43:34 +01:00
|
|
|
private function renderAffectedFilesForMail(DifferentialDiff $diff) {
|
|
|
|
$changesets = $diff->getChangesets();
|
|
|
|
|
|
|
|
$filenames = mpull($changesets, 'getDisplayFilename');
|
|
|
|
sort($filenames);
|
|
|
|
|
|
|
|
$count = count($filenames);
|
|
|
|
$max = 250;
|
|
|
|
if ($count > $max) {
|
|
|
|
$filenames = array_slice($filenames, 0, $max);
|
|
|
|
$filenames[] = pht('(%d more files...)', ($count - $max));
|
|
|
|
}
|
|
|
|
|
|
|
|
return implode("\n", $filenames);
|
|
|
|
}
|
|
|
|
|
2014-08-15 17:04:10 +02:00
|
|
|
private function renderPatchHTMLForMail($patch) {
|
|
|
|
return phutil_tag('pre',
|
|
|
|
array('style' => 'font-family: monospace;'), $patch);
|
|
|
|
}
|
|
|
|
|
2014-03-08 23:43:34 +01:00
|
|
|
private function renderPatchForMail(DifferentialDiff $diff) {
|
|
|
|
$format = PhabricatorEnv::getEnvConfig('metamta.differential.patch-format');
|
|
|
|
|
2014-08-15 17:04:10 +02:00
|
|
|
$patch = id(new DifferentialRawDiffRenderer())
|
2014-03-08 23:43:34 +01:00
|
|
|
->setViewer($this->getActor())
|
|
|
|
->setFormat($format)
|
|
|
|
->setChangesets($diff->getChangesets())
|
|
|
|
->buildPatch();
|
2014-08-15 17:04:10 +02:00
|
|
|
|
|
|
|
$section = new PhabricatorMetaMTAMailSection();
|
|
|
|
$section->addHTMLFragment($this->renderPatchHTMLForMail($patch));
|
|
|
|
$section->addPlaintextFragment($patch);
|
|
|
|
|
|
|
|
return $section;
|
2014-03-08 23:43:34 +01:00
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|