From a102c9a0feeac2b215ca240e4771e242b3620459 Mon Sep 17 00:00:00 2001 From: tuomaspelkonen Date: Tue, 18 Oct 2011 23:35:45 -0700 Subject: [PATCH] Allow to resign from an accepted revision when you didn't accept the diff. Summary: Girish wants to be able to do this. Test Plan: Checked that I had the option in my sandbox on an accepted diff. Reviewers: epriestley, jungejason Reviewed By: jungejason CC: aran, jungejason, tuomaspelkonen, epriestley Differential Revision: 1020 --- .../DifferentialRevisionViewController.php | 3 +++ ...fferentialReviewedByFieldSpecification.php | 22 +++---------------- .../specification/reviewedby/__init__.php | 2 -- .../storage/revision/DifferentialRevision.php | 21 ++++++++++++++++++ .../storage/revision/__init__.php | 2 ++ 5 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php index 2215758b2b..e752acc52d 100644 --- a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php @@ -368,6 +368,7 @@ class DifferentialRevisionViewController extends DifferentialController { $viewer_phid = $this->getRequest()->getUser()->getPHID(); $viewer_is_owner = ($viewer_phid == $revision->getAuthorPHID()); $viewer_is_reviewer = in_array($viewer_phid, $revision->getReviewers()); + $viewer_did_accept = ($viewer_phid === $revision->loadReviewedBy()); if ($viewer_is_owner) { switch ($revision->getStatus()) { @@ -403,6 +404,8 @@ class DifferentialRevisionViewController extends DifferentialController { break; case DifferentialRevisionStatus::ACCEPTED: $actions[DifferentialAction::ACTION_REJECT] = true; + $actions[DifferentialAction::ACTION_RESIGN] = + $viewer_is_reviewer && !$viewer_did_accept; break; case DifferentialRevisionStatus::COMMITTED: case DifferentialRevisionStatus::ABANDONED: diff --git a/src/applications/differential/field/specification/reviewedby/DifferentialReviewedByFieldSpecification.php b/src/applications/differential/field/specification/reviewedby/DifferentialReviewedByFieldSpecification.php index 59feb8013c..8ce201756c 100644 --- a/src/applications/differential/field/specification/reviewedby/DifferentialReviewedByFieldSpecification.php +++ b/src/applications/differential/field/specification/reviewedby/DifferentialReviewedByFieldSpecification.php @@ -24,26 +24,10 @@ final class DifferentialReviewedByFieldSpecification protected function didSetRevision() { $this->reviewedBy = array(); $revision = $this->getRevision(); + $reviewer = $revision->loadReviewedBy(); - $status = $revision->getStatus(); - if ($status == DifferentialRevisionStatus::ACCEPTED || - $status == DifferentialRevisionStatus::COMMITTED) { - $reviewer = null; - $comments = $revision->loadComments(); - foreach ($comments as $comment) { - $action = $comment->getAction(); - if ($action == DifferentialAction::ACTION_ACCEPT) { - $reviewer = $comment->getAuthorPHID(); - } else if ($action == DifferentialAction::ACTION_REJECT || - $action == DifferentialAction::ACTION_ABANDON || - $action == DifferentialAction::ACTION_RETHINK) { - $reviewer = null; - } - } - - if ($reviewer) { - $this->reviewedBy = array($reviewer); - } + if ($reviewer) { + $this->reviewedBy = array($reviewer); } } diff --git a/src/applications/differential/field/specification/reviewedby/__init__.php b/src/applications/differential/field/specification/reviewedby/__init__.php index 5240ff8da0..a87a459dbd 100644 --- a/src/applications/differential/field/specification/reviewedby/__init__.php +++ b/src/applications/differential/field/specification/reviewedby/__init__.php @@ -6,8 +6,6 @@ -phutil_require_module('phabricator', 'applications/differential/constants/action'); -phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/field/specification/base'); diff --git a/src/applications/differential/storage/revision/DifferentialRevision.php b/src/applications/differential/storage/revision/DifferentialRevision.php index 79cfce3479..9ad6e4fb39 100644 --- a/src/applications/differential/storage/revision/DifferentialRevision.php +++ b/src/applications/differential/storage/revision/DifferentialRevision.php @@ -165,4 +165,25 @@ class DifferentialRevision extends DifferentialDAO { public function getUnsubscribedPHIDs() { return array_keys($this->getUnsubscribed()); } + + public function loadReviewedBy() { + $reviewer = null; + + if ($this->status == DifferentialRevisionStatus::ACCEPTED || + $this->status == DifferentialRevisionStatus::COMMITTED) { + $comments = $this->loadComments(); + foreach ($comments as $comment) { + $action = $comment->getAction(); + if ($action == DifferentialAction::ACTION_ACCEPT) { + $reviewer = $comment->getAuthorPHID(); + } else if ($action == DifferentialAction::ACTION_REJECT || + $action == DifferentialAction::ACTION_ABANDON || + $action == DifferentialAction::ACTION_RETHINK) { + $reviewer = null; + } + } + } + + return $reviewer; + } } diff --git a/src/applications/differential/storage/revision/__init__.php b/src/applications/differential/storage/revision/__init__.php index bee7373c37..7307f419a1 100644 --- a/src/applications/differential/storage/revision/__init__.php +++ b/src/applications/differential/storage/revision/__init__.php @@ -6,6 +6,8 @@ +phutil_require_module('phabricator', 'applications/differential/constants/action'); +phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/storage/base'); phutil_require_module('phabricator', 'applications/differential/storage/comment'); phutil_require_module('phabricator', 'applications/differential/storage/diff');