Notification Preferences

[jeff.wise@…]

My team has been using Trac for 3 months now. Overall its been a great success. However, some team members complain that they receive lots of e-mail that doesn't matter to them. This eventually leads to their ignoring Trac e-mails. An example would be when they are placed on the CC list of a ticket and later someone changes one of the ticket properties. They aren't interested in ticket property changes.

I've searched, but I haven't found a mechanism to customize the notification preferences on a per user basis. There is global configuration in TracIni, but no means for a user to indicate the kind of notifications they are interested in receiving.

I would like for a user to indicate which notifications they would like to receive:

• All (The current behaviour)
• None
• Ticket Creation
• Addition of Comments/Replies
• All Ticket properties
• Owner changes
• Milestone changes
• etc.

Please consider adding this feature to Trac. Thanks.

[sid]

Similar to, but not quite the same as #2073.

[Christian Boos]

See also my note about user preferences settings.

Also, the level of details proposed here looks a bit to high for me, especially if in the future we have also notifications for wiki, milestone, changesets, etc.

So I'd propose a 3-levels setting, matching what we have in the timeline:

• All changes
• Only Status changes
• None

[Noah Kantrowitz (coderanger) <coderanger@…>]

If coupled with the TracObjects idea, maybe you could keep a list of (resource, field/facet) pairs to base notifications off.

[sid]

If you are cc'ed on a ticket, you will probably want to know more than just status changes. You probably want to be involved in the discussion as well, and notified of new comments. But possibly care less about ticket property changes.

[sid]

#3269 was marked as a duplicate. It contained a patch for the 0.9 branch that hardcoded cc rules to only cc on comment or description changes.

[Matthew Good]

#4166 and #3155 have been marked as duplicates of this ticket.

[sid]

#1640 was marked as duplicate of this ticket.

[jeff.wise@…]

I'd really like to see ticket notification preferences look something like the following. Bullets are like checkboxes.

Notify me when:

• I am the ticket reporter.
• I am the ticket owner.
• I am a ticket updater.
• I am on the ticket CC list.

Notify me of:

• New comments.
• Status changes.
• Resolution changes.
• Property changes.
  • List all, including custom, properties here.

[Emmanuel Blot]

I think we gonna face a potential issue here: if notification content are based on user preferences, this means that we'll need to emit one email per recipient - rather than one email for all recipients, as per today.

As notification are synchronous (i.e. the web request completes when all notifications have been sent to the SMTP server), how Trac will behave with numerous recipients ?

Does this mean we need an asynchronous service to actually send the notifications? This is something that should be thought about before starting any user-based content settings.

[jeff.wise@…]

Replying to eblot:

I think we gonna face a potential issue here: if notification content are based on user preferences, this means that we'll need to emit one email per recipient - rather than one email for all recipients, as per today.

I don't understand the limitation here. Possible recipients are the reporter, owner, updater, and CC list. Use their preferences and the ticket activity to filter the list down to those who care. Then send a single e-mail to the filtered list. Unless the CC list or the set of updaters is incredibly long, it shouldn't take a significant amount of time to filter the recipient list.

[ThurnerRupert]

this feature would be also very useful for mixed user populations, like "web users" (maybe prefer notification via mail) and eclipse mylar users (get notified by mylar).

[ThurnerRupert]

maybe prevent a notification to self like addressed in #2247 is also something to consider?

[Christian Boos]

See also #2625 (preferred format)

[Pedro Algarvio, aka, s0undt3ch <ufs@…>]

Replying to eblot:

I think we gonna face a potential issue here: if notification content are based on user preferences, this means that we'll need to emit one email per recipient - rather than one email for all recipients, as per today.

As notification are synchronous (i.e. the web request completes when all notifications have been sent to the SMTP server), how Trac will behave with numerous recipients ?

Does this mean we need an asynchronous service to actually send the notifications? This is something that should be thought about before starting any user-based content settings.

Something like the ​AnnouncerPlugin should be though of.

[Carsten Fuchs <CarstenFuchs@…>]

I too have problems with users that get too many or too little email notifications.

A suggestion: Why discriminate between reporter, owner and updater at all? Instead, add a checkbox

• Notify me when the ticket is changed (subscribe ticket)

to the "New Ticket" and "Modify Ticket" forms (keep the decision if this checkbox is checked by default as a per user setting), and a "Subscribe ticket (notify me when the ticket is changed)" link to the ticket view pages?

This would be a simple system that meets all requirements, analogous to how e.g. the phpBB forums notify users about replies.

While this replaces the current always_notify_[owner|reporter|updater] settings in trac.ini, the smtp_always_[cc|bcc] settings should be kept.

The only open question is how a user can automatically subscribe to new tickets (e.g. another per user checkbox setting: "Notify me when a new ticket is created").

[rcorsaro]

I've been working on this: ​http://trac.edgewall.org/wiki/TracDev/Proposals/Announcer#ProposedAnnouncerPreferences

I could use some feedback.

[Peter Suter]

The Extension API for subscriptions proposal adds a new DB table and a new extension point that allows plugins to implement different rules for subscribing to notifications.

