See also T8080, T4144, T1279, and T731

(Just clarifying the title so I don't confuse myself later about this relating to the "Projects" field.)

Differential's home screen currently has some logic which "buckets" revisions based on the viewer's relationship to them, into "Blocking Others", "Action Required" and "Waiting on Others".

This bucketing is broadly useful, but is hard-coded to the specify query key -- the code literally checks if you're viewing "Active Revisions" and does something different and magical:

if ($query->getQueryKey() == 'active') {

This is the only place in the codebase that we do this. I'd like to find a generic, standard way to express this view without any hard-coding; if we could, it would almost certainly resolve this problem implicitly.

This is partly made complicated because these buckets are really unions of two queries: open stuff some users authored, and open stuff some users are reviewing.

This is further complicated by the tasks @chad links to. In particular, T731 discusses making reviewer rules modular to address a wide range of desired reviewer rulesets (like "everyone must review", "at least N reviewers must review", etc). These rulesets would change everything that touches reviewers, and make it difficult or impossible to perform any bucketing or bucket-based filtering purely in the database without denormalizing some sort of new "user state" relationship. It's possible we want or need to do this, but it's a bit of a mess to do.

If we did, we could add query fields per user state:

Others are blocked by:
Action required by:
Owned, but not actionable, by:

But, e.g., T4144 doesn't even like these buckets at all, and the labels above are admittedly unclear. T4144 suggests 6 buckets instead:

Needs review by:
Needs revision by:
Needs land by:
With authors, waiting for review by others:
With reviewers, waiting for revision by others:
With reviewers, waiting for land by others:

This is further complicated because it's desirable for these fields to accept lists of users (e.g., so a manager can go check what all their reports are up before a 1:1), but the bucket that a revision goes in depends on the viewer. For example, a revision with two reviewers (A, who has rejected; B, a blocking reviewer who has not reviewed yet) both needs revision by the author and needs review by B.

A tangential issue with the UI is this:

Needs review by: [ epriestley ]

Does this mean "epriestley", or "epriestley and any project epriestley belongs to"? How do users query for the other one if they don't want whatever this means? We are currently inconsistent about this, and it's sometimes impossible to run the other query.

The original request here is entirely reasonable, and I believe we could address it by implementing a "Blocked on review by: ..." field, but I hesitate to do this because I'm not sure that technique can survive T731 / T4144 or actually represents a pathway forward for this UI. I'd like to have a clearer goal in mind for where this UI should go before moving it forward.

I agree. This can't be the long term result of the UI.

I think there are a whole host of improvements that can be done here: I, e.g., also think that the Herald rule mechanisms can be replaced with a server side imperative php class. But, right now, this is how the functionality works and it is a big burden on us: I think it is a reasonable option to give people while this gets more developed. Is there like a plugin system that could provide this for us? Or a patch that then individuals could use temporarily?

Here's an extension you can drop in src/extensions/. This is unsupported and the code is terrible, etc. This is purely intended to be a stopgap until we find a more permanent solution which is maintainable in the upstream.

Drop these files into phabricator/src/extensions/

• DifferentialExtensionsApplication.php392 BDownload
• DifferentialExtensionsRevisionSearchEngine.php12 KBDownload
• DifferentialExtensionsRevisionListController.php917 BDownload

Now visit /differential-extensions/ in the web UI and you should have a new "Exact Blocking or Rejected Reviewers" field:

You should be able to use this field to find revisions blocked on review by a project like Security.

Let me know if that solves the immediate issue for you.

I've also made a small foray against this in D14013, in Audit, which faces a similar set of problems:

Does this mean "epriestley", or "epriestley and any project epriestley belongs to"?

I made this the default state for the prebuilt query:

I'm not sure if that's really clear enough in the general case, but the old state was garbage so it's progress in that UI at least. If it proves unduly confusing we can re-evaluate; if it seems OK we can maybe bring that to Differential eventually.

I have marked D15925 as fixing this. T10939 has some broader discussion. This isn't an exact fix for the original request, but I suspect it may be close enough to be satisfactory.

After D15925, you can:

• Query for "Responsible Users: SomeProjectName".
• Select "Bucket: Bucket by Required Action".
• Review the first bucket ("Must Review") to find blocking and rejecting reviews by that project.
• Optionally, save this query.

This query will also list other revisions ("Ready to Review", "Waiting on Update", etc) but you can ignore those.

The extension from August will probably continue to function after these changes, but I think at least a couple of things will stop working ("Drafts" checkbox, probably "Subscribers") and more issues may arise in the future.

Some additional changes are planned in conjunction with T10939, see that task for details.

(The slightly more intuitive "Reviewers: SomeProjectName" will also work.)

thanks! Will try it out when we update here. @angie can you ping me once we update to this version on our side?

In T9263#175460, @epriestley wrote:

(The slightly more intuitive "Reviewers: SomeProjectName" will also work.)

Actually, I mistaken on this -- "Bucket: ..." is driven by "Responsible Users". But "Responsible Users: SomeProjectName" should do what you expect.

(This stuff is now in HEAD and deployed here if you want to poke at it.)