1433337 - Allow self-management of groups/projects to support group reviews

Status: RESOLVED FIXED

(Conduit :: Phabricator, enhancement)

Description

[Gregory Szorc [:gps]]

Phabricator allows you to create "Projects"/"Groups" consisting of multiple members.

Individual objects within Phabricator can be associated with 0 or more Projects.

One thing you can do with Projects is assign a review to one - either a normal review or a "blocking" review.

When a review is assigned to a Project/Group, members then have the option of "Accepting" that revision as themselves or as a member of a group.

I think the ability to assign review to a group will be a killer feature of Phabricator at Mozilla. Instead of figuring out what individual to assign review to, we can instead assign review to a group. e.g. "build system." Then, any members of the "build system" group can accept the revision. What's even cooler is that with Herald, we can create rules that automatically associate a revision with a Project/Group based on the files that changed. Not only can we use this to automatically assign review to an appropriate group based on what changed, but we can also ensure a group of people is subscribed to code relevant to their interests. For example, by automatically tagging the "build system" Project when build system files change, people can easily find and follow all changes related to the "build system."

Unfortunately, this potential awesome is currently blocked on permissions limitations.

As a normal Phabricator user, I can create new Projects. Phabricator does have a concept of "Group" that is somehow tied to Projects. However, it isn't clear if we have that feature enabled. If we do, I as a regular user don't seem to be able to create a new Group.

In addition, normal users aren't able to create rules that assign objects to Projects or Groups automatically. I can create a personal Herald rule. However, the Herald actions for associating objects with Projects are not available to me. I /think/ they are only available in the "Global" rules, which seem to only be available to admins.

I think Projects/Groups and Herald rules to automatically associate objects with Projects/Groups is a killer feature. However, the feature is significantly undermined by having to gate through site admins to make Herald rules. People don't know this feature is available to them because it is invisible. And, when they want to change things, they have to go through a formal process as opposed to clicking through the web UI. It would be rad if regular users could create Herald rules for Projects/Groups [that they are a member of].

Comment 1

[Chris Cooper [:coop] (he/him)]

I don't know much about Herald beyond what I've read here: https://secure.phabricator.com/book/phabricator/article/herald/ 
I do agree this sounds like a killer feature, and would accomplish everything I was trying to do with the shared build-config queue.

I can also understand this feature not making the cut in the Phase I roll-out. I would be interested to know the timeline for deplyment if that's the case.

gps: would the Object rule policies mentioned in that phabricator accomplish what you're looking for, i.e. monitoring on a module level?

Alternately, can personal rules be shared in a manner akin to shared queries in bugzilla, i.e. one person sets up a personal rule and everyone subscribes to it?

Comment 2

[Gregory Szorc [:gps]]

I /think/ Object rule policies only apply to commit-level events?? If I try to create a rule for Differential Revisions, it says that Object rule policies don't apply to that type of event. The only events they seem to apply to are things like "a new commit was added to the repository." That doesn't apply for code review since our workflow is to upload patches to Phabricator, not push commits to a review repository.

Phabricator does allow you to share things.

https://phabricator.services.mozilla.com/herald/query/all/ shows all active Herald rules. Although if you click on someone else's rule, it only tells you what it is. I don't see any UI to copy it, etc.

Your searches appear to be available to other people. e.g. https://phabricator.services.mozilla.com/differential/query/BxgmFOQBmhvS/ is a personal Differential query for open reviews in version-control-tools. But if I load that page not signed in, it isn't obvious what the title of the query is or who it belongs to :/ But the search parameters are populated correctly, which means you can clone the query easily enough.

You can, however, create a dashboard and have your search results feed into that. Those can be seen at https://phabricator.services.mozilla.com/dashboard/query/all/. And it appears that as long as the dashboard is public, others can view its contents, see its title, etc. Dashboards seem to be the way to do shared team views. You can even grant others edit privileges on the dashboard. However, the searches that flow into the dashboard seem to be owned by individuals. And I don't see a way to grant privileges to edit dashboards I create. That seems... limiting. What happens when the person who controls the feeds into your team's dashboard gets hit by a bus?

Comment 3

[David Lawrence [:dkl]]