A draft of the proposed changes can be found in log:psuter@advanced-notification-subscriptions (these depend on the proposed changes of comment:11:ticket:3517). These will have to be reviewed and discussed in some more detail. For example I listed some open questions in the relevant wiki pages.

A generic preferences dialog allows users to manage their subscriptions. (changeset:158e97e037b9/psuter, Screenshot​)

[Jun Omae]

I'm reviewing your branch. Also, my proposed changes can be found in ​https://bitbucket.org/jun66j5/trac.advanced_notification/commits/branch/trunk.

• subscription table: at least, we should add subscription (sid, authenticated) index for Subscription model.
  • Also, subscription (class) index if we would use Subscription.find_by_class.
• prefs_notification.html
  • We should add i18n:msg for i18n/l10n
  • In the page, message is made from adverb and description of subscriber, e.g. always notify me of any ticket changes. It's difficult to translate such a message.

[Peter Suter]

Excellent, thanks!

What does ​py:choose="a" do? (I only found it ​documented in combination with py:when.)

[Jun Omae]

Oh, that's my bad. I just removed it in ​e8d8123.

[Peter Suter]

I assume upgrades/db30.py would also need to be adjusted when adding a new index.

[Jun Omae]

I updated ​https://bitbucket.org/jun66j5/trac.advanced_notification/commits/branch/trunk.

• ​f37d03d subscription (sid, authenticated) index is added on upgrades/db30.py
• ​edc936a changed end-of-line of db30.py to LF
• ​c8b9105 imports only symbols in trac/notification.py for backward compatibility
• ​f6f6e24 defined __all__ in api.py, compat.py and mail.py
• ​5467802 fixed a wrong import statement for BatchTicketNotifyEmail
• ​3982f62 added DefaultDomainEmailResolver component which is already in [notification] email_address_resolvers
• ​ed06c0b fixed broken From header if smtp_from_name has unicode characters

Another issue: If adding first subscription rule, format column of created subscription would be empty. The issue isn't fixed until saving email format. I think email format should be stored in session.

sqlite> select * from subscription;
id          time              changetime        class                  sid         authenticated  distributor  format      priority    adverb
----------  ----------------  ----------------  ---------------------  ----------  -------------  -----------  ----------  ----------  ----------
1           1384869157624135  1384869157624135  TicketOwnerSubscriber  jun66j5     1              email                    1           always

[Peter Suter]

• ​3982f62 added DefaultDomainEmailResolver component which is already in [notification] email_address_resolvers

I initially had a DefaultDomainEmailResolver and dropped it again in changeset:2c5d1c0c2abb/psuter (but forgot to remove it from email_address_resolvers). I'm not against adding it back if it's useful. I couldn't find a good reason to keep it, as it seems not quite to do the same thing as before (resolve vs. recognize) and default domain handling is needed in RecipientMatcher anyway to get the same behavior as before.

Another issue: If adding first subscription rule, format column of created subscription would be empty. The issue isn't fixed until saving email format. I think email format should be stored in session.

Yes, storing the format per subscription doesn't seem that useful and has some downsides. Will storing it in the session work for other distributors? (Or maybe only email really needs multiple formats?)

[Jun Omae]

Replying to psuter:

• ​3982f62 added DefaultDomainEmailResolver component which is already in [notification] email_address_resolvers

I initially had a DefaultDomainEmailResolver and dropped it again in changeset:2c5d1c0c2abb/psuter (but forgot to remove it from email_address_resolvers). I'm not against adding it back if it's useful. I couldn't find a good reason to keep it, as it seems not quite to do the same thing as before (resolve vs. recognize) and default domain handling is needed in RecipientMatcher anyway to get the same behavior as before.

Oh, I missed that. I'll retry and remove DefaultDomainEmailResolver from default of email_address_resolvers.

Another issue: If adding first subscription rule, format column of created subscription would be empty. The issue isn't fixed until saving email format. I think email format should be stored in session.

Yes, storing the format per subscription doesn't seem that useful and has some downsides. Will storing it in the session work for other distributors? (Or maybe only email really needs multiple formats?)

Hmm, please ignore it. At least, empty format column doesn't lead notification.

If configuring subscription rules and the format even isn't matched, we should notify with the distributor and the other format. Otherwise, the user would neither receive any notification nor know the reason because the distributor and the format don't shown in subscription rules.

[Peter Suter]

Replying to jomae:

• subscription table: at least, we should add subscription (sid, authenticated) index for Subscription model.
  • Also, subscription (class) index if we would use Subscription.find_by_class.

Adding this index sounds like a good idea, so plugins can use find_by_class.

Do you think we should use a different name for the subscription table, to avoid a name clash with the Announcer plugin?

[Jun Omae]

Replying to psuter:

Do you think we should use a different name for the subscription table, to avoid a name clash with the Announcer plugin?

Sounds good. What is the name good? notification table?

[Peter Suter]

