1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00

Allow to uninstall (hide) Audit application

Summary:
While many Phorge applications can be uninstalled (such as Differential for pre-merge commit review), this has not been the case for Audit (post-merge commit review) since rP11861265fe94fa97e4d0563c5bdb7b8cac27282d.

In installations which do not use Audit in their workflows, exposing Audit related UI elements creates additional clutter and complexity.

Fixes T15129

Test Plan:
* Go to `/applications/view/PhabricatorAuditApplication/` and click "Uninstall"
* View an individual merged commit in Diffusion, click "Edit Commit" in the sidebar to go to `/diffusion/commit/edit/1/`, see that "Auditors" datasource is not displayed
* View an individual merged commit in Diffusion, click the "Add Action..." dropdown, see that the "Audit Action" section and its entries "Accept Commit" and "Raise Concern" are not displayed
* View an individual merged commit in Diffusion, click the "Add Action..." dropdown, see that the "Change Auditors" entry is not displayed
* Go to `/conduit/` and see that the API method "audit.query" and the entire section "audit" is not displayed
* Go to `/diffusion/commit/` and see no "Active Audits" and "Audited" searches in the left side bar
* Go to `/diffusion/commit/query/all/` and see that no "Auditors" entry is shown for each commit
* Go to `/diffusion/commit/query/all/` and see that no audit related entry is shown in the grey column on the right (if Harbormaster is not installed, there will be no grey column at all)
* Go to `/herald/edit/?content_type=commit&rule_type=global` and see that no Condition entry "Auditors" and no Condition entry "Affected packages that need audit" is displayed
* Go to `/herald/edit/?content_type=commit&rule_type=personal` and see that no "Add me as an auditor" Herald action and no "Added Auditors" Herald condition is displayed

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15129

Differential Revision: https://we.phorge.it/D25503
This commit is contained in:
Andre Klapper 2024-01-04 08:33:37 +01:00
parent b445e1d80d
commit 821708414e
11 changed files with 79 additions and 32 deletions

View file

