diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -671,6 +671,7 @@ 'DiffusionCommitRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php', 'DiffusionCommitRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionSubscribersHeraldField.php', 'DiffusionCommitSearchConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionCommitSearchConduitAPIMethod.php', + 'DiffusionCommitStateTransaction' => 'applications/diffusion/xaction/DiffusionCommitStateTransaction.php', 'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php', 'DiffusionCommitTransactionType' => 'applications/diffusion/xaction/DiffusionCommitTransactionType.php', 'DiffusionCompareController' => 'applications/diffusion/controller/DiffusionCompareController.php', @@ -5378,6 +5379,7 @@ 'DiffusionCommitRevisionReviewersHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRevisionSubscribersHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', + 'DiffusionCommitStateTransaction' => 'DiffusionCommitTransactionType', 'DiffusionCommitTagsController' => 'DiffusionController', 'DiffusionCommitTransactionType' => 'PhabricatorModularTransactionType', 'DiffusionCompareController' => 'DiffusionController', diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -361,14 +361,41 @@ } } + $old_status = $object->getAuditStatus(); + $requests = $object->getAudits(); $object->updateAuditStatus($requests); + + $new_status = $object->getAuditStatus(); + $object->save(); if ($import_status_flag) { $object->writeImportStatusFlag($import_status_flag); } + $partial_status = PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED; + + // If the commit has changed state after this edit, add an informational + // transaction about the state change. + if ($old_status !== $new_status) { + if ($new_status === $partial_status) { + // This state isn't interesting enough to get a transaction. The + // best way we could lead the user forward is something like "This + // commit still requires additional audits." but that's redundant and + // probably not very useful. + } else { + $xaction = $object->getApplicationTransactionTemplate() + ->setTransactionType(DiffusionCommitStateTransaction::TRANSACTIONTYPE) + ->setOldValue($old_status) + ->setNewValue($new_status); + + $xaction = $this->populateTransaction($object, $xaction); + + $xaction->save(); + } + } + // Collect auditor PHIDs for building mail. $this->auditorPHIDs = mpull($object->getAudits(), 'getAuditorPHID'); diff --git a/src/applications/audit/storage/PhabricatorAuditTransaction.php b/src/applications/audit/storage/PhabricatorAuditTransaction.php --- a/src/applications/audit/storage/PhabricatorAuditTransaction.php +++ b/src/applications/audit/storage/PhabricatorAuditTransaction.php @@ -498,4 +498,22 @@ } return $tags; } + + public function shouldDisplayGroupWith(array $group) { + // Make the "This commit now requires audit." state message stand alone. + $type_state = DiffusionCommitStateTransaction::TRANSACTIONTYPE; + + if ($this->getTransactionType() == $type_state) { + return false; + } + + foreach ($group as $xaction) { + if ($xaction->getTransactionType() == $type_state) { + return false; + } + } + + return parent::shouldDisplayGroupWith($group); + } + } diff --git a/src/applications/diffusion/xaction/DiffusionCommitStateTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitStateTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/xaction/DiffusionCommitStateTransaction.php @@ -0,0 +1,66 @@ +getNewValue(); + return PhabricatorAuditCommitStatusConstants::getStatusIcon($new); + } + + public function getColor() { + $new = $this->getNewValue(); + return PhabricatorAuditCommitStatusConstants::getStatusColor($new); + } + + public function getTitle() { + $new = $this->getNewValue(); + + switch ($new) { + case PhabricatorAuditCommitStatusConstants::NONE: + return pht('This commit no longer requires audit.'); + case PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT: + return pht('This commit now requires audit.'); + case PhabricatorAuditCommitStatusConstants::CONCERN_RAISED: + return pht('This commit now has outstanding concerns.'); + case PhabricatorAuditCommitStatusConstants::FULLY_AUDITED: + return pht('All concerns with this commit have now been addressed.'); + } + + return null; + } + + public function getTitleForFeed() { + $new = $this->getNewValue(); + + switch ($new) { + case PhabricatorAuditCommitStatusConstants::NONE: + return pht( + '%s no longer requires audit.', + $this->renderObject()); + case PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT: + return pht( + '%s now requires audit.', + $this->renderObject()); + case PhabricatorAuditCommitStatusConstants::CONCERN_RAISED: + return pht( + '%s now has outstanding concerns.', + $this->renderObject()); + case PhabricatorAuditCommitStatusConstants::FULLY_AUDITED: + return pht( + 'All concerns with %s have now been addressed.', + $this->renderObject()); + } + + return null; + } + +}