1474962 - No obvious way to tell which mails from Phabricator are "you have been granted review"

Status: NEW

(Conduit :: Phabricator, enhancement, P5)

Description

[Boris Zbarsky [:bzbarsky]]

I have several emails in my inbox with "[Differential] [Accepted]" in the subject.  Some are people reviewing my patches.  Some are people reviewing other people's patches but I got email anyway.

What I don't see is any way to track the things I have gotten review on, which are the ones that need action from me.  Ideally this would include the ones that were r-.

For reference, with Bugzilla I just query my bugmails for "granted|canceled" in the subject and that gives me a todo list of all of my requests where the requestee responded...

Comment 1

[Steven MacLeod [:smacleod]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #0)
> For reference, with Bugzilla I just query my bugmails for "granted|canceled"
> in the subject and that gives me a todo list of all of my requests where the
> requestee responded...

Phabricator includes a ton of information in headers (and optionally the body as "Stamps"[1]) to allow mail filtering. Here are some examples:

> X-Phabricator-Sent-This-Message: Yes
> X-Phabricator-Mail-Tags: <differential-other>, <differential-reviewers>, <differential-review-request>
> X-Herald-Rules: <28>, <29>, <34>, none, <31>
> X-Phabricator-To: <PHID-USER-pkfkjpmwtukzgjv7yoml>
> X-Phabricator-To: <PHID-USER-trei6k7kwz6to4zny7w6>
> X-Phabricator-Cc: <PHID-USER-uedtrmyeszdpe6r555kg>
> X-Phabricator-Mail-ID: 56944
> X-Phabricator-Send-Attempt: pafvqqfbio6qztj7
> X-Phabricator-Stamps: actor(@smacleod) application(Differential) author(@smacleod) herald(H28) herald(H29) herald(H31) herald(H34) monogram(D2089) object-type(DREV) phid(PHID-DREV-to7owvx2pheqg3tyoxbm) reviewer(@glob) revision-repository(rLANDOAPI) revision-status(needs-review) via(daemon)


And the correspond stamps[1] that show up in the body as well:

> actor(@smacleod) application(Differential) author(@smacleod) herald(H28) herald(H29) herald(H31) herald(H34) monogram(D2089) object-type(DREV) phid(PHID-DREV-to7owvx2pheqg3tyoxbm) reviewer(@glob) revision-repository(rLANDOAPI) revision-status(needs-review) via(daemon)

You could query for something like "author(@bz) AND revision-status(accepted)" as well as "reviewer(@bz) AND revision-status(needs-review)" to find what you'd like. Does this seem sufficient for your use case? If so I'm tempted to morph this bug to add this information to our documentation / enable in body stamps as default for our install...

[1]You can turn on "Stamps" for your Phabricator emails by going to "Settings" -> "Email Format" -> "Send Stamps" -> "Mail Headers and Body"

Comment 2

[Boris Zbarsky [:bzbarsky]]

I'm not trying to filter the mails.  I'm trying to search them, with my mail program's search.  It can usefully search things like from, to, subject, body, but not really headers.

I _could_ try to set up filtering so that I filter out all the phabricator mails that are review grants/denials as opposed to ones that are just comments on the patch.  But that fragments the discussion even worse than the phabricator/bugzilla split, and I need to address those comments anyway...

I'll try the stamps thing and see if it does what I want.  Thank you for that pointer.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Does this seem sufficient for your use case?

I don't think it is.  The stamps are the "current status", not the "changed status".  

What I am looking for is a simple way to get state-change emails that I can easily query for and use as a todo list.  The way Phabricator's stamps work, if a mail has "author(@bz) AND revision-status(accepted)" that doesn't mean "this is your review being granted"; it means "this review got granted at some point in the past (maybe as part of the action that triggered the mail)" and the stamp gets set on all mails after that point.

The problem is that there is a _huge_ amount of noise mail involved when people add comments, update revisions, etc.  Bugzilla has that too, but since it sends clearly identifiable separate mails when someone actually needs to take an action it's possible to separate those from the flood so they don't get lost.  I still haven't found a way to do that with Phabricator...

Comment 4

[Steven MacLeod [:smacleod]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #3)
> > Does this seem sufficient for your use case?
> 
> I don't think it is.  The stamps are the "current status", not the "changed
> status".

Maybe worth requesting upstream add stamps for the transitions as well...

In the mean time, Phabricator emails have a list of things that happened at the top of each email, I think you could get away with searching the bodies for specific phrases like:
- "accepted this revision."
- "This revision is now accepted and ready to land."
- "requested changes to this revision."
- "This revision now requires changes to proceed."

From what I can tell these only show up on the initial transition email. If this works for you, great, otherwise please let me know.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> - "accepted this revision."

As far as I can tell, this appears anytime anyone grants review to anyone else, not just when my review requests get granted.  Same thing for the other phrases; I get mails containing those as an "innocent bystander" (e.g. a reviewer) on a revision.

Comment 6

[Steven MacLeod [:smacleod]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #5)
> > - "accepted this revision."
> 
> As far as I can tell, this appears anytime anyone grants review to anyone
> else, not just when my review requests get granted.  Same thing for the
> other phrases; I get mails containing those as an "innocent bystander" (e.g.
> a reviewer) on a revision.

Sorry, should have been more clear that you'd want to combine this with the stamps. Something like "author(@bz) AND accepted this revision" etc.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ah, that might work... Lemme try that next time I ask for a review in phabricator.