Introduce search result buckets
Summary:
Ref T10939. Currently, Differential hard-codes some behaviors for the "active" filter. This introduces "buckets" to make this grouping behavior more general/flexible.
The buckets don't actually do any grouping yet, this just gets rid of the `$query === 'active'` stuff so far.
These buckets change the page size to a large value, becuase pagination won't currently work with bucketing.
The problem is that we normally paginate by selecting one more result than we need: so if we're building a page of size 10, we'll select 11 results. This is fast, and if we get 11 back, we know there's a next page with at least one result on it.
With buckets, we can't do this, since our 11 results might come back in these buckets:
- A, B, C, A, C, C, A, A, B, B, (B)
So we know there are more results, and we know that bucket B has more results, but we have no clue if bucket A and bucket C have more results or not (or if there's anything in bucket D, etc).
We might need to select a thousand more results to get the first (D) or the next (A).
So we could render something like "Some buckets have more results, click here to go to the next page", but users would normally expect to be able to see "This specific bucket, A, has more results.", and we can't do that without a lot more work.
It doesn't really matter for revisions, because almost no one has 1K of them, but this may need to be resolved eventually.
(I have some OK-ish ideas for resolving it but nothing I'm particularly happy with.)
Test Plan: {F1376542}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10939
Differential Revision: https://secure.phabricator.com/D15923
2016-05-15 09:46:38 -07:00
|
|
|
<?php
|
|
|
|
|
|
|
|
final class DifferentialRevisionRequiredActionResultBucket
|
|
|
|
extends DifferentialRevisionResultBucket {
|
|
|
|
|
|
|
|
const BUCKETKEY = 'action';
|
|
|
|
|
2016-05-19 13:28:21 -07:00
|
|
|
const KEY_MUSTREVIEW = 'must-review';
|
|
|
|
const KEY_SHOULDREVIEW = 'should-review';
|
|
|
|
|
2016-05-16 08:09:11 -07:00
|
|
|
private $objects;
|
|
|
|
|
Introduce search result buckets
Summary:
Ref T10939. Currently, Differential hard-codes some behaviors for the "active" filter. This introduces "buckets" to make this grouping behavior more general/flexible.
The buckets don't actually do any grouping yet, this just gets rid of the `$query === 'active'` stuff so far.
These buckets change the page size to a large value, becuase pagination won't currently work with bucketing.
The problem is that we normally paginate by selecting one more result than we need: so if we're building a page of size 10, we'll select 11 results. This is fast, and if we get 11 back, we know there's a next page with at least one result on it.
With buckets, we can't do this, since our 11 results might come back in these buckets:
- A, B, C, A, C, C, A, A, B, B, (B)
So we know there are more results, and we know that bucket B has more results, but we have no clue if bucket A and bucket C have more results or not (or if there's anything in bucket D, etc).
We might need to select a thousand more results to get the first (D) or the next (A).
So we could render something like "Some buckets have more results, click here to go to the next page", but users would normally expect to be able to see "This specific bucket, A, has more results.", and we can't do that without a lot more work.
It doesn't really matter for revisions, because almost no one has 1K of them, but this may need to be resolved eventually.
(I have some OK-ish ideas for resolving it but nothing I'm particularly happy with.)
Test Plan: {F1376542}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10939
Differential Revision: https://secure.phabricator.com/D15923
2016-05-15 09:46:38 -07:00
|
|
|
public function getResultBucketName() {
|
|
|
|
return pht('Bucket by Required Action');
|
|
|
|
}
|
|
|
|
|
2016-05-16 08:09:11 -07:00
|
|
|
protected function buildResultGroups(
|
|
|
|
PhabricatorSavedQuery $query,
|
|
|
|
array $objects) {
|
|
|
|
|
|
|
|
$this->objects = $objects;
|
|
|
|
|
2017-02-17 10:10:15 +00:00
|
|
|
$phids = $query->getEvaluatedParameter('responsiblePHIDs');
|
2016-05-16 08:09:11 -07:00
|
|
|
if (!$phids) {
|
|
|
|
throw new Exception(
|
|
|
|
pht(
|
|
|
|
'You can not bucket results by required action without '.
|
|
|
|
'specifying "Responsible Users".'));
|
|
|
|
}
|
|
|
|
$phids = array_fuse($phids);
|
|
|
|
|
Store "resigned" as an explicit reviewer state
Summary:
Fixes T11050. Today, when a user resigns, we just delete the record of them ever being a reviewer.
However, this means you have no way to say "I don't care about this and don't want to see it on my dashboard" if you are a member of any project or package reviewers.
Instead, store "resigned" as a distinct state from "not a reviewer", and treat it a little differently in the UI:
- On the bucketing screen, discard revisions any responsible user has resigned from.
- On the main `/Dxxx` page, show these users as resigned explicitly (we could just hide them, too, but I think this is good to start with).
- In the query, don't treat a "resigned" state as a real "reviewer" (this change happened earlier, in D17517).
- When resigning, write a "resigned" state instead of deleting the row.
- When editing a list of reviewers, I'm still treating this reviewer as a reviewer and not special casing it. I think that's sufficiently clear but we could tailor this behavior later.
Test Plan:
- Resigned from a revision.
- Saw "Resigned" in reviewers list.
- Saw revision disappear from my dashboard.
- Edited revision, saw user still appear as an editable reviewer. Saved revision, saw no weird side effects.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11050
Differential Revision: https://secure.phabricator.com/D17531
2017-03-22 06:38:35 -07:00
|
|
|
// Before continuing, throw away any revisions which responsible users
|
|
|
|
// have explicitly resigned from.
|
|
|
|
|
|
|
|
// The goal is to allow users to resign from revisions they don't want to
|
|
|
|
// review to get these revisions off their dashboard, even if there are
|
|
|
|
// other project or package reviewers which they have authority over.
|
|
|
|
$this->filterResigned($phids);
|
|
|
|
|
2016-05-16 08:09:11 -07:00
|
|
|
$groups = array();
|
|
|
|
|
|
|
|
$groups[] = $this->newGroup()
|
|
|
|
->setName(pht('Must Review'))
|
2016-05-19 13:28:21 -07:00
|
|
|
->setKey(self::KEY_MUSTREVIEW)
|
2016-05-16 08:09:11 -07:00
|
|
|
->setNoDataString(pht('No revisions are blocked on your review.'))
|
|
|
|
->setObjects($this->filterMustReview($phids));
|
|
|
|
|
|
|
|
$groups[] = $this->newGroup()
|
|
|
|
->setName(pht('Ready to Review'))
|
2016-05-19 13:28:21 -07:00
|
|
|
->setKey(self::KEY_SHOULDREVIEW)
|
2016-05-16 08:09:11 -07:00
|
|
|
->setNoDataString(pht('No revisions are waiting on you to review them.'))
|
|
|
|
->setObjects($this->filterShouldReview($phids));
|
|
|
|
|
|
|
|
$groups[] = $this->newGroup()
|
|
|
|
->setName(pht('Ready to Land'))
|
|
|
|
->setNoDataString(pht('No revisions are ready to land.'))
|
|
|
|
->setObjects($this->filterShouldLand($phids));
|
|
|
|
|
|
|
|
$groups[] = $this->newGroup()
|
|
|
|
->setName(pht('Ready to Update'))
|
|
|
|
->setNoDataString(pht('No revisions are waiting for updates.'))
|
|
|
|
->setObjects($this->filterShouldUpdate($phids));
|
|
|
|
|
|
|
|
$groups[] = $this->newGroup()
|
|
|
|
->setName(pht('Waiting on Review'))
|
|
|
|
->setNoDataString(pht('None of your revisions are waiting on review.'))
|
|
|
|
->setObjects($this->filterWaitingForReview($phids));
|
|
|
|
|
|
|
|
$groups[] = $this->newGroup()
|
|
|
|
->setName(pht('Waiting on Authors'))
|
|
|
|
->setNoDataString(pht('No revisions are waiting on author action.'))
|
|
|
|
->setObjects($this->filterWaitingOnAuthors($phids));
|
|
|
|
|
2017-02-27 10:16:47 -08:00
|
|
|
$groups[] = $this->newGroup()
|
|
|
|
->setName(pht('Waiting on Other Reviewers'))
|
|
|
|
->setNoDataString(pht('No revisions are waiting for other reviewers.'))
|
|
|
|
->setObjects($this->filterWaitingOnOtherReviewers($phids));
|
|
|
|
|
2016-05-16 08:09:11 -07:00
|
|
|
// 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 Revisions'))
|
|
|
|
->setObjects($this->objects);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $groups;
|
|
|
|
}
|
|
|
|
|
|
|
|
private function filterMustReview(array $phids) {
|
|
|
|
$blocking = array(
|
|
|
|
DifferentialReviewerStatus::STATUS_BLOCKING,
|
|
|
|
DifferentialReviewerStatus::STATUS_REJECTED,
|
|
|
|
DifferentialReviewerStatus::STATUS_REJECTED_OLDER,
|
|
|
|
);
|
|
|
|
$blocking = array_fuse($blocking);
|
|
|
|
|
|
|
|
$objects = $this->getRevisionsUnderReview($this->objects, $phids);
|
|
|
|
|
|
|
|
$results = array();
|
|
|
|
foreach ($objects as $key => $object) {
|
|
|
|
if (!$this->hasReviewersWithStatus($object, $phids, $blocking)) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[$key] = $object;
|
|
|
|
unset($this->objects[$key]);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
|
|
|
private function filterShouldReview(array $phids) {
|
|
|
|
$reviewing = array(
|
|
|
|
DifferentialReviewerStatus::STATUS_ADDED,
|
2016-05-16 11:35:30 -07:00
|
|
|
DifferentialReviewerStatus::STATUS_COMMENTED,
|
2016-05-16 08:09:11 -07:00
|
|
|
);
|
|
|
|
$reviewing = array_fuse($reviewing);
|
|
|
|
|
|
|
|
$objects = $this->getRevisionsUnderReview($this->objects, $phids);
|
|
|
|
|
|
|
|
$results = array();
|
|
|
|
foreach ($objects as $key => $object) {
|
|
|
|
if (!$this->hasReviewersWithStatus($object, $phids, $reviewing)) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[$key] = $object;
|
|
|
|
unset($this->objects[$key]);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
|
|
|
private function filterShouldLand(array $phids) {
|
|
|
|
$status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
|
|
|
|
|
|
|
|
$objects = $this->getRevisionsAuthored($this->objects, $phids);
|
|
|
|
|
|
|
|
$results = array();
|
|
|
|
foreach ($objects as $key => $object) {
|
|
|
|
if ($object->getStatus() != $status_accepted) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[$key] = $object;
|
|
|
|
unset($this->objects[$key]);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
|
|
|
private function filterShouldUpdate(array $phids) {
|
|
|
|
$statuses = array(
|
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVISION,
|
|
|
|
ArcanistDifferentialRevisionStatus::CHANGES_PLANNED,
|
|
|
|
ArcanistDifferentialRevisionStatus::IN_PREPARATION,
|
|
|
|
);
|
|
|
|
$statuses = array_fuse($statuses);
|
|
|
|
|
|
|
|
$objects = $this->getRevisionsAuthored($this->objects, $phids);
|
|
|
|
|
|
|
|
$results = array();
|
|
|
|
foreach ($objects as $key => $object) {
|
|
|
|
if (empty($statuses[$object->getStatus()])) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[$key] = $object;
|
|
|
|
unset($this->objects[$key]);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
|
|
|
private function filterWaitingForReview(array $phids) {
|
|
|
|
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
|
|
|
|
|
|
|
$objects = $this->getRevisionsAuthored($this->objects, $phids);
|
|
|
|
|
|
|
|
$results = array();
|
|
|
|
foreach ($objects as $key => $object) {
|
|
|
|
if ($object->getStatus() != $status_review) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[$key] = $object;
|
|
|
|
unset($this->objects[$key]);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
|
|
|
private function filterWaitingOnAuthors(array $phids) {
|
|
|
|
$statuses = array(
|
|
|
|
ArcanistDifferentialRevisionStatus::ACCEPTED,
|
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVISION,
|
|
|
|
ArcanistDifferentialRevisionStatus::CHANGES_PLANNED,
|
|
|
|
ArcanistDifferentialRevisionStatus::IN_PREPARATION,
|
|
|
|
);
|
|
|
|
$statuses = array_fuse($statuses);
|
|
|
|
|
|
|
|
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
|
|
|
|
|
|
|
|
$results = array();
|
|
|
|
foreach ($objects as $key => $object) {
|
|
|
|
if (empty($statuses[$object->getStatus()])) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[$key] = $object;
|
|
|
|
unset($this->objects[$key]);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
2017-02-27 10:16:47 -08:00
|
|
|
private function filterWaitingOnOtherReviewers(array $phids) {
|
|
|
|
$statuses = array(
|
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVIEW,
|
|
|
|
);
|
|
|
|
$statuses = array_fuse($statuses);
|
|
|
|
|
|
|
|
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
|
|
|
|
|
|
|
|
$results = array();
|
|
|
|
foreach ($objects as $key => $object) {
|
|
|
|
if (!isset($statuses[$object->getStatus()])) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[$key] = $object;
|
|
|
|
unset($this->objects[$key]);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
Store "resigned" as an explicit reviewer state
Summary:
Fixes T11050. Today, when a user resigns, we just delete the record of them ever being a reviewer.
However, this means you have no way to say "I don't care about this and don't want to see it on my dashboard" if you are a member of any project or package reviewers.
Instead, store "resigned" as a distinct state from "not a reviewer", and treat it a little differently in the UI:
- On the bucketing screen, discard revisions any responsible user has resigned from.
- On the main `/Dxxx` page, show these users as resigned explicitly (we could just hide them, too, but I think this is good to start with).
- In the query, don't treat a "resigned" state as a real "reviewer" (this change happened earlier, in D17517).
- When resigning, write a "resigned" state instead of deleting the row.
- When editing a list of reviewers, I'm still treating this reviewer as a reviewer and not special casing it. I think that's sufficiently clear but we could tailor this behavior later.
Test Plan:
- Resigned from a revision.
- Saw "Resigned" in reviewers list.
- Saw revision disappear from my dashboard.
- Edited revision, saw user still appear as an editable reviewer. Saved revision, saw no weird side effects.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11050
Differential Revision: https://secure.phabricator.com/D17531
2017-03-22 06:38:35 -07:00
|
|
|
private function filterResigned(array $phids) {
|
|
|
|
$resigned = array(
|
|
|
|
DifferentialReviewerStatus::STATUS_RESIGNED,
|
|
|
|
);
|
|
|
|
$resigned = array_fuse($resigned);
|
|
|
|
|
|
|
|
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
|
|
|
|
|
|
|
|
$results = array();
|
|
|
|
foreach ($objects as $key => $object) {
|
|
|
|
if (!$this->hasReviewersWithStatus($object, $phids, $resigned)) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[$key] = $object;
|
|
|
|
unset($this->objects[$key]);
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
Introduce search result buckets
Summary:
Ref T10939. Currently, Differential hard-codes some behaviors for the "active" filter. This introduces "buckets" to make this grouping behavior more general/flexible.
The buckets don't actually do any grouping yet, this just gets rid of the `$query === 'active'` stuff so far.
These buckets change the page size to a large value, becuase pagination won't currently work with bucketing.
The problem is that we normally paginate by selecting one more result than we need: so if we're building a page of size 10, we'll select 11 results. This is fast, and if we get 11 back, we know there's a next page with at least one result on it.
With buckets, we can't do this, since our 11 results might come back in these buckets:
- A, B, C, A, C, C, A, A, B, B, (B)
So we know there are more results, and we know that bucket B has more results, but we have no clue if bucket A and bucket C have more results or not (or if there's anything in bucket D, etc).
We might need to select a thousand more results to get the first (D) or the next (A).
So we could render something like "Some buckets have more results, click here to go to the next page", but users would normally expect to be able to see "This specific bucket, A, has more results.", and we can't do that without a lot more work.
It doesn't really matter for revisions, because almost no one has 1K of them, but this may need to be resolved eventually.
(I have some OK-ish ideas for resolving it but nothing I'm particularly happy with.)
Test Plan: {F1376542}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10939
Differential Revision: https://secure.phabricator.com/D15923
2016-05-15 09:46:38 -07:00
|
|
|
}
|