Differential D18413 Diff 44257 src/applications/differential/editor/DifferentialTransactionEditor.php
Changeset View
Changeset View
Standalone View
Standalone View
src/applications/differential/editor/DifferentialTransactionEditor.php
| Show First 20 Lines • Show All 119 Lines • ▼ Show 20 Lines | protected function transactionHasEffect( | ||||
| return parent::transactionHasEffect($object, $xaction); | return parent::transactionHasEffect($object, $xaction); | ||||
| } | } | ||||
| protected function applyCustomInternalTransaction( | protected function applyCustomInternalTransaction( | ||||
| PhabricatorLiskDAO $object, | PhabricatorLiskDAO $object, | ||||
| PhabricatorApplicationTransaction $xaction) { | PhabricatorApplicationTransaction $xaction) { | ||||
| $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; | |||||
| $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; | |||||
| $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; | |||||
| $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; | |||||
| $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; | |||||
| switch ($xaction->getTransactionType()) { | switch ($xaction->getTransactionType()) { | ||||
| case DifferentialTransaction::TYPE_INLINE: | case DifferentialTransaction::TYPE_INLINE: | ||||
| return; | return; | ||||
| case DifferentialTransaction::TYPE_UPDATE: | case DifferentialTransaction::TYPE_UPDATE: | ||||
| if (!$this->getIsCloseByCommit()) { | if (!$this->getIsCloseByCommit()) { | ||||
| switch ($object->getStatus()) { | if ($object->isNeedsRevision() || | ||||
| case $status_revision: | $object->isChangePlanned() || | ||||
| case $status_plan: | $object->isAbandoned()) { | ||||
| case $status_abandoned: | $object->setModernRevisionStatus( | ||||
| $object->setStatus($status_review); | DifferentialRevisionStatus::NEEDS_REVIEW); | ||||
| break; | |||||
| } | } | ||||
| } | } | ||||
| $diff = $this->requireDiff($xaction->getNewValue()); | $diff = $this->requireDiff($xaction->getNewValue()); | ||||
| $object->setLineCount($diff->getLineCount()); | $object->setLineCount($diff->getLineCount()); | ||||
| if ($this->repositoryPHIDOverride !== false) { | if ($this->repositoryPHIDOverride !== false) { | ||||
| $object->setRepositoryPHID($this->repositoryPHIDOverride); | $object->setRepositoryPHID($this->repositoryPHIDOverride); | ||||
| Show All 38 Lines | protected function expandTransaction( | ||||
| PhabricatorApplicationTransaction $xaction) { | PhabricatorApplicationTransaction $xaction) { | ||||
| $results = parent::expandTransaction($object, $xaction); | $results = parent::expandTransaction($object, $xaction); | ||||
| $actor = $this->getActor(); | $actor = $this->getActor(); | ||||
| $actor_phid = $this->getActingAsPHID(); | $actor_phid = $this->getActingAsPHID(); | ||||
| $type_edge = PhabricatorTransactions::TYPE_EDGE; | $type_edge = PhabricatorTransactions::TYPE_EDGE; | ||||
| $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; | |||||
| $edge_ref_task = DifferentialRevisionHasTaskEdgeType::EDGECONST; | $edge_ref_task = DifferentialRevisionHasTaskEdgeType::EDGECONST; | ||||
| $is_sticky_accept = PhabricatorEnv::getEnvConfig( | $is_sticky_accept = PhabricatorEnv::getEnvConfig( | ||||
| 'differential.sticky-accept'); | 'differential.sticky-accept'); | ||||
| $downgrade_rejects = false; | $downgrade_rejects = false; | ||||
| $downgrade_accepts = false; | $downgrade_accepts = false; | ||||
| if ($this->getIsCloseByCommit()) { | if ($this->getIsCloseByCommit()) { | ||||
| // Never downgrade reviewers when we're closing a revision after a | // Never downgrade reviewers when we're closing a revision after a | ||||
| // commit. | // commit. | ||||
| } else { | } else { | ||||
| switch ($xaction->getTransactionType()) { | switch ($xaction->getTransactionType()) { | ||||
| case DifferentialTransaction::TYPE_UPDATE: | case DifferentialTransaction::TYPE_UPDATE: | ||||
| $downgrade_rejects = true; | $downgrade_rejects = true; | ||||
| if (!$is_sticky_accept) { | if (!$is_sticky_accept) { | ||||
| // If "sticky accept" is disabled, also downgrade the accepts. | // If "sticky accept" is disabled, also downgrade the accepts. | ||||
| $downgrade_accepts = true; | $downgrade_accepts = true; | ||||
| } | } | ||||
| break; | break; | ||||
| case DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE: | case DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE: | ||||
| $downgrade_rejects = true; | $downgrade_rejects = true; | ||||
| if ((!$is_sticky_accept) || | if ((!$is_sticky_accept) || | ||||
| ($object->getStatus() != $status_plan)) { | (!$object->isChangePlanned())) { | ||||
| // If the old state isn't "changes planned", downgrade the accepts. | // If the old state isn't "changes planned", downgrade the accepts. | ||||
| // This exception allows an accepted revision to go through | // This exception allows an accepted revision to go through | ||||
| // "Plan Changes" -> "Request Review" and return to "accepted" if | // "Plan Changes" -> "Request Review" and return to "accepted" if | ||||
| // the author didn't update the revision, essentially undoing the | // the author didn't update the revision, essentially undoing the | ||||
| // "Plan Changes". | // "Plan Changes". | ||||
| $downgrade_accepts = true; | $downgrade_accepts = true; | ||||
| } | } | ||||
| break; | break; | ||||
| ▲ Show 20 Lines • Show All 225 Lines • ▼ Show 20 Lines | foreach ($xactions as $xaction) { | ||||
| // diff to a revision. | // diff to a revision. | ||||
| $this->updateRevisionHashTable($object, $diff); | $this->updateRevisionHashTable($object, $diff); | ||||
| $this->updateAffectedPathTable($object, $diff); | $this->updateAffectedPathTable($object, $diff); | ||||
| break; | break; | ||||
| } | } | ||||
| } | } | ||||
| $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; | $xactions = $this->updateReviewStatus($object, $xactions); | ||||
| $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; | $this->markReviewerComments($object, $xactions); | ||||
| $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; | |||||
| $is_sticky_accept = PhabricatorEnv::getEnvConfig( | return $xactions; | ||||
| 'differential.sticky-accept'); | } | ||||
| private function updateReviewStatus( | |||||
| DifferentialRevision $revision, | |||||
| array $xactions) { | |||||
| $was_accepted = $revision->isAccepted(); | |||||
| $was_revision = $revision->isNeedsRevision(); | |||||
| $was_review = $revision->isNeedsReview(); | |||||
| if (!$was_accepted && !$was_revision && !$was_review) { | |||||
| // Revisions can't transition out of other statuses (like closed or | |||||
| // abandoned) as a side effect of reviewer status changes. | |||||
| return $xactions; | |||||
| } | |||||
| $old_status = $object->getStatus(); | |||||
| $active_diff = $object->getActiveDiff(); | |||||
| switch ($old_status) { | |||||
| case $status_accepted: | |||||
| case $status_revision: | |||||
| case $status_review: | |||||
| // 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 | ||||
| // - no rejects; and | // - no rejects; and | ||||
| // - no rejects of older diffs; and | // - no rejects of older diffs; and | ||||
| // - no blocking reviewers. | // - no blocking reviewers. | ||||
| $has_accepting_user = false; | $has_accepting_user = false; | ||||
| $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 ($object->getReviewers() as $reviewer) { | |||||
| $active_diff = $revision->getActiveDiff(); | |||||
| foreach ($revision->getReviewers() as $reviewer) { | |||||
| $reviewer_status = $reviewer->getReviewerStatus(); | $reviewer_status = $reviewer->getReviewerStatus(); | ||||
| switch ($reviewer_status) { | switch ($reviewer_status) { | ||||
| case DifferentialReviewerStatus::STATUS_REJECTED: | case DifferentialReviewerStatus::STATUS_REJECTED: | ||||
| $active_phid = $active_diff->getPHID(); | $active_phid = $active_diff->getPHID(); | ||||
| if ($reviewer->isRejected($active_phid)) { | if ($reviewer->isRejected($active_phid)) { | ||||
| $has_rejecting_reviewer = true; | $has_rejecting_reviewer = true; | ||||
| } else { | } else { | ||||
| $has_rejecting_older_reviewer = true; | $has_rejecting_older_reviewer = true; | ||||
| } | } | ||||
| break; | break; | ||||
| case DifferentialReviewerStatus::STATUS_REJECTED_OLDER: | case DifferentialReviewerStatus::STATUS_REJECTED_OLDER: | ||||
| $has_rejecting_older_reviewer = true; | $has_rejecting_older_reviewer = true; | ||||
| break; | break; | ||||
| case DifferentialReviewerStatus::STATUS_BLOCKING: | case DifferentialReviewerStatus::STATUS_BLOCKING: | ||||
| $has_blocking_reviewer = true; | $has_blocking_reviewer = true; | ||||
| break; | break; | ||||
| case DifferentialReviewerStatus::STATUS_ACCEPTED: | case DifferentialReviewerStatus::STATUS_ACCEPTED: | ||||
| if ($reviewer->isUser()) { | if ($reviewer->isUser()) { | ||||
| $active_phid = $active_diff->getPHID(); | $active_phid = $active_diff->getPHID(); | ||||
| if ($reviewer->isAccepted($active_phid)) { | if ($reviewer->isAccepted($active_phid)) { | ||||
| $has_accepting_user = true; | $has_accepting_user = true; | ||||
| } | } | ||||
| } | } | ||||
| break; | break; | ||||
| } | } | ||||
| } | } | ||||
| $new_status = null; | $new_status = null; | ||||
| if ($has_accepting_user && | if ($has_accepting_user && | ||||
| !$has_rejecting_reviewer && | !$has_rejecting_reviewer && | ||||
| !$has_rejecting_older_reviewer && | !$has_rejecting_older_reviewer && | ||||
| !$has_blocking_reviewer) { | !$has_blocking_reviewer) { | ||||
| $new_status = $status_accepted; | $new_status = DifferentialRevisionStatus::ACCEPTED; | ||||
| } else if ($has_rejecting_reviewer) { | } else if ($has_rejecting_reviewer) { | ||||
| // This isn't accepted, and there's at least one rejecting reviewer, | // This isn't accepted, and there's at least one rejecting reviewer, | ||||
| // so the revision needs changes. This usually happens after a | // so the revision needs changes. This usually happens after a | ||||
| // "reject". | // "reject". | ||||
| $new_status = $status_revision; | $new_status = DifferentialRevisionStatus::NEEDS_REVISION; | ||||
| } else if ($old_status == $status_accepted) { | } else if ($was_accepted) { | ||||
| // This revision was accepted, but it no longer satisfies the | // This revision was accepted, but it no longer satisfies the | ||||
| // conditions for acceptance. This usually happens after an accepting | // conditions for acceptance. This usually happens after an accepting | ||||
| // reviewer resigns or is removed. | // reviewer resigns or is removed. | ||||
| $new_status = $status_review; | $new_status = DifferentialRevisionStatus::NEEDS_REVIEW; | ||||
| } | |||||
| if ($new_status === null) { | |||||
| return $xactions; | |||||
| } | |||||
| $old_legacy_status = $revision->getStatus(); | |||||
| $revision->setModernRevisionStatus($new_status); | |||||
| $new_legacy_status = $revision->getStatus(); | |||||
| if ($new_legacy_status == $old_legacy_status) { | |||||
| return $xactions; | |||||
| } | } | ||||
| if ($new_status !== null && ($new_status != $old_status)) { | |||||
| $xaction = id(new DifferentialTransaction()) | $xaction = id(new DifferentialTransaction()) | ||||
| ->setTransactionType( | ->setTransactionType( | ||||
| DifferentialRevisionStatusTransaction::TRANSACTIONTYPE) | DifferentialRevisionStatusTransaction::TRANSACTIONTYPE) | ||||
| ->setOldValue($old_status) | ->setOldValue($old_legacy_status) | ||||
| ->setNewValue($new_status); | ->setNewValue($new_legacy_status); | ||||
| $xaction = $this->populateTransaction($object, $xaction)->save(); | |||||
| $xaction = $this->populateTransaction($revision, $xaction) | |||||
| ->save(); | |||||
| $xactions[] = $xaction; | $xactions[] = $xaction; | ||||
| $object->setStatus($new_status)->save(); | // Save the status adjustment we made earlier. | ||||
| } | // TODO: This can be a little cleaner and more obvious once storage | ||||
| break; | // migrates. | ||||
| default: | $revision->save(); | ||||
| // Revisions can't transition out of other statuses (like closed or | |||||
| // abandoned) as a side effect of reviewer status changes. | |||||
| break; | |||||
| } | |||||
| $this->markReviewerComments($object, $xactions); | |||||
| return $xactions; | return $xactions; | ||||
| } | } | ||||
| protected function validateTransaction( | protected function validateTransaction( | ||||
| PhabricatorLiskDAO $object, | PhabricatorLiskDAO $object, | ||||
| $type, | $type, | ||||
| array $xactions) { | array $xactions) { | ||||
| $errors = parent::validateTransaction($object, $type, $xactions); | $errors = parent::validateTransaction($object, $type, $xactions); | ||||
| $config_self_accept_key = 'differential.allow-self-accept'; | $config_self_accept_key = 'differential.allow-self-accept'; | ||||
| ▲ Show 20 Lines • Show All 935 Lines • Show Last 20 Lines | |||||