Add new setting

sorry :(

Are these ever useful? Maybe we could just hide them from all notifications instead? I think they're OK in the rooms themselves but they feel bad to me as "last transactions" in the menu and as mail/notifications in general.

The one counterexample I can come up with is that we had a user add another user to a private room which they shouldn't have been added to. That feels super weak to me as a justification and I'd still rather just always hide these from leaking out of the room, but it's at least some kind of argument.

We can hide these unconditionally by implementing ConpherenceTransaction->shouldHideForMail/shouldHideForFeed and having them always return true on this transaction type (PARTICIPANTS) regardless of who was added/removed.

Also, maybe this message behavior is better?

• Remove preference in Settings.
• Default is ALWAYS "notifications".
• Per-room, opt-in to email for every message.
• (In the future, add mention email as part of a greater set of improvements to mentions.)

The problem I'm trying to resolve is knowing I've been added to a new room, aka, new user sends me a message. I'd like to get some sort of push notification for that. Should that be Herald instead? With that, I'm cool just getting rid of Conpherence auto-sending emails by default.

Ah. I think you have to do this:

• When processing transactions (e.g., in applyFinalEffects()) compute added/removed users and save them on something like $this->addedParticipants / $this->removedParticipants (new properties on the Editor object itself).
• In getMailTo(), examine the properties you wrote (if they were written) to figure out what magic should happen.

Basically, you can't bring $xactions into getMailTo() directly, but you can pass information on the Editor object, since we do guarantee that the same Editor object is used for both calls.

Conceivably you could cheat and just do $this->xactions = $xactions; somewhere, but that's not very nice.

Seems like herald is the less hacky solution?

Do we do policy checks after getMailTo. I seem to be able to generate mail from being added, but not being removed, even with the PHID in the array to be sent mail.

Not sure which thing you were thinking about doing with Herald, but at least the specific "you were just added / you were just removed" stuff isn't currently a terribly clean fit for Herald, since most Herald stuff is based only on the state of the object (e.g., if you plug Z123 into the test console, it should ideally be able to run all the rules).

We can't really/easily figure out who was just added/removed with only a Z123. That said, there are some conditions which sort of violate this already (content source, originating inbound email address) and it's usually possible to do it, just feels a little hacky-ish and is hard to test since you can't pass a specific set of transactions to Herald.

That is, overall, Herald acts on the state of the object, not on which changes were made to the object. So "Title" is a Herald field, but "Title was edited" is not, today. We can make it a condition, but if we want users to be able to test it we need to change how the test console works so that you can say "Test what will happen to object X if I apply edits A, B, and C".

The "removed" thing has a related task somewhere -- when any user is removed from subscribers/reviewers/etc, we should really keep them on that final email. I think we'll need to add a mechanism that says "for this email only, you are exempt for permissions checks" since they can often no longer see the object once we do the checks. Let me see if I can dig that up.

T4776 is the general "users should be able to get notified when they get kicked off an object" task.

This is ready for review then.

I guess I can remove the removed code though if it will never work here

bumpypoo

I'm planning to implement T4776 for this.

I also don't want to add a new option -- what was your concern with these proposed behaviors that let us not have any settings?

• Always default to "notify" for messages.
  • Remove existing setting controlling this behavior.
• Always send email on added/removed.
  • No new option for this behavior.
  • After T10448, we'll probably get an option for this "for free", or nearly-for-free.

oh, that sounded like a many years from now kind of feature. ok.

I think it's pretty manageable to sneak in here somewhere.