1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 08:12:40 +01:00
phorge-phorge/src/applications/differential/controller/DifferentialRevisionListController.php

37 lines
908 B
PHP
Raw Normal View History

2011-01-26 00:19:06 +01:00
<?php
final class DifferentialRevisionListController extends DifferentialController {
2011-01-26 00:19:06 +01:00
Use ApplicationSearch in Differential Summary: Ref T603. Ref T2625. Fixes T3241. Depends on D5451. Depends on D6346. @wez, this changes the Differential revision list UI substantially and may generate a lot of bikeshedding / who-moved-my-cheese churn. See T3417 for context, for example. The motivations for this change are: - The list now works on devices, like phones and tablets. This is a requirement to make the rest of Differential work on devices. - Although ApplicationSearch intentionally presents a simpler interface initially and some options which were one click away before aren't now, it is much more powerful than the search it replaces and allows users to build, save, share, fork, edit, and customize a much wider range of queries. Users who used the old filters frequently can use Advanced Search -> Save Custom Query to create new versions of them, and of any other query. "Edit Queries.." allows users to remove and reorder queries, including builtin queries. Basically, there are like three things which have gone from "1-click" to "a few clicks", and ten trillion things which have gone from "hard/impossible" to "relatively easy". The local screenshots look a bit iffy, but I think a lot of this is the fakenesss of my test data. If they still feel iffy in production we can tweak them until they feel good, like we did for Maniphest. Test Plan: {F48477} {F48478} Reviewers: btrahan, chad, wez Reviewed By: btrahan CC: aran, s Maniphest Tasks: T603, T2625, T3241 Differential Revision: https://secure.phabricator.com/D6347
2013-07-03 15:11:07 +02:00
private $queryKey;
2011-01-27 20:35:04 +01:00
Use ApplicationSearch in Differential Summary: Ref T603. Ref T2625. Fixes T3241. Depends on D5451. Depends on D6346. @wez, this changes the Differential revision list UI substantially and may generate a lot of bikeshedding / who-moved-my-cheese churn. See T3417 for context, for example. The motivations for this change are: - The list now works on devices, like phones and tablets. This is a requirement to make the rest of Differential work on devices. - Although ApplicationSearch intentionally presents a simpler interface initially and some options which were one click away before aren't now, it is much more powerful than the search it replaces and allows users to build, save, share, fork, edit, and customize a much wider range of queries. Users who used the old filters frequently can use Advanced Search -> Save Custom Query to create new versions of them, and of any other query. "Edit Queries.." allows users to remove and reorder queries, including builtin queries. Basically, there are like three things which have gone from "1-click" to "a few clicks", and ten trillion things which have gone from "hard/impossible" to "relatively easy". The local screenshots look a bit iffy, but I think a lot of this is the fakenesss of my test data. If they still feel iffy in production we can tweak them until they feel good, like we did for Maniphest. Test Plan: {F48477} {F48478} Reviewers: btrahan, chad, wez Reviewed By: btrahan CC: aran, s Maniphest Tasks: T603, T2625, T3241 Differential Revision: https://secure.phabricator.com/D6347
2013-07-03 15:11:07 +02:00
public function shouldAllowPublic() {
return true;
}
2011-01-27 20:35:04 +01:00
public function willProcessRequest(array $data) {
Use ApplicationSearch in Differential Summary: Ref T603. Ref T2625. Fixes T3241. Depends on D5451. Depends on D6346. @wez, this changes the Differential revision list UI substantially and may generate a lot of bikeshedding / who-moved-my-cheese churn. See T3417 for context, for example. The motivations for this change are: - The list now works on devices, like phones and tablets. This is a requirement to make the rest of Differential work on devices. - Although ApplicationSearch intentionally presents a simpler interface initially and some options which were one click away before aren't now, it is much more powerful than the search it replaces and allows users to build, save, share, fork, edit, and customize a much wider range of queries. Users who used the old filters frequently can use Advanced Search -> Save Custom Query to create new versions of them, and of any other query. "Edit Queries.." allows users to remove and reorder queries, including builtin queries. Basically, there are like three things which have gone from "1-click" to "a few clicks", and ten trillion things which have gone from "hard/impossible" to "relatively easy". The local screenshots look a bit iffy, but I think a lot of this is the fakenesss of my test data. If they still feel iffy in production we can tweak them until they feel good, like we did for Maniphest. Test Plan: {F48477} {F48478} Reviewers: btrahan, chad, wez Reviewed By: btrahan CC: aran, s Maniphest Tasks: T603, T2625, T3241 Differential Revision: https://secure.phabricator.com/D6347
2013-07-03 15:11:07 +02:00
$this->queryKey = idx($data, 'queryKey');
2011-01-27 20:35:04 +01:00
}
2011-01-26 00:19:06 +01:00
public function processRequest() {
Decouple some aspects of request routing and construction Summary: Ref T5702. This is a forward-looking change which provides some very broad API improvements but does not implement them. In particular: - Controllers no longer require `$request` to construct. This is mostly for T5702, directly, but simplifies things in general. Instead, we call `setRequest()` before using a controller. Only a small number of sites activate controllers, so this is less code overall, and more consistent with most constructors not having any parameters or effects. - `$request` now offers `getURIData($key, ...)`. This is an alternate way of accessing `$data` which is currently only available on `willProcessRequest(array $data)`. Almost all controllers which implement this method do so in order to read one or two things out of the URI data. Instead, let them just read this data directly when processing the request. - Introduce `handleRequest(AphrontRequest $request)` and deprecate (very softly) `processRequest()`. The majority of `processRequest()` calls begin `$request = $this->getRequest()`, which is avoided with the more practical signature. - Provide `getViewer()` on `$request`, and a convenience `getViewer()` on `$controller`. This fixes `$viewer = $request->getUser();` into `$viewer = $request->getViewer();`, and converts the `$request + $viewer` two-liner into a single `$this->getViewer()`. Test Plan: - Browsed around in general. - Hit special controllers (redirect, 404). - Hit AuditList controller (uses new style). Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T5702 Differential Revision: https://secure.phabricator.com/D10698
2014-10-17 14:01:40 +02:00
$controller = id(new PhabricatorApplicationSearchController())
Use ApplicationSearch in Differential Summary: Ref T603. Ref T2625. Fixes T3241. Depends on D5451. Depends on D6346. @wez, this changes the Differential revision list UI substantially and may generate a lot of bikeshedding / who-moved-my-cheese churn. See T3417 for context, for example. The motivations for this change are: - The list now works on devices, like phones and tablets. This is a requirement to make the rest of Differential work on devices. - Although ApplicationSearch intentionally presents a simpler interface initially and some options which were one click away before aren't now, it is much more powerful than the search it replaces and allows users to build, save, share, fork, edit, and customize a much wider range of queries. Users who used the old filters frequently can use Advanced Search -> Save Custom Query to create new versions of them, and of any other query. "Edit Queries.." allows users to remove and reorder queries, including builtin queries. Basically, there are like three things which have gone from "1-click" to "a few clicks", and ten trillion things which have gone from "hard/impossible" to "relatively easy". The local screenshots look a bit iffy, but I think a lot of this is the fakenesss of my test data. If they still feel iffy in production we can tweak them until they feel good, like we did for Maniphest. Test Plan: {F48477} {F48478} Reviewers: btrahan, chad, wez Reviewed By: btrahan CC: aran, s Maniphest Tasks: T603, T2625, T3241 Differential Revision: https://secure.phabricator.com/D6347
2013-07-03 15:11:07 +02:00
->setQueryKey($this->queryKey)
->setSearchEngine(new DifferentialRevisionSearchEngine())
->setNavigation($this->buildSideNavView());
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
Use ApplicationSearch in Differential Summary: Ref T603. Ref T2625. Fixes T3241. Depends on D5451. Depends on D6346. @wez, this changes the Differential revision list UI substantially and may generate a lot of bikeshedding / who-moved-my-cheese churn. See T3417 for context, for example. The motivations for this change are: - The list now works on devices, like phones and tablets. This is a requirement to make the rest of Differential work on devices. - Although ApplicationSearch intentionally presents a simpler interface initially and some options which were one click away before aren't now, it is much more powerful than the search it replaces and allows users to build, save, share, fork, edit, and customize a much wider range of queries. Users who used the old filters frequently can use Advanced Search -> Save Custom Query to create new versions of them, and of any other query. "Edit Queries.." allows users to remove and reorder queries, including builtin queries. Basically, there are like three things which have gone from "1-click" to "a few clicks", and ten trillion things which have gone from "hard/impossible" to "relatively easy". The local screenshots look a bit iffy, but I think a lot of this is the fakenesss of my test data. If they still feel iffy in production we can tweak them until they feel good, like we did for Maniphest. Test Plan: {F48477} {F48478} Reviewers: btrahan, chad, wez Reviewed By: btrahan CC: aran, s Maniphest Tasks: T603, T2625, T3241 Differential Revision: https://secure.phabricator.com/D6347
2013-07-03 15:11:07 +02:00
return $this->delegateToController($controller);
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
}
protected function buildApplicationCrumbs() {
$crumbs = parent::buildApplicationCrumbs();
$crumbs->addAction(
id(new PHUIListItemView())
->setHref($this->getApplicationURI('/diff/create/'))
->setName(pht('Create Diff'))
->setIcon('fa-plus-square'));
return $crumbs;
}
2011-01-26 00:19:06 +01:00
}