1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-20 13:52:40 +01:00

Make JOIN changes to CommitQuery only

Summary:
Fixes T4911. See D8879. This gives us the correct query in cases where there are no audits.

This doesn't try to do the GROUP BY stuff yet.

Test Plan:
  - Viewed a commit in Diffusion with no audits, got a commit detail page.
  - Viewed "All Commits" in web UI, saw commits without any audits included in the list.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4911

Differential Revision: https://secure.phabricator.com/D8882
This commit is contained in:
epriestley 2014-04-28 08:25:51 -07:00
parent 0db6aad80d
commit 8716e734f0

View file

@ -88,13 +88,28 @@ final class DiffusionCommitQuery
return $this;
}
public function getAuditRequests() {
/**
* Retuns true if we should join the audit table, either because we're
* interested in the information if it's available or because matching
* rows must always have it.
*/
private function shouldJoinAudits() {
return
$this->needAuditRequests ||
$this->auditStatus ||
$this->rowsMustHaveAudits();
}
/**
* Return true if we should `JOIN` (vs `LEFT JOIN`) the audit table, because
* matching commits will always have audit rows.
*/
private function rowsMustHaveAudits() {
return
$this->auditIDs ||
$this->auditorPHIDs ||
$this->auditAwaitingUser ||
$this->auditStatus;
$this->auditAwaitingUser;
}
public function withAuditIDs(array $ids) {
@ -149,7 +164,7 @@ final class DiffusionCommitQuery
$this->buildOrderClause($conn_r),
$this->buildLimitClause($conn_r));
if ($this->getAuditRequests()) {
if ($this->shouldJoinAudits()) {
$this->loadAuditIds = ipull($data, 'audit_id');
}
@ -157,7 +172,7 @@ final class DiffusionCommitQuery
}
private function buildAuditSelect($conn_r) {
if ($this->getAuditRequests()) {
if ($this->shouldJoinAudits()) {
return qsprintf(
$conn_r,
', audit.id as audit_id');
@ -239,9 +254,14 @@ final class DiffusionCommitQuery
}
}
if ($this->getAuditRequests()) {
$requests = id(new PhabricatorRepositoryAuditRequest())
->loadAllWhere('id IN (%Ld)', $this->loadAuditIds);
if ($this->shouldJoinAudits()) {
$load_ids = array_filter($this->loadAuditIds);
if ($load_ids) {
$requests = id(new PhabricatorRepositoryAuditRequest())
->loadAllWhere('id IN (%Ld)', $this->loadAuditIds);
} else {
$requests = array();
}
$requests = mgroup($requests, 'getCommitPHID');
foreach ($commits as $commit) {
@ -259,35 +279,35 @@ final class DiffusionCommitQuery
private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
$where = array();
if ($this->ids) {
if ($this->ids !== null) {
$where[] = qsprintf(
$conn_r,
'commit.id IN (%Ld)',
$this->ids);
}
if ($this->phids) {
if ($this->phids !== null) {
$where[] = qsprintf(
$conn_r,
'commit.phid IN (%Ls)',
$this->phids);
}
if ($this->repositoryIDs) {
if ($this->repositoryIDs !== null) {
$where[] = qsprintf(
$conn_r,
'commit.repositoryID IN (%Ld)',
$this->repositoryIDs);
}
if ($this->authorPHIDs) {
if ($this->authorPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'commit.authorPHID IN (%Ls)',
$this->authorPHIDs);
}
if ($this->identifiers) {
if ($this->identifiers !== null) {
$min_unqualified = PhabricatorRepository::MINIMUM_UNQUALIFIED_HASH;
$min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH;
@ -378,14 +398,14 @@ final class DiffusionCommitQuery
$where[] = '('.implode(' OR ', $sql).')';
}
if ($this->auditIDs) {
if ($this->auditIDs !== null) {
$where[] = qsprintf(
$conn_r,
'audit.id IN (%Ld)',
$this->auditIDs);
}
if ($this->auditorPHIDs) {
if ($this->auditorPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
'audit.auditorPHID IN (%Ls)',
@ -458,10 +478,11 @@ final class DiffusionCommitQuery
$joins = array();
$audit_request = new PhabricatorRepositoryAuditRequest();
if ($this->getAuditRequests()) {
if ($this->shouldJoinAudits()) {
$joins[] = qsprintf(
$conn_r,
'JOIN %T audit ON commit.phid = audit.commitPHID',
'%Q %T audit ON commit.phid = audit.commitPHID',
($this->rowsMustHaveAudits() ? 'JOIN' : 'LEFT JOIN'),
$audit_request->getTableName());
}