D17192.diff
No OneTemporary
Actions

Size

36 KB

Referenced Files

None

Subscribers

None

D17192.diff
View Options

	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
	@@ -657,7 +657,9 @@
	'DiffusionCommitRemarkupRuleTestCase' => 'applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php',
	'DiffusionCommitRepositoryHeraldField' => 'applications/diffusion/herald/DiffusionCommitRepositoryHeraldField.php',
	'DiffusionCommitRepositoryProjectsHeraldField' => 'applications/diffusion/herald/DiffusionCommitRepositoryProjectsHeraldField.php',
	+ 'DiffusionCommitRequiredActionResultBucket' => 'applications/diffusion/query/DiffusionCommitRequiredActionResultBucket.php',
	'DiffusionCommitResignTransaction' => 'applications/diffusion/xaction/DiffusionCommitResignTransaction.php',
	+ 'DiffusionCommitResultBucket' => 'applications/diffusion/query/DiffusionCommitResultBucket.php',
	'DiffusionCommitRevertedByCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertedByCommitEdgeType.php',
	'DiffusionCommitRevertsCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php',
	'DiffusionCommitReviewerHeraldField' => 'applications/diffusion/herald/DiffusionCommitReviewerHeraldField.php',
	@@ -5361,7 +5363,9 @@
	'DiffusionCommitRemarkupRuleTestCase' => 'PhabricatorTestCase',
	'DiffusionCommitRepositoryHeraldField' => 'DiffusionCommitHeraldField',
	'DiffusionCommitRepositoryProjectsHeraldField' => 'DiffusionCommitHeraldField',
	+ 'DiffusionCommitRequiredActionResultBucket' => 'DiffusionCommitResultBucket',
	'DiffusionCommitResignTransaction' => 'DiffusionCommitAuditTransaction',
	+ 'DiffusionCommitResultBucket' => 'PhabricatorSearchResultBucket',
	'DiffusionCommitRevertedByCommitEdgeType' => 'PhabricatorEdgeType',
	'DiffusionCommitRevertsCommitEdgeType' => 'PhabricatorEdgeType',
	'DiffusionCommitReviewerHeraldField' => 'DiffusionCommitHeraldField',
	diff --git a/src/applications/audit/application/PhabricatorAuditApplication.php b/src/applications/audit/application/PhabricatorAuditApplication.php
	--- a/src/applications/audit/application/PhabricatorAuditApplication.php
	+++ b/src/applications/audit/application/PhabricatorAuditApplication.php
	@@ -34,57 +34,4 @@
	return 0.130;
	}

	- public function loadStatus(PhabricatorUser $user) {
	- $status = array();
	- $limit = self::MAX_STATUS_ITEMS;
	-
	- $phids = PhabricatorAuditCommentEditor::loadAuditPHIDsForUser($user);
	-
	- $query = id(new DiffusionCommitQuery())
	- ->setViewer($user)
	- ->withAuthorPHIDs(array($user->getPHID()))
	- ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_CONCERN)
	- ->setLimit($limit);
	- $commits = $query->execute();
	-
	- $count = count($commits);
	- if ($count >= $limit) {
	- $count_str = pht('%s+ Problem Commits', new PhutilNumber($limit - 1));
	- } else {
	- $count_str = pht('%s Problem Commit(s)', new PhutilNumber($count));
	- }
	-
	- $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION;
	- $status[] = id(new PhabricatorApplicationStatusView())
	- ->setType($type)
	- ->setText($count_str)
	- ->setCount($count);
	-
	- $query = id(new DiffusionCommitQuery())
	- ->setViewer($user)
	- ->withNeedsAuditByPHIDs($phids)
	- ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_OPEN)
	- ->setLimit($limit);
	- $commits = $query->execute();
	-
	- $count = count($commits);
	- if ($count >= $limit) {
	- $count_str = pht(
	- '%s+ Commits Awaiting Audit',
	- new PhutilNumber($limit - 1));
	- } else {
	- $count_str = pht(
	- '%s Commit(s) Awaiting Audit',
	- new PhutilNumber($count));
	- }
	-
	- $type = PhabricatorApplicationStatusView::TYPE_WARNING;
	- $status[] = id(new PhabricatorApplicationStatusView())
	- ->setType($type)
	- ->setText($count_str)
	- ->setCount($count);
	-
	- return $status;
	- }
	-
	}
	diff --git a/src/applications/audit/conduit/AuditQueryConduitAPIMethod.php b/src/applications/audit/conduit/AuditQueryConduitAPIMethod.php
	--- a/src/applications/audit/conduit/AuditQueryConduitAPIMethod.php
	+++ b/src/applications/audit/conduit/AuditQueryConduitAPIMethod.php
	@@ -2,6 +2,12 @@

	final class AuditQueryConduitAPIMethod extends AuditConduitAPIMethod {

	+ const AUDIT_LEGACYSTATUS_ANY = 'audit-status-any';
	+ const AUDIT_LEGACYSTATUS_OPEN = 'audit-status-open';
	+ const AUDIT_LEGACYSTATUS_CONCERN = 'audit-status-concern';
	+ const AUDIT_LEGACYSTATUS_ACCEPTED = 'audit-status-accepted';
	+ const AUDIT_LEGACYSTATUS_PARTIAL = 'audit-status-partial';
	+
	public function getAPIMethodName() {
	return 'audit.query';
	}
	@@ -10,13 +16,23 @@
	return pht('Query audit requests.');
	}

	+ public function getMethodStatus() {
	+ return self::METHOD_STATUS_FROZEN;
	+ }
	+
	+ public function getMethodStatusDescription() {
	+ return pht(
	+ 'This method is frozen and will eventually be deprecated. New code '.
	+ 'should use "diffusion.commit.search" instead.');
	+ }
	+
	protected function defineParamTypes() {
	$statuses = array(
	- DiffusionCommitQuery::AUDIT_STATUS_ANY,
	- DiffusionCommitQuery::AUDIT_STATUS_OPEN,
	- DiffusionCommitQuery::AUDIT_STATUS_CONCERN,
	- DiffusionCommitQuery::AUDIT_STATUS_ACCEPTED,
	- DiffusionCommitQuery::AUDIT_STATUS_PARTIAL,
	+ self::AUDIT_LEGACYSTATUS_ANY,
	+ self::AUDIT_LEGACYSTATUS_OPEN,
	+ self::AUDIT_LEGACYSTATUS_CONCERN,
	+ self::AUDIT_LEGACYSTATUS_ACCEPTED,
	+ self::AUDIT_LEGACYSTATUS_PARTIAL,
	);
	$status_const = $this->formatStringConstants($statuses);

	@@ -50,10 +66,26 @@
	$query->withPHIDs($commit_phids);
	}

	- $status = $request->getValue(
	- 'status',
	- DiffusionCommitQuery::AUDIT_STATUS_ANY);
	- $query->withAuditStatus($status);
	+ $status_map = array(
	+ self::AUDIT_LEGACYSTATUS_OPEN => array(
	+ PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT,
	+ PhabricatorAuditCommitStatusConstants::CONCERN_RAISED,
	+ ),
	+ self::AUDIT_LEGACYSTATUS_CONCERN => array(
	+ PhabricatorAuditCommitStatusConstants::CONCERN_RAISED,
	+ ),
	+ self::AUDIT_LEGACYSTATUS_ACCEPTED => array(
	+ PhabricatorAuditCommitStatusConstants::CONCERN_ACCEPTED,
	+ ),
	+ self::AUDIT_LEGACYSTATUS_PARTIAL => array(
	+ PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED,
	+ ),
	+ );
	+
	+ $status = $request->getValue('status');
	+ if (isset($status_map[$status])) {
	+ $query->withStatuses($status_map[$status]);
	+ }

	// NOTE: These affect the number of commits identified, which is sort of
	// reasonable but means the method may return an arbitrary number of
	diff --git a/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php b/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php
	--- a/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php
	+++ b/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php
	@@ -28,6 +28,7 @@
	return array(
	self::CONCERN_RAISED,
	self::NEEDS_AUDIT,
	+ self::PARTIALLY_AUDITED,
	);
	}

	diff --git a/src/applications/audit/constants/PhabricatorAuditStatusConstants.php b/src/applications/audit/constants/PhabricatorAuditStatusConstants.php
	--- a/src/applications/audit/constants/PhabricatorAuditStatusConstants.php
	+++ b/src/applications/audit/constants/PhabricatorAuditStatusConstants.php
	@@ -28,6 +28,13 @@
	return $map;
	}

	+ public static function getActionRequiredStatusConstants() {
	+ return array(
	+ self::AUDIT_REQUIRED,
	+ self::AUDIT_REQUESTED,
	+ );
	+ }
	+
	public static function getStatusName($code) {
	return idx(self::getStatusNameMap(), $code, pht('Unknown'));
	}
	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
	@@ -67,9 +67,6 @@
	$ids = $this->parseList($args->getArg('ids'));

	$status = $args->getArg('status');
	- if (!$status) {
	- $status = DiffusionCommitQuery::AUDIT_STATUS_OPEN;
	- }

	$min_date = $this->loadDate($args->getArg('min-commit-date'));
	$max_date = $this->loadDate($args->getArg('max-commit-date'));
	@@ -85,7 +82,7 @@
	->needAuditRequests(true);

	if ($status) {
	- $query->withAuditStatus($status);
	+ $query->withStatuses(array($status));
	}

	$id_map = array();
	diff --git a/src/applications/audit/query/PhabricatorCommitSearchEngine.php b/src/applications/audit/query/PhabricatorCommitSearchEngine.php
	--- a/src/applications/audit/query/PhabricatorCommitSearchEngine.php
	+++ b/src/applications/audit/query/PhabricatorCommitSearchEngine.php
	@@ -17,23 +17,27 @@
	->needCommitData(true);
	}

	+ protected function newResultBuckets() {
	+ return DiffusionCommitResultBucket::getAllResultBuckets();
	+ }
	+
	protected function buildQueryFromParameters(array $map) {
	$query = $this->newQuery();

	- if ($map['needsAuditByPHIDs']) {
	- $query->withNeedsAuditByPHIDs($map['needsAuditByPHIDs']);
	+ if ($map['responsiblePHIDs']) {
	+ $query->withResponsiblePHIDs($map['responsiblePHIDs']);
	}

	if ($map['auditorPHIDs']) {
	$query->withAuditorPHIDs($map['auditorPHIDs']);
	}

	- if ($map['commitAuthorPHIDs']) {
	- $query->withAuthorPHIDs($map['commitAuthorPHIDs']);
	+ if ($map['authorPHIDs']) {
	+ $query->withAuthorPHIDs($map['authorPHIDs']);
	}

	- if ($map['auditStatus']) {
	- $query->withAuditStatus($map['auditStatus']);
	+ if ($map['statuses']) {
	+ $query->withStatuses($map['statuses']);
	}

	if ($map['repositoryPHIDs']) {
	@@ -46,28 +50,32 @@
	protected function buildCustomSearchFields() {
	return array(
	id(new PhabricatorSearchDatasourceField())
	- ->setLabel(pht('Needs Audit By'))
	- ->setKey('needsAuditByPHIDs')
	- ->setAliases(array('needs', 'need'))
	- ->setDatasource(new DiffusionAuditorFunctionDatasource()),
	+ ->setLabel(pht('Responsible Users'))
	+ ->setKey('responsiblePHIDs')
	+ ->setConduitKey('responsible')
	+ ->setAliases(array('responsible', 'responsibles', 'responsiblePHID'))
	+ ->setDatasource(new DifferentialResponsibleDatasource()),
	+ id(new PhabricatorUsersSearchField())
	+ ->setLabel(pht('Authors'))
	+ ->setKey('authorPHIDs')
	+ ->setConduitKey('authors')
	+ ->setAliases(array('author', 'authors', 'authorPHID')),
	id(new PhabricatorSearchDatasourceField())
	->setLabel(pht('Auditors'))
	->setKey('auditorPHIDs')
	- ->setAliases(array('auditor', 'auditors'))
	+ ->setConduitKey('auditors')
	+ ->setAliases(array('auditor', 'auditors', 'auditorPHID'))
	->setDatasource(new DiffusionAuditorFunctionDatasource()),
	- id(new PhabricatorUsersSearchField())
	- ->setLabel(pht('Authors'))
	- ->setKey('commitAuthorPHIDs')
	- ->setAliases(array('author', 'authors')),
	- id(new PhabricatorSearchSelectField())
	+ id(new PhabricatorSearchCheckboxesField())
	->setLabel(pht('Audit Status'))
	- ->setKey('auditStatus')
	+ ->setKey('statuses')
	->setAliases(array('status'))
	- ->setOptions($this->getAuditStatusOptions()),
	+ ->setOptions(PhabricatorAuditCommitStatusConstants::getStatusNameMap()),
	id(new PhabricatorSearchDatasourceField())
	->setLabel(pht('Repositories'))
	->setKey('repositoryPHIDs')
	- ->setAliases(array('repository', 'repositories'))
	+ ->setConduitKey('repositories')
	+ ->setAliases(array('repository', 'repositories', 'repositoryPHID'))
	->setDatasource(new DiffusionRepositoryDatasource()),
	);
	}
	@@ -80,14 +88,9 @@
	$names = array();

	if ($this->requireViewer()->isLoggedIn()) {
	- $names['need'] = pht('Needs Audit');
	- $names['problem'] = pht('Problem Commits');
	- }
	-
	- $names['open'] = pht('Open Audits');
	-
	- if ($this->requireViewer()->isLoggedIn()) {
	- $names['authored'] = pht('Authored Commits');
	+ $names['active'] = pht('Active Audits');
	+ $names['authored'] = pht('Authored');
	+ $names['audited'] = pht('Audited');
	}

	$names['all'] = pht('All Commits');
	@@ -101,76 +104,75 @@
	$viewer = $this->requireViewer();

	$viewer_phid = $viewer->getPHID();
	- $status_open = DiffusionCommitQuery::AUDIT_STATUS_OPEN;
	-
	switch ($query_key) {
	case 'all':
	return $query;
	- case 'open':
	- $query->setParameter('auditStatus', $status_open);
	- return $query;
	- case 'need':
	- $needs_tokens = array(
	- $viewer_phid,
	- 'projects('.$viewer_phid.')',
	- 'packages('.$viewer_phid.')',
	- );
	-
	- $query->setParameter('needsAuditByPHIDs', $needs_tokens);
	- $query->setParameter('auditStatus', $status_open);
	+ case 'active':
	+ $bucket_key = DiffusionCommitRequiredActionResultBucket::BUCKETKEY;
	+
	+ $open = PhabricatorAuditCommitStatusConstants::getOpenStatusConstants();
	+
	+ $query
	+ ->setParameter('responsiblePHIDs', array($viewer_phid))
	+ ->setParameter('statuses', $open)
	+ ->setParameter('bucket', $bucket_key);
	return $query;
	case 'authored':
	- $query->setParameter('commitAuthorPHIDs', array($viewer->getPHID()));
	+ $query
	+ ->setParameter('authorPHIDs', array($viewer_phid));
	return $query;
	- case 'problem':
	- $query->setParameter('commitAuthorPHIDs', array($viewer->getPHID()));
	- $query->setParameter(
	- 'auditStatus',
	- DiffusionCommitQuery::AUDIT_STATUS_CONCERN);
	+ case 'audited':
	+ $query
	+ ->setParameter('auditorPHIDs', array($viewer_phid));
	return $query;
	}

	return parent::buildSavedQueryFromBuiltin($query_key);
	}

	- private function getAuditStatusOptions() {
	- return array(
	- DiffusionCommitQuery::AUDIT_STATUS_ANY => pht('Any'),
	- DiffusionCommitQuery::AUDIT_STATUS_OPEN => pht('Open'),
	- DiffusionCommitQuery::AUDIT_STATUS_CONCERN => pht('Concern Raised'),
	- DiffusionCommitQuery::AUDIT_STATUS_ACCEPTED => pht('Accepted'),
	- DiffusionCommitQuery::AUDIT_STATUS_PARTIAL => pht('Partially Audited'),
	- );
	- }
	-
	protected function renderResultList(
	array $commits,
	PhabricatorSavedQuery $query,
	array $handles) {
	-
	assert_instances_of($commits, 'PhabricatorRepositoryCommit');
	-
	$viewer = $this->requireViewer();
	- $nodata = pht('No matching audits.');
	- $view = id(new PhabricatorAuditListView())
	- ->setUser($viewer)
	- ->setCommits($commits)
	- ->setAuthorityPHIDs(
	- PhabricatorAuditCommentEditor::loadAuditPHIDsForUser($viewer))
	- ->setNoDataString($nodata);
	-
	- $phids = $view->getRequiredHandlePHIDs();
	- if ($phids) {
	- $handles = id(new PhabricatorHandleQuery())
	- ->setViewer($viewer)
	- ->withPHIDs($phids)
	- ->execute();
	+
	+ $bucket = $this->getResultBucket($query);
	+
	+ $authority_phids = PhabricatorAuditCommentEditor::loadAuditPHIDsForUser(
	+ $viewer);
	+
	+ $template = id(new PhabricatorAuditListView())
	+ ->setViewer($viewer)
	+ ->setAuthorityPHIDs($authority_phids);
	+
	+ $views = array();
	+ if ($bucket) {
	+ $bucket->setViewer($viewer);
	+
	+ try {
	+ $groups = $bucket->newResultGroups($query, $commits);
	+
	+ foreach ($groups as $group) {
	+ $views[] = id(clone $template)
	+ ->setHeader($group->getName())
	+ ->setNoDataString($group->getNoDataString())
	+ ->setCommits($group->getObjects());
	+ }
	+ } catch (Exception $ex) {
	+ $this->addError($ex->getMessage());
	+ }
	} else {
	- $handles = array();
	+ $views[] = id(clone $template)
	+ ->setCommits($commits)
	+ ->setNoDataString(pht('No matching commits.'));
	}

	- $view->setHandles($handles);
	- $list = $view->buildList();
	+ if (count($views) == 1) {
	+ $list = head($views)->buildList();
	+ } else {
	+ $list = $views;
	+ }

	$result = new PhabricatorApplicationSearchResultView();
	$result->setContent($list);
	diff --git a/src/applications/audit/view/PhabricatorAuditListView.php b/src/applications/audit/view/PhabricatorAuditListView.php
	--- a/src/applications/audit/view/PhabricatorAuditListView.php
	+++ b/src/applications/audit/view/PhabricatorAuditListView.php
	@@ -3,18 +3,11 @@
	final class PhabricatorAuditListView extends AphrontView {

	private $commits;
	- private $handles;
	private $authorityPHIDs = array();
	+ private $header;
	private $noDataString;
	-
	private $highlightedAudits;

	- public function setHandles(array $handles) {
	- assert_instances_of($handles, 'PhabricatorObjectHandle');
	- $this->handles = $handles;
	- return $this;
	- }
	-
	public function setAuthorityPHIDs(array $phids) {
	$this->authorityPHIDs = $phids;
	return $this;
	@@ -29,6 +22,15 @@
	return $this->noDataString;
	}

	+ public function setHeader($header) {
	+ $this->header = $header;
	+ return $this;
	+ }
	+
	+ public function getHeader() {
	+ return $this->header;
	+ }
	+
	/**
	* These commits should have both commit data and audit requests attached.
	*/
	@@ -42,28 +44,6 @@
	return $this->commits;
	}

	- public function getRequiredHandlePHIDs() {
	- $phids = array();
	- $commits = $this->getCommits();
	- foreach ($commits as $commit) {
	- $phids[$commit->getPHID()] = true;
	- $phids[$commit->getAuthorPHID()] = true;
	- $audits = $commit->getAudits();
	- foreach ($audits as $audit) {
	- $phids[$audit->getAuditorPHID()] = true;
	- }
	- }
	- return array_keys($phids);
	- }
	-
	- private function getHandle($phid) {
	- $handle = idx($this->handles, $phid);
	- if (!$handle) {
	- throw new Exception(pht("No handle for '%s'!", $phid));
	- }
	- return $handle;
	- }
	-
	private function getCommitDescription($phid) {
	if ($this->commits === null) {
	return pht('(Unknown Commit)');
	@@ -96,34 +76,28 @@
	}

	public function buildList() {
	- $user = $this->getUser();
	- if (!$user) {
	- throw new Exception(
	- pht(
	- 'You must %s before %s!',
	- 'setUser()',
	- __FUNCTION__.'()'));
	- }
	+ $viewer = $this->getViewer();
	$rowc = array();

	+ $handles = $viewer->loadHandles(mpull($this->commits, 'getPHID'));
	+
	$list = new PHUIObjectItemListView();
	foreach ($this->commits as $commit) {
	$commit_phid = $commit->getPHID();
	- $commit_handle = $this->getHandle($commit_phid);
	+ $commit_handle = $handles[$commit_phid];
	$committed = null;

	$commit_name = $commit_handle->getName();
	$commit_link = $commit_handle->getURI();
	$commit_desc = $this->getCommitDescription($commit_phid);
	- $committed = phabricator_datetime($commit->getEpoch(), $user);
	+ $committed = phabricator_datetime($commit->getEpoch(), $viewer);

	$audits = mpull($commit->getAudits(), null, 'getAuditorPHID');
	$auditors = array();
	$reasons = array();
	foreach ($audits as $audit) {
	$auditor_phid = $audit->getAuditorPHID();
	- $auditors[$auditor_phid] =
	- $this->getHandle($auditor_phid)->renderLink();
	+ $auditors[$auditor_phid] = $viewer->renderHandle($auditor_phid);
	}
	$auditors = phutil_implode_html(', ', $auditors);

	@@ -151,7 +125,7 @@
	}
	$author_phid = $commit->getAuthorPHID();
	if ($author_phid) {
	- $author_name = $this->getHandle($author_phid)->renderLink();
	+ $author_name = $viewer->renderHandle($author_phid);
	} else {
	$author_name = $commit->getCommitData()->getAuthorName();
	}
	@@ -180,6 +154,10 @@
	$list->setNoDataString($this->noDataString);
	}

	+ if ($this->header) {
	+ $list->setHeader($this->header);
	+ }
	+
	return $list;
	}

	diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php
	--- a/src/applications/diffusion/query/DiffusionCommitQuery.php
	+++ b/src/applications/diffusion/query/DiffusionCommitQuery.php
	@@ -11,22 +11,16 @@
	private $repositoryIDs;
	private $repositoryPHIDs;
	private $identifierMap;
	+ private $responsiblePHIDs;
	+ private $statuses;

	private $needAuditRequests;
	private $auditIDs;
	private $auditorPHIDs;
	- private $needsAuditByPHIDs;
	- private $auditStatus;
	private $epochMin;
	private $epochMax;
	private $importing;

	- const AUDIT_STATUS_ANY = 'audit-status-any';
	- const AUDIT_STATUS_OPEN = 'audit-status-open';
	- const AUDIT_STATUS_CONCERN = 'audit-status-concern';
	- const AUDIT_STATUS_ACCEPTED = 'audit-status-accepted';
	- const AUDIT_STATUS_PARTIAL = 'audit-status-partial';
	-
	private $needCommitData;

	public function withIDs(array $ids) {
	@@ -119,16 +113,24 @@
	return $this;
	}

	- public function withNeedsAuditByPHIDs(array $needs_phids) {
	- $this->needsAuditByPHIDs = $needs_phids;
	+ public function withResponsiblePHIDs(array $responsible_phids) {
	+ $this->responsiblePHIDs = $responsible_phids;
	return $this;
	}

	- public function withAuditStatus($status) {
	- $this->auditStatus = $status;
	+ public function withStatuses(array $statuses) {
	+ $this->statuses = $statuses;
	return $this;
	}

	+ public function withAuditStatus($status) {
	+ // TODO: Replace callers with `withStatuses()`.
	+ return $this->withStatuses(
	+ array(
	+ $status,
	+ ));
	+ }
	+
	public function withEpochRange($min, $max) {
	$this->epochMin = $min;
	$this->epochMax = $max;
	@@ -251,10 +253,7 @@
	}
	}

	- // TODO: This should just be `needAuditRequests`, not `shouldJoinAudits()`,
	- // but leave that for a future diff.
	-
	- if ($this->needAuditRequests \|\| $this->shouldJoinAudits()) {
	+ if ($this->needAuditRequests) {
	$requests = id(new PhabricatorRepositoryAuditRequest())->loadAllWhere(
	'commitPHID IN (%Ls)',
	mpull($commits, 'getPHID'));
	@@ -459,67 +458,30 @@
	if ($this->auditIDs !== null) {
	$where[] = qsprintf(
	$conn,
	- 'audit.id IN (%Ld)',
	+ 'auditor.id IN (%Ld)',
	$this->auditIDs);
	}

	if ($this->auditorPHIDs !== null) {
	$where[] = qsprintf(
	$conn,
	- 'audit.auditorPHID IN (%Ls)',
	+ 'auditor.auditorPHID IN (%Ls)',
	$this->auditorPHIDs);
	}

	- if ($this->needsAuditByPHIDs !== null) {
	+ if ($this->responsiblePHIDs !== null) {
	$where[] = qsprintf(
	$conn,
	- 'needs.auditorPHID IN (%Ls)',
	- $this->needsAuditByPHIDs);
	+ '(audit.auditorPHID IN (%Ls) OR commit.authorPHID IN (%Ls))',
	+ $this->responsiblePHIDs,
	+ $this->responsiblePHIDs);
	}

	- $status = $this->auditStatus;
	- if ($status !== null) {
	- switch ($status) {
	- case self::AUDIT_STATUS_PARTIAL:
	- $where[] = qsprintf(
	- $conn,
	- 'commit.auditStatus = %d',
	- PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED);
	- break;
	- case self::AUDIT_STATUS_ACCEPTED:
	- $where[] = qsprintf(
	- $conn,
	- 'commit.auditStatus = %d',
	- PhabricatorAuditCommitStatusConstants::FULLY_AUDITED);
	- break;
	- case self::AUDIT_STATUS_CONCERN:
	- $where[] = qsprintf(
	- $conn,
	- 'status.auditStatus = %s',
	- PhabricatorAuditStatusConstants::CONCERNED);
	- break;
	- case self::AUDIT_STATUS_OPEN:
	- $where[] = qsprintf(
	- $conn,
	- 'status.auditStatus in (%Ls)',
	- PhabricatorAuditStatusConstants::getOpenStatusConstants());
	- break;
	- case self::AUDIT_STATUS_ANY:
	- break;
	- default:
	- $valid = array(
	- self::AUDIT_STATUS_ANY,
	- self::AUDIT_STATUS_OPEN,
	- self::AUDIT_STATUS_CONCERN,
	- self::AUDIT_STATUS_ACCEPTED,
	- self::AUDIT_STATUS_PARTIAL,
	- );
	- throw new Exception(
	- pht(
	- "Unknown audit status '%s'! Valid statuses are: %s.",
	- $status,
	- implode(', ', $valid)));
	- }
	+ if ($this->statuses !== null) {
	+ $where[] = qsprintf(
	+ $conn,
	+ 'commit.auditStatus IN (%Ls)',
	+ $this->statuses);
	}

	return $where;
	@@ -535,61 +497,41 @@
	}
	}

	- private function shouldJoinStatus() {
	- return $this->auditStatus;
	+ private function shouldJoinAuditor() {
	+ return ($this->auditIDs \|\| $this->auditorPHIDs);
	}

	- private function shouldJoinAudits() {
	- return $this->auditIDs \|\| $this->auditorPHIDs;
	- }
	-
	- private function shouldJoinNeeds() {
	- return $this->needsAuditByPHIDs;
	+ private function shouldJoinAudit() {
	+ return (bool)$this->responsiblePHIDs;
	}

	protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
	$join = parent::buildJoinClauseParts($conn);
	$audit_request = new PhabricatorRepositoryAuditRequest();

	- if ($this->shouldJoinStatus()) {
	+ if ($this->shouldJoinAuditor()) {
	$join[] = qsprintf(
	$conn,
	- 'LEFT JOIN %T status ON commit.phid = status.commitPHID',
	+ 'JOIN %T auditor ON commit.phid = auditor.commitPHID',
	$audit_request->getTableName());
	}

	- if ($this->shouldJoinAudits()) {
	+ if ($this->shouldJoinAudit()) {
	$join[] = qsprintf(
	$conn,
	- 'JOIN %T audit ON commit.phid = audit.commitPHID',
	+ 'LEFT JOIN %T audit ON commit.phid = audit.commitPHID',
	$audit_request->getTableName());
	}

	- if ($this->shouldJoinNeeds()) {
	- $join[] = qsprintf(
	- $conn,
	- 'JOIN %T needs ON commit.phid = needs.commitPHID
	- AND needs.auditStatus IN (%Ls)',
	- $audit_request->getTableName(),
	- array(
	- PhabricatorAuditStatusConstants::AUDIT_REQUESTED,
	- PhabricatorAuditStatusConstants::AUDIT_REQUIRED,
	- ));
	- }
	-
	return $join;
	}

	protected function shouldGroupQueryResultRows() {
	- if ($this->shouldJoinStatus()) {
	- return true;
	- }
	-
	- if ($this->shouldJoinAudits()) {
	+ if ($this->shouldJoinAuditor()) {
	return true;
	}

	- if ($this->shouldJoinNeeds()) {
	+ if ($this->shouldJoinAudit()) {
	return true;
	}

	diff --git a/src/applications/diffusion/query/DiffusionCommitRequiredActionResultBucket.php b/src/applications/diffusion/query/DiffusionCommitRequiredActionResultBucket.php
	new file mode 100644
	--- /dev/null
	+++ b/src/applications/diffusion/query/DiffusionCommitRequiredActionResultBucket.php
	@@ -0,0 +1,151 @@
	+<?php
	+
	+final class DiffusionCommitRequiredActionResultBucket
	+ extends DiffusionCommitResultBucket {
	+
	+ const BUCKETKEY = 'action';
	+
	+ private $objects;
	+
	+ public function getResultBucketName() {
	+ return pht('Bucket by Required Action');
	+ }
	+
	+ protected function buildResultGroups(
	+ PhabricatorSavedQuery $query,
	+ array $objects) {
	+
	+ $this->objects = $objects;
	+
	+ $phids = $query->getEvaluatedParameter('responsiblePHIDs', array());
	+ if (!$phids) {
	+ throw new Exception(
	+ pht(
	+ 'You can not bucket results by required action without '.
	+ 'specifying "Responsible Users".'));
	+ }
	+ $phids = array_fuse($phids);
	+
	+ $groups = array();
	+
	+ $groups[] = $this->newGroup()
	+ ->setName(pht('Needs Attention'))
	+ ->setNoDataString(pht('None of your commits have active concerns.'))
	+ ->setObjects($this->filterConcernRaised($phids));
	+
	+ $groups[] = $this->newGroup()
	+ ->setName(pht('Ready to Audit'))
	+ ->setNoDataString(pht('No commits are waiting for you to audit them.'))
	+ ->setObjects($this->filterShouldAudit($phids));
	+
	+ $groups[] = $this->newGroup()
	+ ->setName(pht('Waiting on Authors'))
	+ ->setNoDataString(pht('None of your audits are waiting on authors.'))
	+ ->setObjects($this->filterWaitingOnAuthors($phids));
	+
	+ $groups[] = $this->newGroup()
	+ ->setName(pht('Waiting on Auditors'))
	+ ->setNoDataString(pht('None of your commits are waiting on audit.'))
	+ ->setObjects($this->filterWaitingOnAuditors($phids));
	+
	+ // Because you can apply these buckets to queries which include revisions
	+ // that have been closed, add an "Other" bucket if we still have stuff
	+ // that didn't get filtered into any of the previous buckets.
	+ if ($this->objects) {
	+ $groups[] = $this->newGroup()
	+ ->setName(pht('Other Commits'))
	+ ->setObjects($this->objects);
	+ }
	+
	+ return $groups;
	+ }
	+
	+ private function filterConcernRaised(array $phids) {
	+ $results = array();
	+ $objects = $this->objects;
	+
	+ $status_concern = PhabricatorAuditCommitStatusConstants::CONCERN_RAISED;
	+
	+ foreach ($objects as $key => $object) {
	+ if (empty($phids[$object->getAuthorPHID()])) {
	+ continue;
	+ }
	+
	+ if ($object->getAuditStatus() != $status_concern) {
	+ continue;
	+ }
	+
	+ $results[$key] = $object;
	+ unset($this->objects[$key]);
	+ }
	+
	+ return $results;
	+ }
	+
	+ private function filterShouldAudit(array $phids) {
	+ $results = array();
	+ $objects = $this->objects;
	+
	+ $should_audit = array(
	+ PhabricatorAuditStatusConstants::AUDIT_REQUIRED,
	+ PhabricatorAuditStatusConstants::AUDIT_REQUESTED,
	+ );
	+ $should_audit = array_fuse($should_audit);
	+
	+ foreach ($objects as $key => $object) {
	+ if (!$this->hasAuditorsWithStatus($object, $phids, $should_audit)) {
	+ continue;
	+ }
	+
	+ $results[$key] = $object;
	+ unset($this->objects[$key]);
	+ }
	+
	+ return $results;
	+ }
	+
	+ private function filterWaitingOnAuthors(array $phids) {
	+ $results = array();
	+ $objects = $this->objects;
	+
	+ $status_concern = PhabricatorAuditCommitStatusConstants::CONCERN_RAISED;
	+
	+ foreach ($objects as $key => $object) {
	+ if ($object->getAuditStatus() != $status_concern) {
	+ continue;
	+ }
	+
	+ if (isset($phids[$object->getAuthorPHID()])) {
	+ continue;
	+ }
	+
	+ $results[$key] = $object;
	+ unset($this->objects[$key]);
	+ }
	+
	+ return $results;
	+ }
	+
	+ private function filterWaitingOnAuditors(array $phids) {
	+ $results = array();
	+ $objects = $this->objects;
	+
	+ $status_waiting = array(
	+ PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT,
	+ PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED,
	+ );
	+ $status_waiting = array_fuse($status_waiting);
	+
	+ foreach ($objects as $key => $object) {
	+ if (empty($status_waiting[$object->getAuditStatus()])) {
	+ continue;
	+ }
	+
	+ $results[$key] = $object;
	+ unset($this->objects[$key]);
	+ }
	+
	+ return $results;
	+ }
	+
	+}
	diff --git a/src/applications/diffusion/query/DiffusionCommitResultBucket.php b/src/applications/diffusion/query/DiffusionCommitResultBucket.php
	new file mode 100644
	--- /dev/null
	+++ b/src/applications/diffusion/query/DiffusionCommitResultBucket.php
	@@ -0,0 +1,33 @@
	+<?php
	+
	+abstract class DiffusionCommitResultBucket
	+ extends PhabricatorSearchResultBucket {
	+
	+ public static function getAllResultBuckets() {
	+ return id(new PhutilClassMapQuery())
	+ ->setAncestorClass(__CLASS__)
	+ ->setUniqueMethod('getResultBucketKey')
	+ ->execute();
	+ }
	+
	+ protected function hasAuditorsWithStatus(
	+ PhabricatorRepositoryCommit $commit,
	+ array $phids,
	+ array $statuses) {
	+
	+ foreach ($commit->getAudits() as $audit) {
	+ if (!isset($phids[$audit->getAuditorPHID()])) {
	+ continue;
	+ }
	+
	+ if (!isset($statuses[$audit->getAuditStatus()])) {
	+ continue;
	+ }
	+
	+ return true;
	+ }
	+
	+ return false;
	+ }
	+
	+}
	diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
	--- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php
	+++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
	@@ -71,13 +71,17 @@
	'auditorPHIDs' => $package->getPHID(),
	));

	- $status_concern = DiffusionCommitQuery::AUDIT_STATUS_CONCERN;
	+ $status_concern = PhabricatorAuditCommitStatusConstants::CONCERN_RAISED;

	$attention_commits = id(new DiffusionCommitQuery())
	->setViewer($request->getUser())
	->withAuditorPHIDs(array($package->getPHID()))
	- ->withAuditStatus($status_concern)
	+ ->withStatuses(
	+ array(
	+ $status_concern,
	+ ))
	->needCommitData(true)
	+ ->needAuditRequests(true)
	->setLimit(10)
	->execute();
	$view = id(new PhabricatorAuditListView())
	@@ -100,7 +104,8 @@
	->setViewer($request->getUser())
	->withAuditorPHIDs(array($package->getPHID()))
	->needCommitData(true)
	- ->setLimit(100)
	+ ->needAuditRequests(true)
	+ ->setLimit(25)
	->execute();

	$view = id(new PhabricatorAuditListView())
	@@ -119,13 +124,6 @@
	->setText(pht('View All')),
	);

	- $phids = array();
	- foreach ($commit_views as $commit_view) {
	- $phids[] = $commit_view['view']->getRequiredHandlePHIDs();
	- }
	- $phids = array_mergev($phids);
	- $handles = $this->loadViewerHandles($phids);
	-
	$commit_panels = array();
	foreach ($commit_views as $commit_view) {
	$commit_panel = id(new PHUIObjectBoxView())
	@@ -136,7 +134,6 @@
	if (isset($commit_view['button'])) {
	$commit_header->addActionLink($commit_view['button']);
	}
	- $commit_view['view']->setHandles($handles);
	$commit_panel->setHeader($commit_header);
	$commit_panel->appendChild($commit_view['view']);

	diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php
	--- a/src/applications/policy/filter/PhabricatorPolicyFilter.php
	+++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php
	@@ -917,15 +917,27 @@
	}

	private function getApplicationForPHID($phid) {
	+ static $class_map = array();
	+
	$phid_type = phid_get_type($phid);
	+ if (!isset($class_map[$phid_type])) {
	+ $type_objects = PhabricatorPHIDType::getTypes(array($phid_type));
	+ $type_object = idx($type_objects, $phid_type);
	+ if (!$type_object) {
	+ $class = false;
	+ } else {
	+ $class = $type_object->getPHIDTypeApplicationClass();
	+ }
	+
	+ $class_map[$phid_type] = $class;
	+ }

	- $type_objects = PhabricatorPHIDType::getTypes(array($phid_type));
	- $type_object = idx($type_objects, $phid_type);
	- if (!$type_object) {
	+ $class = $class_map[$phid_type];
	+ if ($class === false) {
	return null;
	}

	- return $type_object->getPHIDTypeApplicationClass();
	+ return $class;
	}

	}
	diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php
	--- a/src/applications/search/controller/PhabricatorApplicationSearchController.php
	+++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php
	@@ -239,6 +239,10 @@
	$nux_view = null;
	}

	+ $is_overflowing =
	+ $pager->willShowPagingControls() &&
	+ $engine->getResultBucket($saved_query);
	+
	$force_overheated = $request->getBool('overheated');
	$is_overheated = $query->getIsOverheated() \|\| $force_overheated;

	@@ -265,6 +269,11 @@
	if ($list->getInfoView()) {
	$box->setInfoView($list->getInfoView());
	}
	+
	+ if ($is_overflowing) {
	+ $box->appendChild($this->newOverflowingView());
	+ }
	+
	if ($list->getContent()) {
	$box->appendChild($list->getContent());
	}
	@@ -545,6 +554,22 @@
	->setDropdownMenu($action_list);
	}

	+ private function newOverflowingView() {
	+ $message = pht(
	+ 'The query matched more than one page of results. Results are '.
	+ 'paginated before bucketing, so later pages may contain additional '.
	+ 'results in any bucket.');
	+
	+ return id(new PHUIInfoView())
	+ ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
	+ ->setFlush(true)
	+ ->setTitle(pht('Buckets Overflowing'))
	+ ->setErrors(
	+ array(
	+ $message,
	+ ));
	+ }
	+
	private function newOverheatedView(array $results) {
	if ($results) {
	$message = pht(
	diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php
	--- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php
	+++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php
	@@ -908,7 +908,7 @@
	return array();
	}

	- protected function getResultBucket(PhabricatorSavedQuery $saved) {
	+ public function getResultBucket(PhabricatorSavedQuery $saved) {
	$key = $saved->getParameter('bucket');
	if ($key == self::BUCKET_NONE) {
	return null;

File Metadata

Mime Type: text/plain
Expires: Mon, Oct 5, 7:09 AM (2 h, 44 m ago)
Storage Engine: amazon-s3
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: phabricator/secure/a7/ir/q4ovx7qmypajhmjs
Default Alt Text: D17192.diff (36 KB)

D17192.diffNo OneTemporaryActions

D17192.diffView Options

File Metadata

Event Timeline

D17192.diff
No OneTemporary
Actions

D17192.diff
View Options