Page MenuHomePhabricator

When users are subscribed explicitly, count them as "mentioned"
Changes PlannedPublic

Authored by epriestley on Feb 8 2018, 8:37 PM.

Details

Summary

Ref T9031. This isn't a complete fix, but makes mail stamps work better for subscription changes. You can now route these on the client, and after T13069 resolves you should be able to route them on the server.

Basically, these three changes are all a "mention" of @dog:

alice added a comment.
@dog, can you look at this?
alice added a subscriber: dog.
Dog, can you take a look at this?
alice added a subscriber: dog.

The last one is a little flimsy, but I think the implied intent is always @dog, heads up on this.

Test Plan

Submitted variations of the comments above, saw an appropriate mention mail stamp turn up on the resulting mail.

Diff Detail

Repository
rP Phabricator
Branch
mention1
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 19437
Build 26290: Run Core Tests
Build 26289: arc lint + arc unit

Event Timeline

One case where I think this is ambiguous is when you're creating an object and add initial subscribers. Do those count as mentions?

I'm inclined to say that they prrrrrobably don't? They seem much less strong than even alice added a subscriber: bailey. as a standalone action.

Also, I think this should only count when a human user adds you as a subscriber. Herald rules adding you as a subscriber shouldn't be mentions.

(I don't see any "added subscriber: avivey" transaction, but I'm subscribed)

update: oh. that's what you're talking about.

I added you as a subscriber in the "Subscribers:" field in the commit message. Long ago we had some variation of this:

epriestley created this revision.
epriestley added reviewers: ...
epriestley added subscribers: ...
epriestley added project: ...

...but we fold all that state into the "creation" transaction now, unless the object had a non-empty default setting which was changed, or in about 500 special cases which don't use this rule.

That kinda makes sense, and kinda confusing. Maybe we should show this information somehow - under view details for the creation, or just consider reviewers/subscribers/project as special cases.

Anyway, I πŸ‘ this revision.

Are there cases of "added subscriber" that don't/shouldn't count as mentions?

The only ones I can come up with offhand are:

  • Object creation: I could go either way on this. I lean toward "not a mention".
  • Herald: definitely shouldn't be a mention.
  • Uhh.. merged tasks, I guess? That's probably not a mention either.

I'll exclude Herald at a bare minimum and grep for TYPE_SUBSCRIBERS and see if I can find anything I'm forgetting.