Page MenuHomePhabricator
Files F15379719

D20127.id48136.diff
No OneTemporary
Actions

D20127.id48136.diff
View Options

diff --git a/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php b/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php
--- a/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php
+++ b/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php
@@ -105,10 +105,10 @@
$query->withPHIDs(mpull($commits, 'getPHID'));
}
- $commits = $query->execute();
- $commits = mpull($commits, null, 'getPHID');
+ $commit_iterator = new PhabricatorQueryIterator($query);
+
$audits = array();
- foreach ($commits as $commit) {
+ foreach ($commit_iterator as $commit) {
$commit_audits = $commit->getAudits();
foreach ($commit_audits as $key => $audit) {
if ($id_map && empty($id_map[$audit->getID()])) {
@@ -131,51 +131,87 @@
continue;
}
}
- $audits[] = $commit_audits;
- }
- $audits = array_mergev($audits);
- $console = PhutilConsole::getConsole();
-
- if (!$audits) {
- $console->writeErr("%s\n", pht('No audits match the query.'));
- return 0;
- }
-
- $handles = id(new PhabricatorHandleQuery())
- ->setViewer($this->getViewer())
- ->withPHIDs(mpull($audits, 'getAuditorPHID'))
- ->execute();
+ if (!$commit_audits) {
+ continue;
+ }
+ $handles = id(new PhabricatorHandleQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(mpull($commit_audits, 'getAuditorPHID'))
+ ->execute();
- foreach ($audits as $audit) {
- $commit = $commits[$audit->getCommitPHID()];
+ foreach ($commit_audits as $audit) {
+ $audit_id = $audit->getID();
- $console->writeOut(
- "%s\n",
- sprintf(
+ $description = sprintf(
'%10d %-16s %-16s %s: %s',
- $audit->getID(),
+ $audit_id,
$handles[$audit->getAuditorPHID()]->getName(),
PhabricatorAuditStatusConstants::getStatusName(
$audit->getAuditStatus()),
$commit->getRepository()->formatCommitName(
$commit->getCommitIdentifier()),
- trim($commit->getSummary())));
+ trim($commit->getSummary()));
+
+ $audits[] = array(
+ 'auditID' => $audit_id,
+ 'commitPHID' => $commit->getPHID(),
+ 'description' => $description,
+ );
+ }
}
- if (!$is_dry_run) {
- $message = pht(
- 'Really delete these %d audit(s)? They will be permanently deleted '.
- 'and can not be recovered.',
- count($audits));
- if ($console->confirm($message)) {
- foreach ($audits as $audit) {
- $id = $audit->getID();
- $console->writeOut("%s\n", pht('Deleting audit %d...', $id));
- $audit->delete();
- }
+ if (!$audits) {
+ echo tsprintf(
+ "%s\n",
+ pht('No audits match the query.'));
+ return 0;
+ }
+
+ foreach ($audits as $audit_spec) {
+ echo tsprintf(
+ "%s\n",
+ $audit_spec['description']);
+ }
+
+ if ($is_dry_run) {
+ echo tsprintf(
+ "%s\n",
+ pht('This is a dry run, so no changes will be made.'));
+ return 0;
+ }
+
+ $message = pht(
+ 'Really delete these %s audit(s)? They will be permanently deleted '.
+ 'and can not be recovered.',
+ phutil_count($audits));
+ if (!phutil_console_confirm($message)) {
+ echo tsprintf(
+ "%s\n",
+ pht('User aborted the workflow.'));
+ return 1;
+ }
+
+ $audits_by_commit = igroup($audits, 'commitPHID');
+ foreach ($audits_by_commit as $commit_phid => $audit_specs) {
+ $audit_ids = ipull($audit_specs, 'auditID');
+
+ $audits = id(new PhabricatorRepositoryAuditRequest())->loadAllWhere(
+ 'id IN (%Ld)',
+ $audit_ids);
+
+ foreach ($audits as $audit) {
+ $id = $audit->getID();
+
+ echo tsprintf(
+ "%s\n",
+ pht('Deleting audit %d...', $id));
+
+ $audit->delete();
}
+
+ $this->synchronizeCommitAuditState($commit_phid);
}
return 0;
diff --git a/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php b/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php
--- a/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php
+++ b/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php
@@ -87,4 +87,39 @@
return $commits;
}
+ protected function synchronizeCommitAuditState($commit_phid) {
+ $viewer = $this->getViewer();
+
+ $commit = id(new DiffusionCommitQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($commit_phid))
+ ->needAuditRequests(true)
+ ->executeOne();
+ if (!$commit) {
+ return;
+ }
+
+ $old_status = $commit->getAuditStatusObject();
+ $commit->updateAuditStatus($commit->getAudits());
+ $new_status = $commit->getAuditStatusObject();
+
+ if ($old_status->getKey() == $new_status->getKey()) {
+ echo tsprintf(
+ "%s\n",
+ pht(
+ 'No synchronization changes for "%s".',
+ $commit->getDisplayName()));
+ } else {
+ echo tsprintf(
+ "%s\n",
+ pht(
+ 'Synchronizing "%s": "%s" -> "%s".',
+ $commit->getDisplayName(),
+ $old_status->getName(),
+ $new_status->getName()));
+
+ $commit->save();
+ }
+ }
+
}
diff --git a/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php b/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php
--- a/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php
+++ b/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php
@@ -6,8 +6,16 @@
protected function didConstruct() {
$this
->setName('synchronize')
- ->setExamples('**synchronize** ...')
- ->setSynopsis(pht('Update audit status for commits.'))
+ ->setExamples(
+ "**synchronize** __repository__ ...\n".
+ "**synchronize** __commit__ ...\n".
+ "**synchronize** --all")
+ ->setSynopsis(
+ pht(
+ 'Update commits to make their summary audit state reflect the '.
+ 'state of their actual audit requests. This can fix inconsistencies '.
+ 'in database state if audit requests have been mangled '.
+ 'accidentally (or on purpose).'))
->setArguments(
array_merge(
$this->getCommitConstraintArguments(),
@@ -21,36 +29,7 @@
foreach ($objects as $object) {
$commits = $this->loadCommitsForConstraintObject($object);
foreach ($commits as $commit) {
- $commit = id(new DiffusionCommitQuery())
- ->setViewer($viewer)
- ->withPHIDs(array($commit->getPHID()))
- ->needAuditRequests(true)
- ->executeOne();
- if (!$commit) {
- continue;
- }
-
- $old_status = $commit->getAuditStatusObject();
- $commit->updateAuditStatus($commit->getAudits());
- $new_status = $commit->getAuditStatusObject();
-
- if ($old_status->getKey() == $new_status->getKey()) {
- echo tsprintf(
- "%s\n",
- pht(
- 'No changes for "%s".',
- $commit->getDisplayName()));
- } else {
- echo tsprintf(
- "%s\n",
- pht(
- 'Updating "%s": "%s" -> "%s".',
- $commit->getDisplayName(),
- $old_status->getName(),
- $new_status->getName()));
-
- $commit->save();
- }
+ $this->synchronizeCommitAuditState($commit->getPHID());
}
}
}
diff --git a/src/docs/user/userguide/audit.diviner b/src/docs/user/userguide/audit.diviner
--- a/src/docs/user/userguide/audit.diviner
+++ b/src/docs/user/userguide/audit.diviner
@@ -175,16 +175,13 @@
incorrectly (for example, because a package or Herald rule was configured in an
overbroad way).
-After deleting audits, you may want to run `bin/audit synchronize` to
-synchronize audit state.
-
**Synchronize Audit State**: Synchronize the audit state of commits to the
current open audit requests with `bin/audit synchronize`.
Normally, overall audit state is automatically kept up to date as changes are
-made to an audit. However, if you delete audits or manually update the database
-to make changes to audit request state, the state of corresponding commits may
-no longer be correct.
+made to an audit. However, if you manually update the database to make changes
+to audit request state, the state of corresponding commits may no longer be
+consistent.
This command will update commits so their overall audit state reflects the
cumulative state of their actual audit requests.

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 14, 10:59 PM (3 d, 6 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7675099
Default Alt Text
D20127.id48136.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility