2011-10-02 21:03:16 +02:00
|
|
|
<?php
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Flexible query API for Differential revisions. Example:
|
|
|
|
*
|
|
|
|
* // Load open revisions
|
|
|
|
* $revisions = id(new DifferentialRevisionQuery())
|
|
|
|
* ->withStatus(DifferentialRevisionQuery::STATUS_OPEN)
|
|
|
|
* ->execute();
|
|
|
|
*
|
|
|
|
* @task config Query Configuration
|
|
|
|
* @task exec Query Execution
|
|
|
|
* @task internal Internals
|
|
|
|
*/
|
|
|
|
final class DifferentialRevisionQuery {
|
|
|
|
|
|
|
|
// TODO: Replace DifferentialRevisionListData with this class.
|
|
|
|
|
|
|
|
private $pathIDs = array();
|
|
|
|
|
2012-06-29 00:26:00 +02:00
|
|
|
private $status = 'status-any';
|
|
|
|
const STATUS_ANY = 'status-any';
|
|
|
|
const STATUS_OPEN = 'status-open';
|
|
|
|
const STATUS_ACCEPTED = 'status-accepted';
|
|
|
|
const STATUS_NEEDS_REVIEW = 'status-needs-review';
|
|
|
|
const STATUS_CLOSED = 'status-closed'; // NOTE: Same as 'committed'.
|
|
|
|
const STATUS_COMMITTED = 'status-committed'; // TODO: Remove.
|
|
|
|
const STATUS_ABANDONED = 'status-abandoned';
|
2011-10-02 21:03:16 +02:00
|
|
|
|
2011-12-06 23:21:36 +01:00
|
|
|
private $authors = array();
|
2012-03-20 02:35:32 +01:00
|
|
|
private $draftAuthors = array();
|
2011-12-02 01:57:06 +01:00
|
|
|
private $ccs = array();
|
|
|
|
private $reviewers = array();
|
|
|
|
private $revIDs = array();
|
2012-01-04 06:08:12 +01:00
|
|
|
private $commitHashes = array();
|
2011-12-02 01:57:06 +01:00
|
|
|
private $phids = array();
|
2011-12-07 17:30:49 +01:00
|
|
|
private $subscribers = array();
|
|
|
|
private $responsibles = array();
|
2012-01-24 17:31:45 +01:00
|
|
|
private $branches = array();
|
2012-04-10 21:51:34 +02:00
|
|
|
private $arcanistProjectPHIDs = array();
|
2012-10-04 06:16:57 +02:00
|
|
|
private $draftRevisions = array();
|
2011-12-02 01:57:06 +01:00
|
|
|
|
2011-10-02 21:03:16 +02:00
|
|
|
private $order = 'order-modified';
|
|
|
|
const ORDER_MODIFIED = 'order-modified';
|
|
|
|
const ORDER_CREATED = 'order-created';
|
|
|
|
/**
|
|
|
|
* This is essentially a denormalized copy of the revision modified time that
|
|
|
|
* should perform better for path queries with a LIMIT. Critically, when you
|
|
|
|
* browse "/", every revision in that repository for all time will match so
|
|
|
|
* the query benefits from being able to stop before fully materializing the
|
|
|
|
* result set.
|
|
|
|
*/
|
|
|
|
const ORDER_PATH_MODIFIED = 'order-path-modified';
|
|
|
|
|
|
|
|
private $limit = 1000;
|
|
|
|
private $offset = 0;
|
|
|
|
|
2011-12-21 02:05:52 +01:00
|
|
|
private $needRelationships = false;
|
|
|
|
private $needActiveDiffs = false;
|
|
|
|
private $needDiffIDs = false;
|
|
|
|
private $needCommitPHIDs = false;
|
2012-06-26 18:07:52 +02:00
|
|
|
private $needHashes = false;
|
2011-10-02 21:03:16 +02:00
|
|
|
|
|
|
|
|
|
|
|
/* -( Query Configuration )------------------------------------------------ */
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Filter results to revisions which affect a Diffusion path ID in a given
|
|
|
|
* repository. You can call this multiple times to select revisions for
|
|
|
|
* several paths.
|
|
|
|
*
|
|
|
|
* @param int Diffusion repository ID.
|
|
|
|
* @param int Diffusion path ID.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withPath($repository_id, $path_id) {
|
|
|
|
$this->pathIDs[] = array(
|
|
|
|
'repositoryID' => $repository_id,
|
|
|
|
'pathID' => $path_id,
|
|
|
|
);
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
2011-12-02 01:57:06 +01:00
|
|
|
/**
|
2012-01-04 06:08:12 +01:00
|
|
|
* Filter results to revisions authored by one of the given PHIDs. Calling
|
|
|
|
* this function will clear anything set by previous calls to
|
|
|
|
* @{method:withAuthors}.
|
2011-12-06 23:21:36 +01:00
|
|
|
*
|
|
|
|
* @param array List of PHIDs of authors
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
2011-12-07 17:30:49 +01:00
|
|
|
public function withAuthors(array $author_phids) {
|
|
|
|
$this->authors = $author_phids;
|
2011-12-06 23:21:36 +01:00
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
2012-03-20 02:35:32 +01:00
|
|
|
/**
|
|
|
|
* Filter results to revisions with comments authored bythe given PHIDs
|
|
|
|
*
|
|
|
|
* @param array List of PHIDs of authors
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withDraftRepliesByAuthors(array $author_phids) {
|
|
|
|
$this->draftAuthors = $author_phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
2011-12-06 23:21:36 +01:00
|
|
|
/**
|
|
|
|
* Filter results to revisions which CC one of the listed people. Calling this
|
2011-12-07 17:30:49 +01:00
|
|
|
* function will clear anything set by previous calls to @{method:withCCs}.
|
2011-12-02 01:57:06 +01:00
|
|
|
*
|
|
|
|
* @param array List of PHIDs of subscribers
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withCCs(array $cc_phids) {
|
|
|
|
$this->ccs = $cc_phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
2011-12-06 23:21:36 +01:00
|
|
|
* Filter results to revisions that have one of the provided PHIDs as
|
2011-12-02 01:57:06 +01:00
|
|
|
* reviewers. Calling this function will clear anything set by previous calls
|
2011-12-07 17:30:49 +01:00
|
|
|
* to @{method:withReviewers}.
|
2011-12-02 01:57:06 +01:00
|
|
|
*
|
|
|
|
* @param array List of PHIDs of reviewers
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withReviewers(array $reviewer_phids) {
|
|
|
|
$this->reviewers = $reviewer_phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
2012-01-04 06:08:12 +01:00
|
|
|
/**
|
|
|
|
* Filter results to revisions that have one of the provided commit hashes.
|
|
|
|
* Calling this function will clear anything set by previous calls to
|
|
|
|
* @{method:withCommitHashes}.
|
|
|
|
*
|
2012-01-10 20:39:11 +01:00
|
|
|
* @param array List of pairs <Class
|
|
|
|
* ArcanistDifferentialRevisionHash::HASH_$type constant,
|
|
|
|
* hash>
|
2012-01-04 06:08:12 +01:00
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withCommitHashes(array $commit_hashes) {
|
|
|
|
$this->commitHashes = $commit_hashes;
|
|
|
|
return $this;
|
|
|
|
}
|
2011-10-02 21:03:16 +02:00
|
|
|
|
|
|
|
/**
|
|
|
|
* Filter results to revisions with a given status. Provide a class constant,
|
|
|
|
* such as ##DifferentialRevisionQuery::STATUS_OPEN##.
|
|
|
|
*
|
|
|
|
* @param const Class STATUS constant, like STATUS_OPEN.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withStatus($status_constant) {
|
|
|
|
$this->status = $status_constant;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2012-01-24 17:31:45 +01:00
|
|
|
/**
|
|
|
|
* Filter results to revisions on given branches.
|
|
|
|
*
|
|
|
|
* @param list List of branch names.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withBranches(array $branches) {
|
|
|
|
$this->branches = $branches;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2011-12-02 01:57:06 +01:00
|
|
|
/**
|
|
|
|
* Filter results to only return revisions whose ids are in the given set.
|
|
|
|
*
|
|
|
|
* @param array List of revision ids
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withIDs(array $ids) {
|
|
|
|
$this->revIDs = $ids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Filter results to only return revisions whose PHIDs are in the given set.
|
|
|
|
*
|
|
|
|
* @param array List of revision PHIDs
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withPHIDs(array $phids) {
|
|
|
|
$this->phids = $phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2011-12-07 17:30:49 +01:00
|
|
|
/**
|
|
|
|
* Given a set of users, filter results to return only revisions they are
|
|
|
|
* responsible for (i.e., they are either authors or reviewers).
|
|
|
|
*
|
|
|
|
* @param array List of user PHIDs.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withResponsibleUsers(array $responsible_phids) {
|
|
|
|
$this->responsibles = $responsible_phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Filter results to only return revisions with a given set of subscribers
|
|
|
|
* (i.e., they are authors, reviewers or CC'd).
|
|
|
|
*
|
|
|
|
* @param array List of user PHIDs.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withSubscribers(array $subscriber_phids) {
|
|
|
|
$this->subscribers = $subscriber_phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2012-04-10 21:51:34 +02:00
|
|
|
/**
|
|
|
|
* Filter results to only return revisions with a given set of arcanist
|
|
|
|
* projects.
|
|
|
|
*
|
|
|
|
* @param array List of project PHIDs.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withArcanistProjectPHIDs(array $arc_project_phids) {
|
|
|
|
$this->arcanistProjectPHIDs = $arc_project_phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2011-10-02 21:03:16 +02:00
|
|
|
/**
|
|
|
|
* Set result ordering. Provide a class constant, such as
|
|
|
|
* ##DifferentialRevisionQuery::ORDER_CREATED##.
|
|
|
|
*
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function setOrder($order_constant) {
|
|
|
|
$this->order = $order_constant;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Set result limit. If unspecified, defaults to 1000.
|
|
|
|
*
|
|
|
|
* @param int Result limit.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function setLimit($limit) {
|
|
|
|
$this->limit = $limit;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Set result offset. If unspecified, defaults to 0.
|
|
|
|
*
|
|
|
|
* @param int Result offset.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function setOffset($offset) {
|
|
|
|
$this->offset = $offset;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Set whether or not the query will load and attach relationships.
|
|
|
|
*
|
|
|
|
* @param bool True to load and attach relationships.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function needRelationships($need_relationships) {
|
|
|
|
$this->needRelationships = $need_relationships;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2011-12-21 02:05:52 +01:00
|
|
|
/**
|
|
|
|
* Set whether or not the query should load the active diff for each
|
|
|
|
* revision.
|
|
|
|
*
|
|
|
|
* @param bool True to load and attach diffs.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function needActiveDiffs($need_active_diffs) {
|
|
|
|
$this->needActiveDiffs = $need_active_diffs;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Set whether or not the query should load the associated commit PHIDs for
|
|
|
|
* each revision.
|
|
|
|
*
|
|
|
|
* @param bool True to load and attach diffs.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function needCommitPHIDs($need_commit_phids) {
|
|
|
|
$this->needCommitPHIDs = $need_commit_phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Set whether or not the query should load associated diff IDs for each
|
|
|
|
* revision.
|
|
|
|
*
|
|
|
|
* @param bool True to load and attach diff IDs.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function needDiffIDs($need_diff_ids) {
|
|
|
|
$this->needDiffIDs = $need_diff_ids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2012-06-26 18:07:52 +02:00
|
|
|
/**
|
|
|
|
* Set whether or not the query should load associated commit hashes for each
|
|
|
|
* revision.
|
|
|
|
*
|
|
|
|
* @param bool True to load and attach commit hashes.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function needHashes($need_hashes) {
|
|
|
|
$this->needHashes = $need_hashes;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2011-10-02 21:03:16 +02:00
|
|
|
/* -( Query Execution )---------------------------------------------------- */
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Execute the query as configured, returning matching
|
|
|
|
* @{class:DifferentialRevision} objects.
|
|
|
|
*
|
|
|
|
* @return list List of matching DifferentialRevision objects.
|
|
|
|
* @task exec
|
|
|
|
*/
|
|
|
|
public function execute() {
|
|
|
|
$table = new DifferentialRevision();
|
|
|
|
$conn_r = $table->establishConnection('r');
|
|
|
|
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-08 00:35:10 +01:00
|
|
|
if ($this->shouldUseResponsibleFastPath()) {
|
|
|
|
$data = $this->loadDataUsingResponsibleFastPath();
|
|
|
|
} else {
|
|
|
|
$data = $this->loadData();
|
|
|
|
}
|
2011-10-02 21:03:16 +02:00
|
|
|
|
|
|
|
$revisions = $table->loadAllFromArray($data);
|
|
|
|
|
2011-12-21 02:05:52 +01:00
|
|
|
if ($revisions) {
|
|
|
|
if ($this->needRelationships) {
|
|
|
|
$this->loadRelationships($conn_r, $revisions);
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->needCommitPHIDs) {
|
|
|
|
$this->loadCommitPHIDs($conn_r, $revisions);
|
|
|
|
}
|
|
|
|
|
2012-04-10 21:51:34 +02:00
|
|
|
$need_active = $this->needActiveDiffs;
|
2012-01-24 17:31:45 +01:00
|
|
|
$need_ids = $need_active ||
|
|
|
|
$this->needDiffIDs;
|
|
|
|
|
|
|
|
if ($need_ids) {
|
2011-12-21 02:05:52 +01:00
|
|
|
$this->loadDiffIDs($conn_r, $revisions);
|
|
|
|
}
|
|
|
|
|
2012-01-24 17:31:45 +01:00
|
|
|
if ($need_active) {
|
2011-12-21 02:05:52 +01:00
|
|
|
$this->loadActiveDiffs($conn_r, $revisions);
|
2011-10-02 21:03:16 +02:00
|
|
|
}
|
2012-06-26 18:07:52 +02:00
|
|
|
|
|
|
|
if ($this->needHashes) {
|
|
|
|
$this->loadHashes($conn_r, $revisions);
|
|
|
|
}
|
2011-10-02 21:03:16 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
return $revisions;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-08 00:35:10 +01:00
|
|
|
/**
|
|
|
|
* Determine if we should execute an optimized, fast-path query to fetch
|
|
|
|
* open revisions for one responsible user. This is used by the Differential
|
|
|
|
* dashboard and much faster when executed as a UNION ALL than with JOIN
|
|
|
|
* and WHERE, which is why we special case it.
|
|
|
|
*/
|
|
|
|
private function shouldUseResponsibleFastPath() {
|
|
|
|
if ((count($this->responsibles) == 1) &&
|
|
|
|
($this->status == self::STATUS_OPEN) &&
|
|
|
|
($this->order == self::ORDER_MODIFIED) &&
|
|
|
|
!$this->offset &&
|
|
|
|
!$this->limit &&
|
|
|
|
!$this->subscribers &&
|
|
|
|
!$this->reviewers &&
|
|
|
|
!$this->ccs &&
|
|
|
|
!$this->authors &&
|
|
|
|
!$this->revIDs &&
|
2012-01-04 06:08:12 +01:00
|
|
|
!$this->commitHashes &&
|
2012-04-10 21:51:34 +02:00
|
|
|
!$this->phids &&
|
|
|
|
!$this->branches &&
|
|
|
|
!$this->arcanistProjectPHIDs) {
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-08 00:35:10 +01:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
private function loadDataUsingResponsibleFastPath() {
|
|
|
|
$table = new DifferentialRevision();
|
|
|
|
$conn_r = $table->establishConnection('r');
|
|
|
|
|
|
|
|
$responsible_phid = reset($this->responsibles);
|
|
|
|
$open_statuses = array(
|
2012-01-10 20:39:11 +01:00
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVIEW,
|
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVISION,
|
|
|
|
ArcanistDifferentialRevisionStatus::ACCEPTED,
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-08 00:35:10 +01:00
|
|
|
);
|
|
|
|
|
|
|
|
return queryfx_all(
|
|
|
|
$conn_r,
|
|
|
|
'SELECT * FROM %T WHERE authorPHID = %s AND status IN (%Ld)
|
|
|
|
UNION ALL
|
|
|
|
SELECT r.* FROM %T r JOIN %T rel
|
|
|
|
ON rel.revisionID = r.id
|
|
|
|
AND rel.relation = %s
|
|
|
|
AND rel.objectPHID = %s
|
2011-12-16 22:34:17 +01:00
|
|
|
WHERE r.status IN (%Ld) ORDER BY dateModified DESC',
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-08 00:35:10 +01:00
|
|
|
$table->getTableName(),
|
|
|
|
$responsible_phid,
|
|
|
|
$open_statuses,
|
|
|
|
|
|
|
|
$table->getTableName(),
|
|
|
|
DifferentialRevision::RELATIONSHIP_TABLE,
|
|
|
|
DifferentialRevision::RELATION_REVIEWER,
|
|
|
|
$responsible_phid,
|
|
|
|
$open_statuses);
|
|
|
|
}
|
|
|
|
|
|
|
|
private function loadData() {
|
|
|
|
$table = new DifferentialRevision();
|
|
|
|
$conn_r = $table->establishConnection('r');
|
|
|
|
|
2012-10-04 06:16:57 +02:00
|
|
|
if ($this->draftAuthors) {
|
2012-10-09 20:49:48 +02:00
|
|
|
$this->draftRevisions = array();
|
|
|
|
|
2012-10-04 06:16:57 +02:00
|
|
|
$draft_key = 'differential-comment-';
|
|
|
|
$drafts = id(new PhabricatorDraft())->loadAllWhere(
|
|
|
|
'authorPHID IN (%Ls) AND draftKey LIKE %> AND draft != %s',
|
|
|
|
$this->draftAuthors,
|
|
|
|
$draft_key,
|
|
|
|
'');
|
|
|
|
$len = strlen($draft_key);
|
|
|
|
foreach ($drafts as $draft) {
|
|
|
|
$this->draftRevisions[] = substr($draft->getDraftKey(), $len);
|
|
|
|
}
|
2012-10-09 20:49:48 +02:00
|
|
|
|
|
|
|
$inlines = id(new DifferentialInlineComment())->loadAllWhere(
|
|
|
|
'commentID IS NULL AND authorPHID IN (%Ls)',
|
|
|
|
$this->draftAuthors);
|
|
|
|
foreach ($inlines as $inline) {
|
|
|
|
$this->draftRevisions[] = $inline->getRevisionID();
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!$this->draftRevisions) {
|
|
|
|
return array();
|
|
|
|
}
|
2012-10-04 06:16:57 +02:00
|
|
|
}
|
|
|
|
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-08 00:35:10 +01:00
|
|
|
$select = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'SELECT r.* FROM %T r',
|
|
|
|
$table->getTableName());
|
|
|
|
|
|
|
|
$joins = $this->buildJoinsClause($conn_r);
|
|
|
|
$where = $this->buildWhereClause($conn_r);
|
|
|
|
$group_by = $this->buildGroupByClause($conn_r);
|
|
|
|
$order_by = $this->buildOrderByClause($conn_r);
|
|
|
|
|
|
|
|
$limit = '';
|
|
|
|
if ($this->offset || $this->limit) {
|
|
|
|
$limit = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'LIMIT %d, %d',
|
|
|
|
(int)$this->offset,
|
|
|
|
$this->limit);
|
|
|
|
}
|
|
|
|
|
|
|
|
return queryfx_all(
|
|
|
|
$conn_r,
|
|
|
|
'%Q %Q %Q %Q %Q %Q',
|
|
|
|
$select,
|
|
|
|
$joins,
|
|
|
|
$where,
|
|
|
|
$group_by,
|
|
|
|
$order_by,
|
|
|
|
$limit);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2011-10-02 21:03:16 +02:00
|
|
|
/* -( Internals )---------------------------------------------------------- */
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task internal
|
|
|
|
*/
|
|
|
|
private function buildJoinsClause($conn_r) {
|
|
|
|
$joins = array();
|
|
|
|
if ($this->pathIDs) {
|
|
|
|
$path_table = new DifferentialAffectedPath();
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'JOIN %T p ON p.revisionID = r.id',
|
|
|
|
$path_table->getTableName());
|
|
|
|
}
|
|
|
|
|
2012-01-04 06:08:12 +01:00
|
|
|
if ($this->commitHashes) {
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'JOIN %T hash_rel ON hash_rel.revisionID = r.id',
|
2012-01-10 20:39:11 +01:00
|
|
|
ArcanistDifferentialRevisionHash::TABLE_NAME);
|
2012-01-04 06:08:12 +01:00
|
|
|
}
|
|
|
|
|
2011-12-06 23:21:36 +01:00
|
|
|
if ($this->ccs) {
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
2011-12-07 17:30:49 +01:00
|
|
|
'JOIN %T cc_rel ON cc_rel.revisionID = r.id '.
|
|
|
|
'AND cc_rel.relation = %s '.
|
|
|
|
'AND cc_rel.objectPHID in (%Ls)',
|
2011-12-06 23:21:36 +01:00
|
|
|
DifferentialRevision::RELATIONSHIP_TABLE,
|
|
|
|
DifferentialRevision::RELATION_SUBSCRIBED,
|
|
|
|
$this->ccs);
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->reviewers) {
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
2011-12-07 17:30:49 +01:00
|
|
|
'JOIN %T reviewer_rel ON reviewer_rel.revisionID = r.id '.
|
|
|
|
'AND reviewer_rel.relation = %s '.
|
|
|
|
'AND reviewer_rel.objectPHID in (%Ls)',
|
2011-12-06 23:21:36 +01:00
|
|
|
DifferentialRevision::RELATIONSHIP_TABLE,
|
|
|
|
DifferentialRevision::RELATION_REVIEWER,
|
|
|
|
$this->reviewers);
|
|
|
|
}
|
|
|
|
|
2011-12-07 17:30:49 +01:00
|
|
|
if ($this->subscribers) {
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'JOIN %T sub_rel ON sub_rel.revisionID = r.id '.
|
|
|
|
'AND sub_rel.relation IN (%Ls) '.
|
|
|
|
'AND sub_rel.objectPHID in (%Ls)',
|
|
|
|
DifferentialRevision::RELATIONSHIP_TABLE,
|
|
|
|
array(
|
|
|
|
DifferentialRevision::RELATION_SUBSCRIBED,
|
|
|
|
DifferentialRevision::RELATION_REVIEWER,
|
|
|
|
),
|
|
|
|
$this->subscribers);
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->responsibles) {
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'LEFT JOIN %T responsibles_rel ON responsibles_rel.revisionID = r.id '.
|
|
|
|
'AND responsibles_rel.relation = %s '.
|
|
|
|
'AND responsibles_rel.objectPHID in (%Ls)',
|
|
|
|
DifferentialRevision::RELATIONSHIP_TABLE,
|
|
|
|
DifferentialRevision::RELATION_REVIEWER,
|
|
|
|
$this->responsibles);
|
|
|
|
}
|
|
|
|
|
2011-10-02 21:03:16 +02:00
|
|
|
$joins = implode(' ', $joins);
|
|
|
|
|
|
|
|
return $joins;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task internal
|
|
|
|
*/
|
|
|
|
private function buildWhereClause($conn_r) {
|
|
|
|
$where = array();
|
|
|
|
|
|
|
|
if ($this->pathIDs) {
|
|
|
|
$path_clauses = array();
|
|
|
|
$repo_info = igroup($this->pathIDs, 'repositoryID');
|
|
|
|
foreach ($repo_info as $repository_id => $paths) {
|
|
|
|
$path_clauses[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'(p.repositoryID = %d AND p.pathID IN (%Ld))',
|
2011-10-02 21:03:16 +02:00
|
|
|
$repository_id,
|
|
|
|
ipull($paths, 'pathID'));
|
|
|
|
}
|
|
|
|
$path_clauses = '('.implode(' OR ', $path_clauses).')';
|
|
|
|
$where[] = $path_clauses;
|
|
|
|
}
|
|
|
|
|
2011-12-06 23:21:36 +01:00
|
|
|
if ($this->authors) {
|
2011-12-02 01:57:06 +01:00
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'r.authorPHID IN (%Ls)',
|
2011-12-06 23:21:36 +01:00
|
|
|
$this->authors);
|
2011-12-02 01:57:06 +01:00
|
|
|
}
|
|
|
|
|
2012-10-09 20:49:48 +02:00
|
|
|
if ($this->draftRevisions) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'r.id IN (%Ld)',
|
|
|
|
$this->draftRevisions);
|
2012-03-20 02:35:32 +01:00
|
|
|
}
|
2012-10-09 20:49:48 +02:00
|
|
|
|
2011-12-02 01:57:06 +01:00
|
|
|
if ($this->revIDs) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'r.id IN (%Ld)',
|
2011-12-02 01:57:06 +01:00
|
|
|
$this->revIDs);
|
|
|
|
}
|
|
|
|
|
2012-01-04 06:08:12 +01:00
|
|
|
if ($this->commitHashes) {
|
|
|
|
$hash_clauses = array();
|
|
|
|
foreach ($this->commitHashes as $info) {
|
|
|
|
list($type, $hash) = $info;
|
|
|
|
$hash_clauses[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'(hash_rel.type = %s AND hash_rel.hash = %s)',
|
|
|
|
$type,
|
|
|
|
$hash);
|
|
|
|
}
|
|
|
|
$hash_clauses = '('.implode(' OR ', $hash_clauses).')';
|
|
|
|
$where[] = $hash_clauses;
|
|
|
|
}
|
|
|
|
|
2011-12-02 01:57:06 +01:00
|
|
|
if ($this->phids) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'r.phid IN (%Ls)',
|
2011-12-02 01:57:06 +01:00
|
|
|
$this->phids);
|
|
|
|
}
|
|
|
|
|
2011-12-07 17:30:49 +01:00
|
|
|
if ($this->responsibles) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'(responsibles_rel.objectPHID IS NOT NULL OR r.authorPHID IN (%Ls))',
|
|
|
|
$this->responsibles);
|
|
|
|
}
|
|
|
|
|
2012-04-10 21:51:34 +02:00
|
|
|
if ($this->branches) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'r.branchName in (%Ls)',
|
|
|
|
$this->branches);
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->arcanistProjectPHIDs) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'r.arcanistProjectPHID in (%Ls)',
|
|
|
|
$this->arcanistProjectPHIDs);
|
|
|
|
}
|
|
|
|
|
2011-10-02 21:03:16 +02:00
|
|
|
switch ($this->status) {
|
|
|
|
case self::STATUS_ANY:
|
|
|
|
break;
|
|
|
|
case self::STATUS_OPEN:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'r.status IN (%Ld)',
|
2011-10-02 21:03:16 +02:00
|
|
|
array(
|
2012-01-10 20:39:11 +01:00
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVIEW,
|
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVISION,
|
|
|
|
ArcanistDifferentialRevisionStatus::ACCEPTED,
|
2011-10-02 21:03:16 +02:00
|
|
|
));
|
|
|
|
break;
|
2012-06-29 00:26:00 +02:00
|
|
|
case self::STATUS_NEEDS_REVIEW:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'r.status IN (%Ld)',
|
2012-06-29 00:26:00 +02:00
|
|
|
array(
|
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVIEW,
|
|
|
|
));
|
|
|
|
break;
|
2012-01-25 23:50:34 +01:00
|
|
|
case self::STATUS_ACCEPTED:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'r.status IN (%Ld)',
|
2012-01-25 23:50:34 +01:00
|
|
|
array(
|
|
|
|
ArcanistDifferentialRevisionStatus::ACCEPTED,
|
|
|
|
));
|
|
|
|
break;
|
2012-01-13 01:56:07 +01:00
|
|
|
case self::STATUS_COMMITTED:
|
2012-04-24 02:40:57 +02:00
|
|
|
phlog(
|
|
|
|
"WARNING: DifferentialRevisionQuery using deprecated ".
|
|
|
|
"STATUS_COMMITTED constant. This will be removed soon. ".
|
|
|
|
"Use STATUS_CLOSED.");
|
|
|
|
// fallthrough
|
|
|
|
case self::STATUS_CLOSED:
|
2012-01-13 01:56:07 +01:00
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'r.status IN (%Ld)',
|
2012-01-13 01:56:07 +01:00
|
|
|
array(
|
2012-04-24 02:40:57 +02:00
|
|
|
ArcanistDifferentialRevisionStatus::CLOSED,
|
2012-01-13 01:56:07 +01:00
|
|
|
));
|
|
|
|
break;
|
2012-03-07 01:42:41 +01:00
|
|
|
case self::STATUS_ABANDONED:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2012-08-17 03:32:57 +02:00
|
|
|
'r.status IN (%Ld)',
|
2012-03-07 01:42:41 +01:00
|
|
|
array(
|
|
|
|
ArcanistDifferentialRevisionStatus::ABANDONED,
|
|
|
|
));
|
|
|
|
break;
|
2011-10-02 21:03:16 +02:00
|
|
|
default:
|
|
|
|
throw new Exception(
|
|
|
|
"Unknown revision status filter constant '{$this->status}'!");
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($where) {
|
|
|
|
$where = 'WHERE '.implode(' AND ', $where);
|
|
|
|
} else {
|
|
|
|
$where = '';
|
|
|
|
}
|
|
|
|
|
|
|
|
return $where;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task internal
|
|
|
|
*/
|
|
|
|
private function buildGroupByClause($conn_r) {
|
2011-12-07 17:30:49 +01:00
|
|
|
$join_triggers = array_merge(
|
|
|
|
$this->pathIDs,
|
|
|
|
$this->ccs,
|
|
|
|
$this->reviewers,
|
|
|
|
$this->subscribers,
|
|
|
|
$this->responsibles);
|
|
|
|
|
|
|
|
$needs_distinct = (count($join_triggers) > 1);
|
2011-10-02 21:03:16 +02:00
|
|
|
|
|
|
|
if ($needs_distinct) {
|
|
|
|
return 'GROUP BY r.id';
|
|
|
|
} else {
|
|
|
|
return '';
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task internal
|
|
|
|
*/
|
|
|
|
private function buildOrderByClause($conn_r) {
|
|
|
|
switch ($this->order) {
|
|
|
|
case self::ORDER_MODIFIED:
|
|
|
|
return 'ORDER BY r.dateModified DESC';
|
|
|
|
case self::ORDER_CREATED:
|
|
|
|
return 'ORDER BY r.dateCreated DESC';
|
|
|
|
case self::ORDER_PATH_MODIFIED:
|
|
|
|
if (!$this->pathIDs) {
|
|
|
|
throw new Exception(
|
|
|
|
"To use ORDER_PATH_MODIFIED, you must specify withPath().");
|
|
|
|
}
|
|
|
|
return 'ORDER BY p.epoch DESC';
|
|
|
|
default:
|
|
|
|
throw new Exception("Unknown query order constant '{$this->order}'.");
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2011-12-21 02:05:52 +01:00
|
|
|
private function loadRelationships($conn_r, array $revisions) {
|
2012-04-04 22:13:08 +02:00
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
2011-12-21 02:05:52 +01:00
|
|
|
$relationships = queryfx_all(
|
|
|
|
$conn_r,
|
|
|
|
'SELECT * FROM %T WHERE revisionID in (%Ld) ORDER BY sequence',
|
|
|
|
DifferentialRevision::RELATIONSHIP_TABLE,
|
|
|
|
mpull($revisions, 'getID'));
|
|
|
|
$relationships = igroup($relationships, 'revisionID');
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$revision->attachRelationships(
|
|
|
|
idx(
|
|
|
|
$relationships,
|
|
|
|
$revision->getID(),
|
|
|
|
array()));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
private function loadCommitPHIDs($conn_r, array $revisions) {
|
2012-04-04 22:13:08 +02:00
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
2011-12-21 02:05:52 +01:00
|
|
|
$commit_phids = queryfx_all(
|
|
|
|
$conn_r,
|
|
|
|
'SELECT * FROM %T WHERE revisionID IN (%Ld)',
|
|
|
|
DifferentialRevision::TABLE_COMMIT,
|
|
|
|
mpull($revisions, 'getID'));
|
|
|
|
$commit_phids = igroup($commit_phids, 'revisionID');
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$phids = idx($commit_phids, $revision->getID(), array());
|
|
|
|
$phids = ipull($phids, 'commitPHID');
|
|
|
|
$revision->attachCommitPHIDs($phids);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
private function loadDiffIDs($conn_r, array $revisions) {
|
2012-04-04 22:13:08 +02:00
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
|
|
|
|
2011-12-21 02:05:52 +01:00
|
|
|
$diff_table = new DifferentialDiff();
|
|
|
|
|
|
|
|
$diff_ids = queryfx_all(
|
|
|
|
$conn_r,
|
|
|
|
'SELECT revisionID, id FROM %T WHERE revisionID IN (%Ld)
|
|
|
|
ORDER BY id DESC',
|
|
|
|
$diff_table->getTableName(),
|
|
|
|
mpull($revisions, 'getID'));
|
|
|
|
$diff_ids = igroup($diff_ids, 'revisionID');
|
|
|
|
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$ids = idx($diff_ids, $revision->getID(), array());
|
|
|
|
$ids = ipull($ids, 'id');
|
|
|
|
$revision->attachDiffIDs($ids);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
private function loadActiveDiffs($conn_r, array $revisions) {
|
2012-04-04 22:13:08 +02:00
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
|
|
|
|
2011-12-21 02:05:52 +01:00
|
|
|
$diff_table = new DifferentialDiff();
|
|
|
|
|
|
|
|
$load_ids = array();
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$diffs = $revision->getDiffIDs();
|
|
|
|
if ($diffs) {
|
|
|
|
$load_ids[] = max($diffs);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$active_diffs = array();
|
|
|
|
if ($load_ids) {
|
|
|
|
$active_diffs = $diff_table->loadAllWhere(
|
|
|
|
'id IN (%Ld)',
|
|
|
|
$load_ids);
|
|
|
|
}
|
|
|
|
|
|
|
|
$active_diffs = mpull($active_diffs, null, 'getRevisionID');
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$revision->attachActiveDiff(idx($active_diffs, $revision->getID()));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2012-06-26 18:07:52 +02:00
|
|
|
private function loadHashes(
|
|
|
|
AphrontDatabaseConnection $conn_r,
|
|
|
|
array $revisions) {
|
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
|
|
|
|
|
|
|
$data = queryfx_all(
|
|
|
|
$conn_r,
|
|
|
|
'SELECT * FROM %T WHERE revisionID IN (%Ld)',
|
|
|
|
'differential_revisionhash',
|
|
|
|
mpull($revisions, 'getID'));
|
|
|
|
|
|
|
|
$data = igroup($data, 'revisionID');
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$hashes = idx($data, $revision->getID(), array());
|
|
|
|
$list = array();
|
|
|
|
foreach ($hashes as $hash) {
|
|
|
|
$list[] = array($hash['type'], $hash['hash']);
|
|
|
|
}
|
|
|
|
$revision->attachHashes($list);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2013-01-11 21:18:34 +01:00
|
|
|
public static function splitResponsible(array $revisions, array $user_phids) {
|
2013-01-24 01:37:08 +01:00
|
|
|
$blocking = array();
|
Replace home directory list with a dashboard
Summary:
Rough cut that still needs a lot of polish, but replace the directory list with
more of a dashboard type thing:
- Show "Unbreak Now", triage-in-your-projects, and other stuff that you're
supposed to deal with, then feed.
- Move tools a click a way behind nav -- this also lets us put more stuff
there and subtools, etc., later.
- Remove tabs.
- Merge the category/item editing views.
- I also added a light blue wash to the side nav, not sure if I like that or
not.
Test Plan:
- Viewed all elements in empty and nonempty states.
- Viewed applications, edited items/categories.
Reviewers: btrahan, aran
Reviewed By: btrahan
CC: aran, epriestley, davidreuss
Maniphest Tasks: T21, T631
Differential Revision: https://secure.phabricator.com/D1574
2012-02-08 01:04:48 +01:00
|
|
|
$active = array();
|
|
|
|
$waiting = array();
|
|
|
|
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
|
|
|
|
2013-01-24 01:37:08 +01:00
|
|
|
// Bucket revisions into $blocking (revisions where you are blocking
|
|
|
|
// others), $active (revisions you need to do something about) and $waiting
|
|
|
|
// (revisions you're waiting on someone else to do something about).
|
Replace home directory list with a dashboard
Summary:
Rough cut that still needs a lot of polish, but replace the directory list with
more of a dashboard type thing:
- Show "Unbreak Now", triage-in-your-projects, and other stuff that you're
supposed to deal with, then feed.
- Move tools a click a way behind nav -- this also lets us put more stuff
there and subtools, etc., later.
- Remove tabs.
- Merge the category/item editing views.
- I also added a light blue wash to the side nav, not sure if I like that or
not.
Test Plan:
- Viewed all elements in empty and nonempty states.
- Viewed applications, edited items/categories.
Reviewers: btrahan, aran
Reviewed By: btrahan
CC: aran, epriestley, davidreuss
Maniphest Tasks: T21, T631
Differential Revision: https://secure.phabricator.com/D1574
2012-02-08 01:04:48 +01:00
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$needs_review = ($revision->getStatus() == $status_review);
|
2013-01-11 21:18:34 +01:00
|
|
|
$filter_is_author = in_array($revision->getAuthorPHID(), $user_phids);
|
2013-02-21 12:10:02 +01:00
|
|
|
if (!$revision->getReviewers()) {
|
|
|
|
$needs_review = false;
|
|
|
|
}
|
Replace home directory list with a dashboard
Summary:
Rough cut that still needs a lot of polish, but replace the directory list with
more of a dashboard type thing:
- Show "Unbreak Now", triage-in-your-projects, and other stuff that you're
supposed to deal with, then feed.
- Move tools a click a way behind nav -- this also lets us put more stuff
there and subtools, etc., later.
- Remove tabs.
- Merge the category/item editing views.
- I also added a light blue wash to the side nav, not sure if I like that or
not.
Test Plan:
- Viewed all elements in empty and nonempty states.
- Viewed applications, edited items/categories.
Reviewers: btrahan, aran
Reviewed By: btrahan
CC: aran, epriestley, davidreuss
Maniphest Tasks: T21, T631
Differential Revision: https://secure.phabricator.com/D1574
2012-02-08 01:04:48 +01:00
|
|
|
|
|
|
|
// If exactly one of "needs review" and "the user is the author" is
|
|
|
|
// true, the user needs to act on it. Otherwise, they're waiting on
|
|
|
|
// it.
|
|
|
|
if ($needs_review ^ $filter_is_author) {
|
2013-01-24 01:37:08 +01:00
|
|
|
if ($needs_review) {
|
2013-01-25 01:09:16 +01:00
|
|
|
array_unshift($blocking, $revision);
|
2013-01-24 01:37:08 +01:00
|
|
|
} else {
|
|
|
|
$active[] = $revision;
|
|
|
|
}
|
Replace home directory list with a dashboard
Summary:
Rough cut that still needs a lot of polish, but replace the directory list with
more of a dashboard type thing:
- Show "Unbreak Now", triage-in-your-projects, and other stuff that you're
supposed to deal with, then feed.
- Move tools a click a way behind nav -- this also lets us put more stuff
there and subtools, etc., later.
- Remove tabs.
- Merge the category/item editing views.
- I also added a light blue wash to the side nav, not sure if I like that or
not.
Test Plan:
- Viewed all elements in empty and nonempty states.
- Viewed applications, edited items/categories.
Reviewers: btrahan, aran
Reviewed By: btrahan
CC: aran, epriestley, davidreuss
Maniphest Tasks: T21, T631
Differential Revision: https://secure.phabricator.com/D1574
2012-02-08 01:04:48 +01:00
|
|
|
} else {
|
|
|
|
$waiting[] = $revision;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2013-01-24 01:37:08 +01:00
|
|
|
return array($blocking, $active, $waiting);
|
Replace home directory list with a dashboard
Summary:
Rough cut that still needs a lot of polish, but replace the directory list with
more of a dashboard type thing:
- Show "Unbreak Now", triage-in-your-projects, and other stuff that you're
supposed to deal with, then feed.
- Move tools a click a way behind nav -- this also lets us put more stuff
there and subtools, etc., later.
- Remove tabs.
- Merge the category/item editing views.
- I also added a light blue wash to the side nav, not sure if I like that or
not.
Test Plan:
- Viewed all elements in empty and nonempty states.
- Viewed applications, edited items/categories.
Reviewers: btrahan, aran
Reviewed By: btrahan
CC: aran, epriestley, davidreuss
Maniphest Tasks: T21, T631
Differential Revision: https://secure.phabricator.com/D1574
2012-02-08 01:04:48 +01:00
|
|
|
}
|
|
|
|
|
2011-10-02 21:03:16 +02:00
|
|
|
|
|
|
|
}
|