@ -19,9 +19,7 @@ final class PhabricatorAuditApplication extends PhabricatorApplication {
} }
public function canUninstall() { public function canUninstall() {
// Audit was once a separate application, but has largely merged with return true;
// Diffusion.
return false;
} }
public function isPinnedByDefault(PhabricatorUser $viewer) { public function isPinnedByDefault(PhabricatorUser $viewer) {

View file

@ -4,7 +4,7 @@ abstract class AuditConduitAPIMethod extends ConduitAPIMethod {
final public function getApplication() { final public function getApplication() {
return PhabricatorApplication::getByClass( return PhabricatorApplication::getByClass(
'PhabricatorDiffusionApplication'); 'PhabricatorAuditApplication');
} }
} }

View file

@ -70,6 +70,7 @@ final class PhabricatorCommitSearchEngine
} }
protected function buildCustomSearchFields() { protected function buildCustomSearchFields() {
$show_audit_fields = (id(new PhabricatorAuditApplication())->isInstalled());
$show_packages = PhabricatorApplication::isClassInstalled( $show_packages = PhabricatorApplication::isClassInstalled(
'PhabricatorPackagesApplication'); 'PhabricatorPackagesApplication');
return array( return array(
@ -95,6 +96,7 @@ final class PhabricatorCommitSearchEngine
->setConduitKey('auditors') ->setConduitKey('auditors')
->setAliases(array('auditor', 'auditors', 'auditorPHID')) ->setAliases(array('auditor', 'auditors', 'auditorPHID'))
->setDatasource(new DiffusionAuditorFunctionDatasource()) ->setDatasource(new DiffusionAuditorFunctionDatasource())
->setIsHidden(!$show_audit_fields)
->setDescription( ->setDescription(
pht( pht(
'Find commits where given users, projects, or packages are '. 'Find commits where given users, projects, or packages are '.
@ -106,6 +108,7 @@ final class PhabricatorCommitSearchEngine
->setOptions(DiffusionCommitAuditStatus::newOptions()) ->setOptions(DiffusionCommitAuditStatus::newOptions())
->setDeprecatedOptions( ->setDeprecatedOptions(
DiffusionCommitAuditStatus::newDeprecatedOptions()) DiffusionCommitAuditStatus::newDeprecatedOptions())
->setIsHidden(!$show_audit_fields)
->setDescription(pht('Find commits with given audit statuses.')), ->setDescription(pht('Find commits with given audit statuses.')),
id(new PhabricatorSearchDatasourceField()) id(new PhabricatorSearchDatasourceField())
->setLabel(pht('Repositories')) ->setLabel(pht('Repositories'))
@ -172,9 +175,13 @@ final class PhabricatorCommitSearchEngine
$names = array(); $names = array();
if ($this->requireViewer()->isLoggedIn()) { if ($this->requireViewer()->isLoggedIn()) {
$names['active'] = pht('Active Audits'); if (id(new PhabricatorAuditApplication())->isInstalled()) {
$names['active'] = pht('Active Audits');
}
$names['authored'] = pht('Authored'); $names['authored'] = pht('Authored');
$names['audited'] = pht('Audited'); if (id(new PhabricatorAuditApplication())->isInstalled()) {
$names['audited'] = pht('Audited');
}
} }
$names['all'] = pht('All Commits'); $names['all'] = pht('All Commits');
@ -224,9 +231,11 @@ final class PhabricatorCommitSearchEngine
$bucket = $this->getResultBucket($query); $bucket = $this->getResultBucket($query);
// hide "Auditors" on /diffusion/commit/query/all/ if Audit not installed
$show_auditors = id(new PhabricatorAuditApplication())->isInstalled();
$template = id(new DiffusionCommitGraphView()) $template = id(new DiffusionCommitGraphView())
->setViewer($viewer) ->setViewer($viewer)
->setShowAuditors(true); ->setShowAuditors($show_auditors);
$views = array(); $views = array();
if ($bucket) { if ($bucket) {

View file

@ -100,19 +100,22 @@ final class DiffusionCommitEditEngine
$data = $object->getCommitData(); $data = $object->getCommitData();
$fields = array(); $fields = array();
// remove "Change Auditors" from "Add Action" dropdown etc
$fields[] = id(new PhabricatorDatasourceEditField()) // if Audit is not installed
->setKey('auditors') if (id(new PhabricatorAuditApplication())->isInstalled()) {
->setLabel(pht('Auditors')) $fields[] = id(new PhabricatorDatasourceEditField())
->setDatasource(new DiffusionAuditorDatasource()) ->setKey('auditors')
->setUseEdgeTransactions(true) ->setLabel(pht('Auditors'))
->setTransactionType( ->setDatasource(new DiffusionAuditorDatasource())
DiffusionCommitAuditorsTransaction::TRANSACTIONTYPE) ->setUseEdgeTransactions(true)
->setCommentActionLabel(pht('Change Auditors')) ->setTransactionType(
->setDescription(pht('Auditors for this commit.')) DiffusionCommitAuditorsTransaction::TRANSACTIONTYPE)
->setConduitDescription(pht('Change the auditors for this commit.')) ->setCommentActionLabel(pht('Change Auditors'))
->setConduitTypeDescription(pht('New auditors.')) ->setDescription(pht('Auditors for this commit.'))
->setValue($object->getAuditorPHIDsForEdit()); ->setConduitDescription(pht('Change the auditors for this commit.'))
->setConduitTypeDescription(pht('New auditors.'))
->setValue($object->getAuditorPHIDsForEdit());
}
$actions = DiffusionCommitActionTransaction::loadAllActions(); $actions = DiffusionCommitActionTransaction::loadAllActions();
$actions = msortv($actions, 'getCommitActionOrderVector'); $actions = msortv($actions, 'getCommitActionOrderVector');

View file

@ -9,8 +9,13 @@ final class DiffusionAuditorsAddAuditorsHeraldAction
return pht('Add auditors'); return pht('Add auditors');
} }
// hide "Add auditors" Herald action if Audit not installed
public function supportsRuleType($rule_type) { public function supportsRuleType($rule_type) {
return ($rule_type != HeraldRuleTypeConfig::RULE_TYPE_PERSONAL); if (id(new PhabricatorAuditApplication())->isInstalled()) {
return ($rule_type != HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
} else {
return false;
}
} }
public function applyEffect($object, HeraldEffect $effect) { public function applyEffect($object, HeraldEffect $effect) {

View file

@ -9,8 +9,13 @@ final class DiffusionAuditorsAddSelfHeraldAction
return pht('Add me as an auditor'); return pht('Add me as an auditor');
} }
// hide "Add me as an auditor" Herald action if Audit not installed
public function supportsRuleType($rule_type) { public function supportsRuleType($rule_type) {
return ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL); if (id(new PhabricatorAuditApplication())->isInstalled()) {
return ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
} else {
return false;
}
} }
public function applyEffect($object, HeraldEffect $effect) { public function applyEffect($object, HeraldEffect $effect) {

View file

@ -5,6 +5,15 @@ final class DiffusionCommitAuditorsHeraldField
const FIELDCONST = 'diffusion.commit.auditors'; const FIELDCONST = 'diffusion.commit.auditors';
// hide "Auditors" Herald condition if Audit not installed
public function supportsObject($object) {
if (id(new PhabricatorAuditApplication())->isInstalled()) {
return ($object instanceof PhabricatorRepositoryCommit);
} else {
return false;
}
}
public function getHeraldFieldName() { public function getHeraldFieldName() {
return pht('Auditors'); return pht('Auditors');
} }

View file

@ -5,6 +5,16 @@ final class DiffusionCommitPackageAuditHeraldField
const FIELDCONST = 'diffusion.commit.package.audit'; const FIELDCONST = 'diffusion.commit.package.audit';
// hide "Affected packages that need audit" Herald condition
// if Audit not installed
public function supportsObject($object) {
if (id(new PhabricatorAuditApplication())->isInstalled()) {
return ($object instanceof PhabricatorRepositoryCommit);
} else {
return false;
}
}
public function getHeraldFieldName() { public function getHeraldFieldName() {
return pht('Affected packages that need audit'); return pht('Affected packages that need audit');
} }

View file

@ -169,7 +169,10 @@ final class DiffusionCommitGraphView
$this->addBuildAction($item_view, $hash); $this->addBuildAction($item_view, $hash);
} }
$this->addAuditAction($item_view, $hash); // hide Audit entry on /diffusion/commit/query/all if Audit not installed
if (id(new PhabricatorAuditApplication())->isInstalled()) {
$this->addAuditAction($item_view, $hash);
}
if ($show_auditors) { if ($show_auditors) {
$auditor_list = $item_view->newMapView(); $auditor_list = $item_view->newMapView();

View file

@ -8,6 +8,9 @@ abstract class DiffusionCommitActionTransaction
} }
public function isActionAvailable($object, PhabricatorUser $viewer) { public function isActionAvailable($object, PhabricatorUser $viewer) {
if (!id(new PhabricatorAuditApplication())->isInstalled()) {
return false;
}
try { try {
$this->validateAction($object, $viewer); $this->validateAction($object, $viewer);
return true; return true;

View file

@ -275,16 +275,18 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
->setOptions($orders); ->setOptions($orders);
} }
$buckets = $this->newResultBuckets(); if (id(new PhabricatorAuditApplication())->isInstalled()) {
if ($query && $buckets) { $buckets = $this->newResultBuckets();
$bucket_options = array( if ($query && $buckets) {
self::BUCKET_NONE => pht('No Bucketing'), $bucket_options = array(
) + mpull($buckets, 'getResultBucketName'); self::BUCKET_NONE => pht('No Bucketing'),
) + mpull($buckets, 'getResultBucketName');
$fields[] = id(new PhabricatorSearchSelectField()) $fields[] = id(new PhabricatorSearchSelectField())
->setLabel(pht('Bucket')) ->setLabel(pht('Bucket'))
->setKey('bucket') ->setKey('bucket')
->setOptions($bucket_options); ->setOptions($bucket_options);
}
} }
$field_map = array(); $field_map = array();