Updated in log:psuter@advanced-notification-subscriptions.2. (Still depends on #3517, and I folded in all of ​Jun's improvements for easier reviewing. Thanks again!)

What is the name good? notification table?

The proposal now uses notify_subscription. I hope this is an acceptable compromise.

[Peter Suter]

Updated in log:psuter@advanced-notification-subscriptions.4.

As requested the previous updaters are notified by TicketPreviousUpdatersSubscriber​.

[Peter Suter]

Committed to trunk in [13469].

[Christian Boos]

Anything noteworthy on this topic to add to 1.1/TracUpgrade? Looking towards upgrading t.e.o at some point.

[Peter Suter]

Replying to cboos:

Anything noteworthy on this topic to add to 1.1/TracUpgrade?

I'm not sure, there's not really anything that special so far. There's a new DB table and some new components, but nothing to upgrade manually I think, no new configuration options and the old ones should all still work the same as before.

[Jun Omae]

Hints in prefs_notification.html say;

Example: The subscription rule <strong>"Notify: Any ticket changes"</strong> overrides …

However, no components which INotificationSubscriber is implemented have Any ticket changes as subscriber's description.

[Peter Suter]

Replying to jomae:

Hints in prefs_notification.html say;

Example: The subscription rule <strong>"Notify: Any ticket changes"</strong> overrides …

However, no components which INotificationSubscriber is implemented have Any ticket changes as subscriber's description.

Right, it makes more sense with AllTicketSubscriber​ (Any ticket is modified) proposed in #11870.

I left it because the example still seems more helpful for understanding than the alternatives.

Also any static example could have that problem, depending on the configuration. Creating the example dynamically seems excessive though. Maybe it would be better to remove the example here and instead describe it on TracNotification?

[Ryan J Ollos]

Minor defect in [13469#file13] fixed in [13600].

[Jun Omae]

In [13673], fixed missing id attribute.

[Ryan J Ollos]

Subscription rules select box is empty for anonymous user. Is that intended? If so, should we instead hide some or all of the Notifications preference panel for anonymous user?

[Peter Suter]

Replying to rjollos:

Subscription rules select box is empty for anonymous user. Is that intended? If so, should we instead hide some or all of the Notifications preference panel for anonymous user?

It is ​intended that some subscriptions rules are not available to anonymous users. This includes all subscription rules available to authenticated users by default. I agree it would probably make sense to hide that UI if no rules are available to the current user.

[Ryan J Ollos]

Thanks, I created ticket #11990.

[Ryan J Ollos]

I posted a proposed fix to #11990 and I'm considering to commit it after adding tests. Could you take a look and let me know if it looks right to you? Thanks!

[Ryan J Ollos]

log:rjollos.git:t4056_inotificationsubscriber_refactoring proposes a refactoring to simplify the implementation of INotificationSubscribers and ensure batchmodify events behave like ticket notification events unless explicitly overridden in the INotificationSubscriber. All tests pass after the change.

[Ryan J Ollos]

comment:47 changes committed to trunk in [14328].

[Ryan J Ollos]

The Subscription and Watch classes have a lot of classmethods and don't fit the typical class design pattern in Trac. Is there a design reason for that? Or was that code just carried over from TracAnnouncer?

The overhead in instantiating an object must be trivially small for our use-cases.

trac/ticket/notification.py

@@ -661 +661 @@
             return
 
         klass = self.__class__.__name__
-        for s in Subscription.find_by_class(self.env, klass):
+        for s in Subscription(self.env).find_by_class(klass):
             yield s.subscription_tuple()
 
     def description(self):
@@ -773 +773 @@
         if sid:
             sids.add((sid, auth))
 
-    for s in Subscription.find_by_sids_and_class(subscriber.env, sids, klass):
+    for s in Subscription(subscriber.env).find_by_sids_and_class(sids, klass):
         yield s.subscription_tuple()

However, even if the latter is a better or more Trac-typical class design, it would be an API change to modify this now.

[Peter Suter]

The Subscription and Watch classes have a lot of classmethods and don't fit the typical class design pattern in Trac. Is there a design reason for that? Or was that code just carried over from TracAnnouncer?

​Right, it was just carried over. (Watch was renamed from SubscriptionAttribute if I remember correctly.)

I don't know if this design (a mix of repository and static factory method design patterns?) was originally chosen for performance or other reasons.

Note that the classes do get instantiated​ for each found DB record, and each such object contains the fields of one DB record. Creating (and throwing away) an "empty" object first could be considered strange. Other Trac models actually do also use class methods for batch operations like *.select​ or Attachment.delete_all. Subscription and Watch are mostly used for batch operations. I think the only non-batch operation is add. (Which would arguably be nicer as a normal method and is a classmethod only for consistency I guess)

[Ryan J Ollos]

Replying to Peter Suter:

Note that the classes do get instantiated​ for each found DB record, and each such object contains the fields of one DB record. Creating (and throwing away) an "empty" object first could be considered strange. Other Trac models actually do also use class methods for batch operations like *.select​ or Attachment.delete_all. Subscription and Watch are mostly used for batch operations. I think the only non-batch operation is add. (Which would arguably be nicer as a normal method and is a classmethod only for consistency I guess)

Yeah, that makes sense. Thanks!