Page MenuHomePhabricator

Differental does not trigger Herald for mundane updates
Closed, ResolvedPublic

Description

After seeing that T7731 had landed, I was hopeful that I could set my email settings for Differential to "Notify" and then set up a Herald rule to email me only about updates to revisions for which I was specifically listed as a reviewer. (This seemed like it might be a possible alternative to T9535.)

To that end, I switched all of my Differential email preferences to Notify and created a personal Herald rule for Differential revisions that would fire if I was in the list of reviewers and that had an action to send me an email.

Using the test console, I verified that the rule would match as expected on a particular revision that had me as an explicit reviewer. Then, I added a comment to that revision. Since I configured my email preferences to notify on self-actions, I expected that the Herald rule would trigger and I would get an email.

After checking /mail/, however, I saw that the delivery of the message was voided to due Mail Tags: "This mail has tags which control which users receive it, and this recipient has not elected to receive mail with any of the tags on this message (Settings > Email Preferences)."

Based on T7731, I would have expected my Herald rule to punch through the mail tags for this message to make sure that it was delivered, but I might be misunderstanding (or T7731 might have been reverted).

Event Timeline

Did your Herald rule actually fire? I don't see a step like "I looked at the Herald transcript for the revision and saw that it had fired, matched, and claimed to have forced delivery." in your narrative. That would be the simplest explanation.

Your expectations are otherwise consistent with my expectations.

Oh, I think this is because Differential doesn't fire Herald for comments.

If you update the revision (which triggers Herald), the rules should work as you expect. Let me verify that...

Here's what I did locally. I wrote a rule to notify me about "dog" revisions:

Screen Shot 2016-01-09 at 7.24.20 AM.png (1×1 px, 152 KB)

I set all my preferences to "notify":

Screen Shot 2016-01-09 at 7.24.54 AM.png (228×738 px, 40 KB)

I updated (i.e., attached a new diff to) two revisions, one a "dog revision" and one a "cat revision".

The "cat revision" downgraded to a notification:

Screen Shot 2016-01-09 at 7.29.08 AM.png (839×1 px, 147 KB)

The "dog revision" hit the rule and sent me a forced email:

Screen Shot 2016-01-09 at 7.29.13 AM.png (839×1 px, 157 KB)

eadler added a project: Restricted Project.Jan 11 2016, 9:38 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jan 11 2016, 9:42 PM
epriestley renamed this task from "Send me an email" personal Herald rules do not punch through settings to Differental does not trigger Herald for mundane updates.Jan 20 2016, 3:09 PM

Thanks, @epriestley (and sorry for getting back to you so late)! You're totally right that this is because I didn't actually update the revision.

(This is definitely confusing, but I'm not sure what the best way forward is offhand. I'm hesitant to just make Herald always trigger without putting more thought into the issue.)

eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 4 2016, 9:16 PM

I'm curious, is there any other way to trigger Herald to process a Differential review *other* than uploading a new diff?

From https://discourse.phabricator-community.org/t/644 - it appears that HM now triggers a build on revisions when they are closed via landing, which fails because the build expects a Staging ref. I don't have the setup to test any of it, but the latest revision landed here doesn't show the behavior, so either f7f3dd5b2084 hasn't rolled in here yet, or that user is doing something funny.

Ah, thanks. I think your guess is right. I'll make a note in T2543, should be a one-line fix I think.