diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
@@ -2,42 +2,6 @@
final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
- /**
- * Load the PHIDs for all objects the user has the authority to act as an
- * audit for. This includes themselves, and any packages they are an owner
- * of.
- */
- public static function loadAuditPHIDsForUser(PhabricatorUser $user) {
- $phids = array();
-
- // TODO: This method doesn't really use the right viewer, but in practice we
- // never issue this query of this type on behalf of another user and are
- // unlikely to do so in the future. This entire method should be refactored
- // into a Query class, however, and then we should use a proper viewer.
-
- // The user can audit on their own behalf.
- $phids[$user->getPHID()] = true;
-
- $owned_packages = id(new PhabricatorOwnersPackageQuery())
- ->setViewer($user)
- ->withAuthorityPHIDs(array($user->getPHID()))
- ->execute();
- foreach ($owned_packages as $package) {
- $phids[$package->getPHID()] = true;
- }
-
- // The user can audit on behalf of all projects they are a member of.
- $projects = id(new PhabricatorProjectQuery())
- ->setViewer($user)
- ->withMemberPHIDs(array($user->getPHID()))
- ->execute();
- foreach ($projects as $project) {
- $phids[$project->getPHID()] = true;
- }
-
- return array_keys($phids);
- }
-
public static function getMailThreading(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
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
@@ -78,6 +78,10 @@
}
}
+ $object->loadAndAttachAuditAuthority(
+ $this->getActor(),
+ $this->getActingAsPHID());
+
return parent::expandTransactions($object, $xactions);
}
diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -4,9 +4,6 @@
const CHANGES_LIMIT = 100;
- private $auditAuthorityPHIDs;
- private $highlightedAudits;
-
private $commitParents;
private $commitRefs;
private $commitMerges;
@@ -67,8 +64,7 @@
}
$audit_requests = $commit->getAudits();
- $this->auditAuthorityPHIDs =
- PhabricatorAuditCommentEditor::loadAuditPHIDsForUser($viewer);
+ $commit->loadAndAttachAuditAuthority($viewer);
$commit_data = $commit->getCommitData();
$is_foreign = $commit_data->getCommitDetail('foreign-svn-stub');
@@ -209,10 +205,6 @@
$timeline = $this->buildComments($commit);
$merge_table = $this->buildMergesTable($commit);
- $highlighted_audits = $commit->getAuthorityAudits(
- $viewer,
- $this->auditAuthorityPHIDs);
-
$show_changesets = false;
$info_panel = null;
$change_list = null;
@@ -520,13 +512,13 @@
if ($user_requests) {
$view->addProperty(
pht('Auditors'),
- $this->renderAuditStatusView($user_requests));
+ $this->renderAuditStatusView($commit, $user_requests));
}
if ($other_requests) {
$view->addProperty(
pht('Group Auditors'),
- $this->renderAuditStatusView($other_requests));
+ $this->renderAuditStatusView($commit, $other_requests));
}
}
@@ -848,12 +840,12 @@
return $file->getRedirectResponse();
}
- private function renderAuditStatusView(array $audit_requests) {
+ private function renderAuditStatusView(
+ PhabricatorRepositoryCommit $commit,
+ array $audit_requests) {
assert_instances_of($audit_requests, 'PhabricatorRepositoryAuditRequest');
$viewer = $this->getViewer();
- $authority_map = array_fill_keys($this->auditAuthorityPHIDs, true);
-
$view = new PHUIStatusListView();
foreach ($audit_requests as $request) {
$code = $request->getAuditStatus();
@@ -873,7 +865,7 @@
$target = $viewer->renderHandle($auditor_phid);
$item->setTarget($target);
- if (isset($authority_map[$auditor_phid])) {
+ if ($commit->hasAuditAuthority($viewer, $request)) {
$item->setHighlighted(true);
}
diff --git a/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php
--- a/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php
+++ b/src/applications/diffusion/xaction/DiffusionCommitAuditTransaction.php
@@ -91,6 +91,9 @@
$value,
$status) {
+ $actor = $this->getActor();
+ $acting_phid = $this->getActingAsPHID();
+
$audits = $commit->getAudits();
$audits = mpull($audits, null, 'getAuditorPHID');
@@ -98,13 +101,9 @@
$with_authority = ($status != PhabricatorAuditStatusConstants::RESIGNED);
if ($with_authority) {
- $has_authority = PhabricatorAuditCommentEditor::loadAuditPHIDsForUser(
- $viewer);
- $has_authority = array_fuse($has_authority);
foreach ($audits as $audit) {
- $auditor_phid = $audit->getAuditorPHID();
- if (isset($has_authority[$auditor_phid])) {
- $map[$auditor_phid] = $status;
+ if ($commit->hasAuditAuthority($actor, $audit, $acting_phid)) {
+ $map[$audit->getAuditorPHID()] = $status;
}
}
}
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -41,6 +41,7 @@
private $repository = self::ATTACHABLE;
private $customFields = self::ATTACHABLE;
private $drafts = array();
+ private $auditAuthorityPHIDs = array();
public function attachRepository(PhabricatorRepository $repository) {
$this->repository = $repository;
@@ -180,30 +181,85 @@
return $this->assertAttached($this->audits);
}
- public function getAuthorityAudits(
- PhabricatorUser $user,
- array $authority_phids) {
+ public function loadAndAttachAuditAuthority(
+ PhabricatorUser $viewer,
+ $actor_phid = null) {
- $authority = array_fill_keys($authority_phids, true);
- $audits = $this->getAudits();
- $authority_audits = array();
- foreach ($audits as $audit) {
- $has_authority = !empty($authority[$audit->getAuditorPHID()]);
- if ($has_authority) {
- $commit_author = $this->getAuthorPHID();
+ if ($actor_phid === null) {
+ $actor_phid = $viewer->getPHID();
+ }
+
+ // TODO: This method is a little weird and sketchy, but worlds better than
+ // what came before it. Eventually, this should probably live in a Query
+ // class.
- // You don't have authority over package and project audits on your
- // own commits.
+ // Figure out which requests the actor has authority over: these are user
+ // requests where they are the auditor, and packages and projects they are
+ // a member of.
- $auditor_is_user = ($audit->getAuditorPHID() == $user->getPHID());
- $user_is_author = ($commit_author == $user->getPHID());
+ if (!$actor_phid) {
+ $attach_key = $viewer->getCacheFragment();
+ $phids = array();
+ } else {
+ $attach_key = $actor_phid;
+ // At least currently, when modifying your own commits, you act only on
+ // behalf of yourself, not your packages/projects -- the idea being that
+ // you can't accept your own commits. This may change or depend on
+ // config.
+ $actor_is_author = ($actor_phid == $this->getAuthorPHID());
+ if ($actor_is_author) {
+ $phids = array($actor_phid);
+ } else {
+ $phids = array();
+ $phids[$actor_phid] = true;
+
+ $owned_packages = id(new PhabricatorOwnersPackageQuery())
+ ->setViewer($viewer)
+ ->withAuthorityPHIDs(array($actor_phid))
+ ->execute();
+ foreach ($owned_packages as $package) {
+ $phids[$package->getPHID()] = true;
+ }
- if ($auditor_is_user || !$user_is_author) {
- $authority_audits[$audit->getID()] = $audit;
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($viewer)
+ ->withMemberPHIDs(array($actor_phid))
+ ->execute();
+ foreach ($projects as $project) {
+ $phids[$project->getPHID()] = true;
}
+
+ $phids = array_keys($phids);
}
}
- return $authority_audits;
+
+ $this->auditAuthorityPHIDs[$attach_key] = array_fuse($phids);
+
+ return $this;
+ }
+
+ public function hasAuditAuthority(
+ PhabricatorUser $viewer,
+ PhabricatorRepositoryAuditRequest $audit,
+ $actor_phid = null) {
+
+ if ($actor_phid === null) {
+ $actor_phid = $viewer->getPHID();
+ }
+
+ if (!$actor_phid) {
+ $attach_key = $viewer->getCacheFragment();
+ } else {
+ $attach_key = $actor_phid;
+ }
+
+ $map = $this->assertAttachedKey($this->auditAuthorityPHIDs, $attach_key);
+
+ if (!$actor_phid) {
+ return false;
+ }
+
+ return isset($map[$audit->getAuditorPHID()]);
}
public function getAuditorPHIDsForEdit() {