Homec4science

Drive Differential landing page with DifferentialRevisionQuery, simplify UI

Authored by epriestley <git@epriestley.com> on Dec 8 2011, 00:35.

Description

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

Details

Committed
epriestley <git@epriestley.com>Dec 16 2011, 22:21
Pushed
aubortJan 31 2017, 17:16
Parents
rPH074bf4ed7d54: Add a script for purging long-lived caches
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPH4dd87f1ad3ca: Drive Differential landing page with DifferentialRevisionQuery, simplify UI (authored by epriestley <git@epriestley.com>).Dec 16 2011, 22:21