From 2636d84d0c24cbcf62fa70d4d0ed112844854e13 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 9 Mar 2021 15:11:28 -0800 Subject: [PATCH] Remove very old Audit status constants and AuditRequest data Summary: Ref T13631. See that task for discussion. - "NONE": Probably never used? - "CC": Obsoleted by subscribers. - "AUDIT_NOT_REQUIRED": For Owners packages, obsoleted by edges. - "CLOSED": For "Close Audit", obsoleted by "Request Verification". Test Plan: - Grepped for constants, browsed Diffusion. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13631 Differential Revision: https://secure.phabricator.com/D21598 --- .../20210309.auditors.01.status.sql | 5 ++++ .../PhabricatorAuditStatusConstants.php | 18 --------------- .../audit/editor/PhabricatorAuditEditor.php | 7 ------ .../controller/DiffusionCommitController.php | 4 ---- .../herald/DiffusionAuditorsHeraldAction.php | 4 +--- .../DiffusionCommitAuditorsHeraldField.php | 6 +++-- .../PhabricatorRepositoryAuditRequest.php | 23 ------------------- 7 files changed, 10 insertions(+), 57 deletions(-) create mode 100644 resources/sql/autopatches/20210309.auditors.01.status.sql diff --git a/resources/sql/autopatches/20210309.auditors.01.status.sql b/resources/sql/autopatches/20210309.auditors.01.status.sql new file mode 100644 index 0000000000..731ce3ca44 --- /dev/null +++ b/resources/sql/autopatches/20210309.auditors.01.status.sql @@ -0,0 +1,5 @@ +UPDATE {$NAMESPACE}_repository.repository_auditrequest + SET auditStatus = 'accepted' WHERE auditStatus = 'closed'; + +DELETE FROM {$NAMESPACE}_repository.repository_auditrequest + WHERE auditStatus IN ('', 'cc', 'audit-not-required'); diff --git a/src/applications/audit/constants/PhabricatorAuditStatusConstants.php b/src/applications/audit/constants/PhabricatorAuditStatusConstants.php index 920cfd875e..54eb332124 100644 --- a/src/applications/audit/constants/PhabricatorAuditStatusConstants.php +++ b/src/applications/audit/constants/PhabricatorAuditStatusConstants.php @@ -2,27 +2,19 @@ final class PhabricatorAuditStatusConstants extends Phobject { - const NONE = ''; - const AUDIT_NOT_REQUIRED = 'audit-not-required'; const AUDIT_REQUIRED = 'audit-required'; const CONCERNED = 'concerned'; const ACCEPTED = 'accepted'; const AUDIT_REQUESTED = 'requested'; const RESIGNED = 'resigned'; - const CLOSED = 'closed'; - const CC = 'cc'; public static function getStatusNameMap() { $map = array( - self::NONE => pht('Not Applicable'), - self::AUDIT_NOT_REQUIRED => pht('Audit Not Required'), self::AUDIT_REQUIRED => pht('Audit Required'), self::CONCERNED => pht('Concern Raised'), self::ACCEPTED => pht('Accepted'), self::AUDIT_REQUESTED => pht('Audit Requested'), self::RESIGNED => pht('Resigned'), - self::CLOSED => pht('Closed'), - self::CC => pht("Was CC'd"), ); return $map; @@ -51,12 +43,6 @@ final class PhabricatorAuditStatusConstants extends Phobject { case self::ACCEPTED: $color = 'green'; break; - case self::AUDIT_NOT_REQUIRED: - $color = 'blue'; - break; - case self::CLOSED: - $color = 'dark'; - break; case self::RESIGNED: $color = 'grey'; break; @@ -69,9 +55,6 @@ final class PhabricatorAuditStatusConstants extends Phobject { public static function getStatusIcon($code) { switch ($code) { - case self::AUDIT_NOT_REQUIRED: - $icon = PHUIStatusItemView::ICON_OPEN; - break; case self::AUDIT_REQUIRED: case self::AUDIT_REQUESTED: $icon = PHUIStatusItemView::ICON_WARNING; @@ -80,7 +63,6 @@ final class PhabricatorAuditStatusConstants extends Phobject { $icon = PHUIStatusItemView::ICON_REJECT; break; case self::ACCEPTED: - case self::CLOSED: $icon = PHUIStatusItemView::ICON_ACCEPT; break; case self::RESIGNED: diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 98c0d92624..81317fd47e 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -179,7 +179,6 @@ final class PhabricatorAuditEditor $object->attachAudits($commit->getAudits()); $status_concerned = PhabricatorAuditStatusConstants::CONCERNED; - $status_closed = PhabricatorAuditStatusConstants::CLOSED; $status_resigned = PhabricatorAuditStatusConstants::RESIGNED; $status_accepted = PhabricatorAuditStatusConstants::ACCEPTED; $status_concerned = PhabricatorAuditStatusConstants::CONCERNED; @@ -491,12 +490,6 @@ final class PhabricatorAuditEditor } foreach ($object->getAudits() as $audit) { - if (!$audit->isInteresting()) { - // Don't send mail to uninteresting auditors, like packages which - // own this code but which audits have not triggered for. - continue; - } - if (!$audit->isResigned()) { $phids[] = $audit->getAuditorPHID(); } diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 473e8911ac..6037781edc 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -598,10 +598,6 @@ final class DiffusionCommitController extends DiffusionController { $other_requests = array(); foreach ($audit_requests as $audit_request) { - if (!$audit_request->isInteresting()) { - continue; - } - if ($audit_request->isUser()) { $user_requests[] = $audit_request; } else { diff --git a/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php b/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php index 4ec23b1faa..bd75799f03 100644 --- a/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php +++ b/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php @@ -38,9 +38,7 @@ abstract class DiffusionAuditorsHeraldAction $current = array(); foreach ($auditors as $auditor) { - if ($auditor->isInteresting()) { - $current[] = $auditor->getAuditorPHID(); - } + $current[] = $auditor->getAuditorPHID(); } $allowed_types = array( diff --git a/src/applications/diffusion/herald/DiffusionCommitAuditorsHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitAuditorsHeraldField.php index 8deee3417e..5f0da133f8 100644 --- a/src/applications/diffusion/herald/DiffusionCommitAuditorsHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionCommitAuditorsHeraldField.php @@ -22,9 +22,11 @@ final class DiffusionCommitAuditorsHeraldField $phids = array(); foreach ($audits as $audit) { - if ($audit->isActiveAudit()) { - $phids[] = $audit->getAuditorPHID(); + if ($audit->isResigned()) { + continue; } + + $phids[] = $audit->getAuditorPHID(); } return $phids; diff --git a/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php b/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php index e05820c825..1165b03a3f 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php +++ b/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php @@ -49,29 +49,6 @@ final class PhabricatorRepositoryAuditRequest return $this->assertAttached($this->commit); } - public function isActiveAudit() { - switch ($this->getAuditStatus()) { - case PhabricatorAuditStatusConstants::NONE: - case PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED: - case PhabricatorAuditStatusConstants::RESIGNED: - case PhabricatorAuditStatusConstants::CLOSED: - case PhabricatorAuditStatusConstants::CC: - return false; - } - - return true; - } - - public function isInteresting() { - switch ($this->getAuditStatus()) { - case PhabricatorAuditStatusConstants::NONE: - case PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED: - return false; - } - - return true; - } - public function isResigned() { switch ($this->getAuditStatus()) { case PhabricatorAuditStatusConstants::RESIGNED: