From b941331bdfe611794cd7a4163f63b63df36d2c88 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 11 Jan 2017 15:25:41 -0800 Subject: [PATCH] Prevent users from resigning from audits they've already resigned from Summary: Ref T10978. Since "Resigned" is a status in Audit, you could repeatedly resign. This is confusing; prevent it. Test Plan: Tried to resign twice; was only allowed to resign once. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10978 Differential Revision: https://secure.phabricator.com/D17187 --- .../xaction/DiffusionCommitAuditTransaction.php | 17 +++++++++++++++++ .../DiffusionCommitResignTransaction.php | 6 +++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php index 64d32d1700..86656d85a6 100644 --- a/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php +++ b/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php @@ -13,6 +13,23 @@ abstract class DiffusionCommitAuditTransaction return ($this->getViewerAuditStatus($commit, $viewer) !== null); } + protected function isViewerAnyActiveAuditor( + PhabricatorRepositoryCommit $commit, + PhabricatorUser $viewer) { + + // This omits various inactive states like "Resigned" and "Not Required". + + return $this->isViewerAuditStatusAmong( + $commit, + $viewer, + array( + PhabricatorAuditStatusConstants::AUDIT_REQUIRED, + PhabricatorAuditStatusConstants::CONCERNED, + PhabricatorAuditStatusConstants::ACCEPTED, + PhabricatorAuditStatusConstants::AUDIT_REQUESTED, + )); + } + protected function isViewerAcceptingAuditor( PhabricatorRepositoryCommit $commit, PhabricatorUser $viewer) { diff --git a/src/applications/diffusion/xaction/DiffusionCommitResignTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitResignTransaction.php index 1a553e3471..4a68252414 100644 --- a/src/applications/diffusion/xaction/DiffusionCommitResignTransaction.php +++ b/src/applications/diffusion/xaction/DiffusionCommitResignTransaction.php @@ -28,7 +28,7 @@ final class DiffusionCommitResignTransaction public function generateOldValue($object) { $actor = $this->getActor(); - return !$this->isViewerAnyAuditor($object, $actor); + return !$this->isViewerAnyActiveAuditor($object, $actor); } public function applyExternalEffects($object, $value) { @@ -38,11 +38,11 @@ final class DiffusionCommitResignTransaction } protected function validateAction($object, PhabricatorUser $viewer) { - if (!$this->isViewerAnyAuditor($object, $viewer)) { + if (!$this->isViewerAnyActiveAuditor($object, $viewer)) { throw new Exception( pht( 'You can not resign from this commit because you are not an '. - 'auditor.')); + 'active auditor.')); } }