2013-06-21 21:54:29 +02:00
|
|
|
<?php
|
|
|
|
|
|
|
|
final class DifferentialTransaction extends PhabricatorApplicationTransaction {
|
|
|
|
|
2014-03-12 14:04:30 +01:00
|
|
|
private $isCommandeerSideEffect;
|
|
|
|
|
|
|
|
|
|
|
|
public function setIsCommandeerSideEffect($is_side_effect) {
|
|
|
|
$this->isCommandeerSideEffect = $is_side_effect;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
public function getIsCommandeerSideEffect() {
|
|
|
|
return $this->isCommandeerSideEffect;
|
|
|
|
}
|
|
|
|
|
Migrate Differential comments to ApplicationTransactions
Summary:
Ref T2222. This is the big one.
This migrates each `DifferentialComment` to one or more ApplicationTransactions (action, cc, reviewers, update, comment, inlines), and makes `DifferentialComment` a double-reader for ApplicationTransactions.
The migration is pretty straightforward:
- If a comment took an action not otherwise covered, it gets an "action" transaction. This is something like "epriestley abandoned this revision.".
- If a comment updated the diff, it gets an "updated diff" transaction. Very old transactions of this type may not have a diff ID (probably only at Facebook).
- If a comment added or removed reviewers, it gets a "changed reviewers" transaction.
- If a comment added CCs, it gets a "subscribers" transaction.
- If a comment added comment text, it gets a "comment" transaction.
- For each inline attached to a comment, we generate an "inline" transaction.
Most comments generate a small number of transactions, but a few generate a significant number.
At HEAD, the code is basically already doing this, so comments in the last day or two already obey these rules, roughly, and will all generate only one transaction (except inlines).
Because we've already preallocated PHIDs in the comment text table, we only need to write to the transaction table.
NOTE: This significantly degrades Differential, making inline comments pretty much useless (they each get their own transaction, and don't show line numbers or files). The data is all fine, but the UI is garbage now. This needs to be fixed before we can deploy this to users, but it's easily separable since it's all just display code.
Specifically, they look like this:
{F112270}
Test Plan:
I've migrated locally and put things through their paces, but it's hard to catch sketchy stuff locally because most of my test data is nonsense and bad migrations wouldn't necessarily look out of place.
IMPORTANT: I'm planning to push this to a branch and then shift production over to the branch, and run it for a day or two before bringing it to master.
I generally feel good about this change: it's not that big since we were able to separate a lot of pieces out of it, and it's pretty straightforward. That said, it's still one of the most scary/dangerous changes we've ever made.
Reviewers: btrahan
CC: chad, aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8210
2014-02-12 00:36:58 +01:00
|
|
|
const TYPE_INLINE = 'differential:inline';
|
|
|
|
const TYPE_UPDATE = 'differential:update';
|
|
|
|
const TYPE_ACTION = 'differential:action';
|
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
|
|
|
const TYPE_STATUS = 'differential:status';
|
Migrate Differential comments to ApplicationTransactions
Summary:
Ref T2222. This is the big one.
This migrates each `DifferentialComment` to one or more ApplicationTransactions (action, cc, reviewers, update, comment, inlines), and makes `DifferentialComment` a double-reader for ApplicationTransactions.
The migration is pretty straightforward:
- If a comment took an action not otherwise covered, it gets an "action" transaction. This is something like "epriestley abandoned this revision.".
- If a comment updated the diff, it gets an "updated diff" transaction. Very old transactions of this type may not have a diff ID (probably only at Facebook).
- If a comment added or removed reviewers, it gets a "changed reviewers" transaction.
- If a comment added CCs, it gets a "subscribers" transaction.
- If a comment added comment text, it gets a "comment" transaction.
- For each inline attached to a comment, we generate an "inline" transaction.
Most comments generate a small number of transactions, but a few generate a significant number.
At HEAD, the code is basically already doing this, so comments in the last day or two already obey these rules, roughly, and will all generate only one transaction (except inlines).
Because we've already preallocated PHIDs in the comment text table, we only need to write to the transaction table.
NOTE: This significantly degrades Differential, making inline comments pretty much useless (they each get their own transaction, and don't show line numbers or files). The data is all fine, but the UI is garbage now. This needs to be fixed before we can deploy this to users, but it's easily separable since it's all just display code.
Specifically, they look like this:
{F112270}
Test Plan:
I've migrated locally and put things through their paces, but it's hard to catch sketchy stuff locally because most of my test data is nonsense and bad migrations wouldn't necessarily look out of place.
IMPORTANT: I'm planning to push this to a branch and then shift production over to the branch, and run it for a day or two before bringing it to master.
I generally feel good about this change: it's not that big since we were able to separate a lot of pieces out of it, and it's pretty straightforward. That said, it's still one of the most scary/dangerous changes we've ever made.
Reviewers: btrahan
CC: chad, aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8210
2014-02-12 00:36:58 +01:00
|
|
|
|
2013-06-21 21:54:29 +02:00
|
|
|
public function getApplicationName() {
|
|
|
|
return 'differential';
|
|
|
|
}
|
|
|
|
|
|
|
|
public function getApplicationTransactionType() {
|
2014-07-24 00:05:46 +02:00
|
|
|
return DifferentialRevisionPHIDType::TYPECONST;
|
2013-06-21 21:54:29 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
public function getApplicationTransactionCommentObject() {
|
|
|
|
return new DifferentialTransactionComment();
|
|
|
|
}
|
|
|
|
|
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
|
|
|
public function shouldHide() {
|
2014-03-08 20:21:38 +01:00
|
|
|
$old = $this->getOldValue();
|
|
|
|
$new = $this->getNewValue();
|
|
|
|
|
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
|
|
|
switch ($this->getTransactionType()) {
|
2014-03-08 20:21:38 +01:00
|
|
|
case self::TYPE_UPDATE:
|
|
|
|
// Older versions of this transaction have an ID for the new value,
|
|
|
|
// and/or do not record the old value. Only hide the transaction if
|
|
|
|
// the new value is a PHID, indicating that this is a newer style
|
|
|
|
// transaction.
|
|
|
|
if ($old === null) {
|
2014-07-24 00:05:46 +02:00
|
|
|
if (phid_get_type($new) == DifferentialDiffPHIDType::TYPECONST) {
|
2014-03-08 20:21:38 +01:00
|
|
|
return 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 PhabricatorTransactions::TYPE_EDGE:
|
|
|
|
$add = array_diff_key($new, $old);
|
|
|
|
$rem = array_diff_key($old, $new);
|
|
|
|
|
|
|
|
// Hide metadata-only edge transactions. These correspond to users
|
|
|
|
// accepting or rejecting revisions, but the change is always explicit
|
|
|
|
// because of the TYPE_ACTION transaction. Rendering these transactions
|
|
|
|
// just creates clutter.
|
|
|
|
|
|
|
|
if (!$add && !$rem) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
2014-03-08 20:21:38 +01:00
|
|
|
return parent::shouldHide();
|
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
|
|
|
}
|
|
|
|
|
2014-02-26 00:29:10 +01:00
|
|
|
public function shouldHideForMail(array $xactions) {
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case self::TYPE_INLINE:
|
|
|
|
// Hide inlines when rendering mail transactions if any other
|
|
|
|
// transaction type exists.
|
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
if ($xaction->getTransactionType() != self::TYPE_INLINE) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// If only inline transactions exist, we just render the first one.
|
|
|
|
return ($this !== head($xactions));
|
|
|
|
}
|
|
|
|
|
2014-04-19 02:51:59 +02:00
|
|
|
return parent::shouldHideForMail($xactions);
|
2014-02-26 00:29:10 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
public function getBodyForMail() {
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case self::TYPE_INLINE:
|
|
|
|
// Don't render inlines into the mail body; they render into a special
|
|
|
|
// section immediately after the body instead.
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getBodyForMail();
|
|
|
|
}
|
|
|
|
|
2014-03-01 01:49:22 +01:00
|
|
|
public function getRequiredHandlePHIDs() {
|
|
|
|
$phids = parent::getRequiredHandlePHIDs();
|
|
|
|
|
|
|
|
$old = $this->getOldValue();
|
|
|
|
$new = $this->getNewValue();
|
|
|
|
|
|
|
|
switch ($this->getTransactionType()) {
|
2014-09-12 19:12:52 +02:00
|
|
|
case self::TYPE_ACTION:
|
|
|
|
if ($new == DifferentialAction::ACTION_CLOSE &&
|
|
|
|
$this->getMetadataValue('isCommitClose')) {
|
|
|
|
$phids[] = $this->getMetadataValue('commitPHID');
|
|
|
|
if ($this->getMetadataValue('committerPHID')) {
|
|
|
|
$phids[] = $this->getMetadataValue('committerPHID');
|
|
|
|
}
|
|
|
|
if ($this->getMetadataValue('authorPHID')) {
|
|
|
|
$phids[] = $this->getMetadataValue('authorPHID');
|
|
|
|
}
|
|
|
|
}
|
|
|
|
break;
|
2014-03-01 01:49:22 +01:00
|
|
|
case self::TYPE_UPDATE:
|
|
|
|
if ($new) {
|
|
|
|
$phids[] = $new;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $phids;
|
|
|
|
}
|
2014-02-26 00:29:10 +01:00
|
|
|
|
2014-04-26 21:44:05 +02:00
|
|
|
public function getActionStrength() {
|
|
|
|
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case self::TYPE_ACTION:
|
|
|
|
return 3;
|
|
|
|
case self::TYPE_UPDATE:
|
|
|
|
return 2;
|
|
|
|
case self::TYPE_INLINE:
|
|
|
|
return 0.25;
|
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getActionStrength();
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2014-03-03 17:36:40 +01:00
|
|
|
public function getActionName() {
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case self::TYPE_INLINE:
|
|
|
|
return pht('Commented On');
|
|
|
|
case self::TYPE_UPDATE:
|
2014-03-08 20:43:50 +01:00
|
|
|
$old = $this->getOldValue();
|
|
|
|
if ($old === null) {
|
|
|
|
return pht('Request');
|
|
|
|
} else {
|
|
|
|
return pht('Updated');
|
|
|
|
}
|
2014-03-03 17:36:40 +01:00
|
|
|
case self::TYPE_ACTION:
|
|
|
|
$map = array(
|
|
|
|
DifferentialAction::ACTION_ACCEPT => pht('Accepted'),
|
|
|
|
DifferentialAction::ACTION_REJECT => pht('Requested Changes To'),
|
|
|
|
DifferentialAction::ACTION_RETHINK => pht('Planned Changes To'),
|
|
|
|
DifferentialAction::ACTION_ABANDON => pht('Abandoned'),
|
|
|
|
DifferentialAction::ACTION_CLOSE => pht('Closed'),
|
|
|
|
DifferentialAction::ACTION_REQUEST => pht('Requested A Review Of'),
|
|
|
|
DifferentialAction::ACTION_RESIGN => pht('Resigned From'),
|
|
|
|
DifferentialAction::ACTION_ADDREVIEWERS => pht('Added Reviewers'),
|
|
|
|
DifferentialAction::ACTION_CLAIM => pht('Commandeered'),
|
|
|
|
DifferentialAction::ACTION_REOPEN => pht('Reopened'),
|
|
|
|
);
|
|
|
|
$name = idx($map, $this->getNewValue());
|
|
|
|
if ($name !== null) {
|
|
|
|
return $name;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getActionName();
|
|
|
|
}
|
|
|
|
|
|
|
|
public function getMailTags() {
|
|
|
|
$tags = array();
|
|
|
|
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case PhabricatorTransactions::TYPE_SUBSCRIBERS;
|
|
|
|
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_CC;
|
|
|
|
break;
|
|
|
|
case self::TYPE_ACTION:
|
|
|
|
switch ($this->getNewValue()) {
|
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_CLOSED;
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
break;
|
2014-03-08 23:43:34 +01:00
|
|
|
case self::TYPE_UPDATE:
|
|
|
|
$old = $this->getOldValue();
|
|
|
|
if ($old === null) {
|
|
|
|
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEW_REQUEST;
|
|
|
|
} else {
|
|
|
|
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_UPDATED;
|
|
|
|
}
|
|
|
|
break;
|
2014-03-03 17:36:40 +01:00
|
|
|
case PhabricatorTransactions::TYPE_EDGE:
|
|
|
|
switch ($this->getMetadataValue('edge:type')) {
|
|
|
|
case PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER:
|
|
|
|
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEWERS;
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
case PhabricatorTransactions::TYPE_COMMENT:
|
|
|
|
case self::TYPE_INLINE:
|
|
|
|
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMENT;
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!$tags) {
|
|
|
|
$tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_OTHER;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $tags;
|
|
|
|
}
|
|
|
|
|
2014-02-12 23:05:40 +01:00
|
|
|
public function getTitle() {
|
|
|
|
$author_phid = $this->getAuthorPHID();
|
2014-02-14 00:35:44 +01:00
|
|
|
$author_handle = $this->renderHandleLink($author_phid);
|
2014-02-12 23:05:40 +01:00
|
|
|
|
|
|
|
$old = $this->getOldValue();
|
|
|
|
$new = $this->getNewValue();
|
|
|
|
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case self::TYPE_INLINE:
|
|
|
|
return pht(
|
|
|
|
'%s added inline comments.',
|
2014-02-14 00:35:44 +01:00
|
|
|
$author_handle);
|
|
|
|
case self::TYPE_UPDATE:
|
2014-09-12 19:12:52 +02:00
|
|
|
if ($this->getMetadataValue('isCommitUpdate')) {
|
|
|
|
return pht(
|
|
|
|
'This revision was automatically updated to reflect the '.
|
|
|
|
'committed changes.');
|
|
|
|
} else if ($new) {
|
2014-02-14 00:35:44 +01:00
|
|
|
// TODO: Migrate to PHIDs and use handles here?
|
2014-07-24 00:05:46 +02:00
|
|
|
if (phid_get_type($new) == DifferentialDiffPHIDType::TYPECONST) {
|
2014-03-01 01:49:22 +01:00
|
|
|
return pht(
|
|
|
|
'%s updated this revision to %s.',
|
|
|
|
$author_handle,
|
|
|
|
$this->renderHandleLink($new));
|
|
|
|
} else {
|
|
|
|
return pht(
|
|
|
|
'%s updated this revision.',
|
|
|
|
$author_handle);
|
|
|
|
}
|
2014-02-14 00:35:44 +01:00
|
|
|
} else {
|
|
|
|
return pht(
|
|
|
|
'%s updated this revision.',
|
|
|
|
$author_handle);
|
|
|
|
}
|
|
|
|
case self::TYPE_ACTION:
|
2014-09-12 19:12:52 +02:00
|
|
|
switch ($new) {
|
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
if (!$this->getMetadataValue('isCommitClose')) {
|
|
|
|
return DifferentialAction::getBasicStoryText(
|
|
|
|
$new,
|
|
|
|
$author_handle);
|
|
|
|
}
|
|
|
|
$commit_name = $this->renderHandleLink(
|
|
|
|
$this->getMetadataValue('commitPHID'));
|
|
|
|
$committer_phid = $this->getMetadataValue('committerPHID');
|
|
|
|
$author_phid = $this->getMetadataValue('authorPHID');
|
|
|
|
if ($this->getHandleIfExists($committer_phid)) {
|
|
|
|
$committer_name = $this->renderHandleLink($committer_phid);
|
|
|
|
} else {
|
|
|
|
$committer_name = $this->getMetadataValue('committerName');
|
|
|
|
}
|
|
|
|
if ($this->getHandleIfExists($author_phid)) {
|
|
|
|
$author_name = $this->renderHandleLink($author_phid);
|
|
|
|
} else {
|
|
|
|
$author_name = $this->getMetadataValue('authorName');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($committer_name && ($committer_name != $author_name)) {
|
|
|
|
return pht(
|
|
|
|
'Closed by commit %s (authored by %s, committed by %s).',
|
|
|
|
$commit_name,
|
|
|
|
$author_name,
|
|
|
|
$committer_name);
|
|
|
|
} else {
|
|
|
|
return pht(
|
|
|
|
'Closed by commit %s (authored by %s).',
|
|
|
|
$commit_name,
|
|
|
|
$author_name);
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
default:
|
|
|
|
return DifferentialAction::getBasicStoryText($new, $author_handle);
|
|
|
|
}
|
|
|
|
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 self::TYPE_STATUS:
|
|
|
|
switch ($this->getNewValue()) {
|
|
|
|
case ArcanistDifferentialRevisionStatus::ACCEPTED:
|
|
|
|
return pht(
|
|
|
|
'This revision is now accepted and ready to land.');
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
|
|
|
|
return pht(
|
|
|
|
'This revision now requires changes to proceed.');
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
|
|
|
|
return pht(
|
|
|
|
'This revision now requires review to proceed.');
|
|
|
|
}
|
2014-02-12 23:05:40 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getTitle();
|
|
|
|
}
|
|
|
|
|
2014-02-28 00:16:32 +01:00
|
|
|
public function getTitleForFeed(PhabricatorFeedStory $story) {
|
|
|
|
$author_phid = $this->getAuthorPHID();
|
|
|
|
$object_phid = $this->getObjectPHID();
|
|
|
|
|
|
|
|
$old = $this->getOldValue();
|
|
|
|
$new = $this->getNewValue();
|
|
|
|
|
|
|
|
$author_link = $this->renderHandleLink($author_phid);
|
|
|
|
$object_link = $this->renderHandleLink($object_phid);
|
|
|
|
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case self::TYPE_INLINE:
|
|
|
|
return pht(
|
|
|
|
'%s added inline comments to %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case self::TYPE_UPDATE:
|
|
|
|
return pht(
|
|
|
|
'%s updated the diff for %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case self::TYPE_ACTION:
|
|
|
|
switch ($new) {
|
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
return pht(
|
|
|
|
'%s accepted %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
return pht(
|
|
|
|
'%s requested changes to %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
|
|
|
return pht(
|
|
|
|
'%s planned changes to %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
|
|
|
return pht(
|
|
|
|
'%s abandoned %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
2014-09-12 19:12:52 +02:00
|
|
|
if (!$this->getMetadataValue('isCommitClose')) {
|
|
|
|
return pht(
|
|
|
|
'%s closed %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
} else {
|
|
|
|
$commit_name = $this->renderHandleLink(
|
|
|
|
$this->getMetadataValue('commitPHID'));
|
|
|
|
$committer_phid = $this->getMetadataValue('committerPHID');
|
|
|
|
$author_phid = $this->getMetadataValue('authorPHID');
|
|
|
|
if ($this->getHandleIfExists($committer_phid)) {
|
|
|
|
$committer_name = $this->renderHandleLink($committer_phid);
|
|
|
|
} else {
|
|
|
|
$committer_name = $this->getMetadataValue('committerName');
|
|
|
|
}
|
|
|
|
if ($this->getHandleIfExists($author_phid)) {
|
|
|
|
$author_name = $this->renderHandleLink($author_phid);
|
|
|
|
} else {
|
|
|
|
$author_name = $this->getMetadataValue('authorName');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($committer_name && ($committer_name != $author_name)) {
|
|
|
|
return pht(
|
|
|
|
'%s closed %s by commit %s (authored by %s).',
|
|
|
|
$author_link,
|
|
|
|
$object_link,
|
|
|
|
$commit_name,
|
|
|
|
$author_name);
|
|
|
|
} else {
|
|
|
|
return pht(
|
|
|
|
'%s closed %s by commit %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link,
|
|
|
|
$commit_name);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
2014-02-28 00:16:32 +01:00
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
return pht(
|
|
|
|
'%s requested review of %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
return pht(
|
|
|
|
'%s reclaimed %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
return pht(
|
|
|
|
'%s resigned from %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
return pht(
|
|
|
|
'%s commandeered %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
return pht(
|
|
|
|
'%s reopened %s.',
|
|
|
|
$author_link,
|
|
|
|
$object_link);
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
case self::TYPE_STATUS:
|
|
|
|
switch ($this->getNewValue()) {
|
|
|
|
case ArcanistDifferentialRevisionStatus::ACCEPTED:
|
|
|
|
return pht(
|
|
|
|
'%s is now accepted and ready to land.',
|
|
|
|
$object_link);
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
|
|
|
|
return pht(
|
|
|
|
'%s now requires changes to proceed.',
|
|
|
|
$object_link);
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
|
|
|
|
return pht(
|
|
|
|
'%s now requires review to proceed.',
|
|
|
|
$object_link);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getTitleForFeed($story);
|
|
|
|
}
|
|
|
|
|
2014-02-14 00:35:44 +01:00
|
|
|
public function getIcon() {
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case self::TYPE_INLINE:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-comment';
|
2014-02-14 00:35:44 +01:00
|
|
|
case self::TYPE_UPDATE:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-refresh';
|
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 self::TYPE_STATUS:
|
|
|
|
switch ($this->getNewValue()) {
|
|
|
|
case ArcanistDifferentialRevisionStatus::ACCEPTED:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-check';
|
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 ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-times';
|
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 ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-undo';
|
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
|
|
|
}
|
|
|
|
break;
|
2014-02-14 00:35:44 +01:00
|
|
|
case self::TYPE_ACTION:
|
|
|
|
switch ($this->getNewValue()) {
|
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-check';
|
2014-04-22 23:24:36 +02:00
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
2014-04-22 23:55:45 +02:00
|
|
|
return 'fa-check-circle-o';
|
2014-02-14 00:35:44 +01:00
|
|
|
case DifferentialAction::ACTION_REJECT:
|
2014-04-22 23:55:45 +02:00
|
|
|
return 'fa-times-circle-o';
|
2014-02-14 00:35:44 +01:00
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-plane';
|
2014-02-14 00:35:44 +01:00
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-headphones';
|
2014-02-14 00:35:44 +01:00
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-refresh';
|
2014-02-14 00:35:44 +01:00
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-bullhorn';
|
2014-02-14 00:35:44 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-flag';
|
2014-02-14 00:35:44 +01:00
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
2014-04-22 17:25:54 +02:00
|
|
|
return 'fa-flag';
|
2014-02-14 00:35:44 +01:00
|
|
|
}
|
2014-04-22 23:32:45 +02:00
|
|
|
case PhabricatorTransactions::TYPE_EDGE:
|
|
|
|
switch ($this->getMetadataValue('edge:type')) {
|
|
|
|
case PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER:
|
|
|
|
return 'fa-user';
|
|
|
|
}
|
2014-02-14 00:35:44 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getIcon();
|
|
|
|
}
|
|
|
|
|
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
|
|
|
public function shouldDisplayGroupWith(array $group) {
|
|
|
|
|
|
|
|
// Never group status changes with other types of actions, they're indirect
|
|
|
|
// and don't make sense when combined with direct actions.
|
|
|
|
|
|
|
|
$type_status = self::TYPE_STATUS;
|
|
|
|
|
|
|
|
if ($this->getTransactionType() == $type_status) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
|
|
|
foreach ($group as $xaction) {
|
|
|
|
if ($xaction->getTransactionType() == $type_status) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return parent::shouldDisplayGroupWith($group);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2014-02-14 00:35:44 +01:00
|
|
|
public function getColor() {
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case self::TYPE_UPDATE:
|
|
|
|
return PhabricatorTransactions::COLOR_SKY;
|
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 self::TYPE_STATUS:
|
|
|
|
switch ($this->getNewValue()) {
|
|
|
|
case ArcanistDifferentialRevisionStatus::ACCEPTED:
|
|
|
|
return PhabricatorTransactions::COLOR_GREEN;
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
|
|
|
|
return PhabricatorTransactions::COLOR_RED;
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
|
|
|
|
return PhabricatorTransactions::COLOR_ORANGE;
|
|
|
|
}
|
|
|
|
break;
|
2014-02-14 00:35:44 +01:00
|
|
|
case self::TYPE_ACTION:
|
|
|
|
switch ($this->getNewValue()) {
|
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
return PhabricatorTransactions::COLOR_BLUE;
|
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
return PhabricatorTransactions::COLOR_GREEN;
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
return PhabricatorTransactions::COLOR_RED;
|
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
|
|
|
return PhabricatorTransactions::COLOR_BLACK;
|
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
|
|
|
return PhabricatorTransactions::COLOR_RED;
|
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
return PhabricatorTransactions::COLOR_SKY;
|
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
return PhabricatorTransactions::COLOR_SKY;
|
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
return PhabricatorTransactions::COLOR_SKY;
|
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
return PhabricatorTransactions::COLOR_ORANGE;
|
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
return PhabricatorTransactions::COLOR_YELLOW;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
return parent::getColor();
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:26 +01:00
|
|
|
public function getNoEffectDescription() {
|
|
|
|
switch ($this->getTransactionType()) {
|
|
|
|
case PhabricatorTransactions::TYPE_EDGE:
|
|
|
|
switch ($this->getMetadataValue('edge:type')) {
|
|
|
|
case PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER:
|
|
|
|
return pht(
|
2014-02-25 21:36:02 +01:00
|
|
|
'The reviewers you are trying to add are already reviewing '.
|
|
|
|
'this revision.');
|
2014-02-25 00:57:26 +01:00
|
|
|
}
|
2014-02-25 00:57:49 +01:00
|
|
|
break;
|
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
switch ($this->getNewValue()) {
|
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
return pht('This revision is already closed.');
|
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
|
|
|
return pht('This revision has already been abandoned.');
|
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
return pht(
|
|
|
|
'You can not reclaim this revision because his revision is '.
|
|
|
|
'not abandoned.');
|
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
return pht(
|
|
|
|
'You can not reopen this revision because this revision is '.
|
|
|
|
'not closed.');
|
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
|
|
|
return pht('This revision already requires changes.');
|
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
return pht('Review is already requested for this revision.');
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
return pht(
|
|
|
|
'You can not resign from this revision because you are not '.
|
|
|
|
'a reviewer.');
|
2014-02-25 21:36:24 +01:00
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
return pht(
|
|
|
|
'You can not commandeer this revision because you already own '.
|
|
|
|
'it.');
|
2014-02-25 21:36:39 +01:00
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
return pht(
|
|
|
|
'You have already accepted this revision.');
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
return pht(
|
|
|
|
'You have already requested changes to this revision.');
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
|
|
|
break;
|
2014-02-25 00:57:26 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getNoEffectDescription();
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2013-06-21 21:54:29 +02:00
|
|
|
}
|