(In reply to Gregory Szorc [:gps] from comment #0)
> As a normal Phabricator user, I can create new Projects. Phabricator does
> have a concept of "Group" that is somehow tied to Projects. However, it
> isn't clear if we have that feature enabled. If we do, I as a regular user
> don't seem to be able to create a new Group.

There isn't a concept of 'groups' although there is an icon called 'Group' which I use for the bmo-* related security groups. So just use that.

> In addition, normal users aren't able to create rules that assign objects to
> Projects or Groups automatically. I can create a personal Herald rule.
> However, the Herald actions for associating objects with Projects are not
> available to me. I /think/ they are only available in the "Global" rules,
> which seem to only be available to admins.
> 
> I think Projects/Groups and Herald rules to automatically associate objects
> with Projects/Groups is a killer feature. However, the feature is
> significantly undermined by having to gate through site admins to make
> Herald rules. People don't know this feature is available to them because it
> is invisible. And, when they want to change things, they have to go through
> a formal process as opposed to clicking through the web UI. It would be rad
> if regular users could create Herald rules for Projects/Groups [that they
> are a member of].

To solve this I have created a new group (project) called Herald Admins that will allow admins to add any members we deem appropriate to it. Those members will have the ability to add/remove/edit global Herald rules and actions. The Herald application can be configured like any other object to have edit rights so I made it where Admins and Herald Admins could manage global rules.

As of right now I have added myself and gps to the new group. We can add more in the future as needed. gps, please play around with it. You should now be able to add new global rules to commits, revisions, etc. With Revision objects you can file a rule if the repo matches or if there are specific files or paths in the diffs. This should help accomplish what you need to do.

dkl

Comment 4

[Gregory Szorc [:gps]]

OK.

I now grok that when you create a "Project," depending on the icon you choose, that "Project" can quack like a "Group," "Tag," "Organization," or about a dozen other concepts. But under the hood I /think/ they all get mapped to the same "Project" primitive and Phabricator treats them all more or less as the same type of entity.

I'm able to create global Herald rules now. That allows me to create rules that associate any differential revision with a Project or Group, add a Group reviewer, etc. Pretty cool.

https://phabricator.services.mozilla.com/H17 now adds the "Autoland" project and the "reviewers-autoland" group to changes to Autoland files in VCT. https://phabricator.services.mozilla.com/H18 does something similar for the Firefox build system. I /think/ if you go to https://phabricator.services.mozilla.com/tag/reviewers-firefox-build-system/ and click the "Watch" button in the top right, you will be notified if a review is assigned to that group.

FWIW, I read that "global" herald rules are bad for performance. And they may have security implications. And they currently require special permission to create. So it seems we have multiple scaling problems here.

Taking a step back, I wonder if we should handle things differently.

When you create Differential revisions (under the hood I believe it is calling the "differential.revision.edit" API - https://phabricator.services.mozilla.com/conduit/method/differential.revision.edit/), you can specify reviewers, projects, and subscribers. Herald rules allow you to do this as well. I was thinking Herald rules might be the easiest way to automatically associate paths to projects. But given the limitations of Global Herald rules, I'm now thinking it might be best to perform this association on the client at Differential revision creation/update time. The trick here is we'd need a mechanism to map files in the repo to Differential actions. And we'd need the client to query that metadata and add it to the Differential API call. That sounds like bug 1137042 and bug 1366401. We'd also probably want a more well-defined mechanism for maintaining projects/groups in Phabricator. If the Modules data were machine readable, we could automatically synchronize that to Phabricator groups. Lots of options here.

Comment 5

[David Lawrence [:dkl]]

I have set up Herald Admins project on https://phabricator-dev.allizom.org that can be used for testing these types of things in the future. I probably should have done that before when you were experimenting anyway. I just need gps to log in to that system and then I can add him to the admins group. phab-dev authenticates against bugzilla-dev.allizom.org so as long as you have an account there it should work.

dkl

Comment 6

[Gregory Szorc [:gps]]

I created my gps account on phabricator-dev.

Comment 7

[David Lawrence [:dkl]]

(In reply to Gregory Szorc [:gps] from comment #6)
> I created my gps account on phabricator-dev.

Added you to the Herald Admins group.