1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 12:00:55 +01:00

Fix two minor Differential issues

Summary: The JIRA field is currently always enabled. This isn't correct; it
should be disabled if there's no JIRA provider.

We also use the old set of reviewers to compute mail delivery. Instead, reload
the correct set of reviewers before moving on after finalizing transactions.
This commit is contained in:
epriestley 2014-03-11 13:47:30 -07:00
parent 8818252f52
commit 0b15624c37
2 changed files with 24 additions and 14 deletions

View file

@ -13,6 +13,10 @@ final class DifferentialJIRAIssuesField
return 'jira.issues'; return 'jira.issues';
} }
public function isFieldEnabled() {
return (bool)PhabricatorAuthProviderOAuth1JIRA::getJIRAProvider();
}
public function canDisableField() { public function canDisableField() {
return false; return false;
} }

View file

@ -504,6 +504,25 @@ final class DifferentialTransactionEditor
PhabricatorLiskDAO $object, PhabricatorLiskDAO $object,
array $xactions) { array $xactions) {
// 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)
->withIDs(array($object->getID()))
->executeOne();
if (!$new_revision) {
throw new Exception(
pht('Failed to load revision from transaction finalization.'));
}
$object->attachReviewerStatus($new_revision->getReviewerStatus());
foreach ($xactions as $xaction) { foreach ($xactions as $xaction) {
switch ($xaction->getTransactionType()) { switch ($xaction->getTransactionType()) {
case DifferentialTransaction::TYPE_UPDATE: case DifferentialTransaction::TYPE_UPDATE:
@ -527,19 +546,6 @@ final class DifferentialTransactionEditor
case $status_accepted: case $status_accepted:
case $status_revision: case $status_revision:
case $status_review: case $status_review:
// 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.
$new_revision = id(new DifferentialRevisionQuery())
->setViewer($this->getActor())
->needReviewerStatus(true)
->withIDs(array($object->getID()))
->executeOne();
if (!$new_revision) {
throw new Exception(
pht('Failed to load revision from transaction finalization.'));
}
// Try to move a revision to "accepted". We look for: // Try to move a revision to "accepted". We look for:
// //
// - at least one accepting reviewer who is a user; and // - at least one accepting reviewer who is a user; and
@ -551,7 +557,7 @@ final class DifferentialTransactionEditor
$has_rejecting_reviewer = false; $has_rejecting_reviewer = false;
$has_rejecting_older_reviewer = false; $has_rejecting_older_reviewer = false;
$has_blocking_reviewer = false; $has_blocking_reviewer = false;
foreach ($new_revision->getReviewerStatus() as $reviewer) { foreach ($object->getReviewerStatus() as $reviewer) {
$reviewer_status = $reviewer->getStatus(); $reviewer_status = $reviewer->getStatus();
switch ($reviewer_status) { switch ($reviewer_status) {
case DifferentialReviewerStatus::STATUS_REJECTED: case DifferentialReviewerStatus::STATUS_REJECTED: