Edgewall Software

Opened 17 years ago

Closed 9 years ago

Last modified 4 years ago

#4056 closed enhancement (fixed)

Notification Preferences — at Version 36

Reported by: jeff.wise@… Owned by: Peter Suter
Priority: normal Milestone: 1.1.3
Component: notification Version: 0.10
Severity: normal Keywords: notification email
Cc: Jun Omae Branch:
Release Notes:

Added a Notification preferences panel.

API Changes:

Added new extension point INotificationSubscriber.

Internal Changes:

Description

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.

Change History (36)

comment:1 by sid, 17 years ago

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

comment:2 by Christian Boos, 17 years ago

Milestone: 1.0

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

comment:3 by Noah Kantrowitz (coderanger) <coderanger@…>, 17 years ago

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

comment:4 by sid, 17 years ago

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.

comment:5 by Alec Thomas, 17 years ago

Owner: changed from Jonas Borgström to Alec Thomas

comment:6 by sid, 17 years ago

#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.

comment:7 by Matthew Good, 17 years ago

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

comment:8 by Emmanuel Blot, 17 years ago

Keywords: notification email added

comment:9 by Andy Schlaikjer <hazen@…>, 17 years ago

Cc: hazen@… added

comment:10 by sid, 17 years ago

#1640 was marked as duplicate of this ticket.

comment:11 by jeff.wise@…, 17 years ago

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.

comment:12 by Emmanuel Blot, 17 years ago

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.

in reply to:  12 comment:13 by jeff.wise@…, 17 years ago

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.

comment:14 by hazen@…, 17 years ago

Cc: hazen@… removed

comment:15 by ThurnerRupert, 17 years ago

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).

comment:16 by ThurnerRupert, 17 years ago

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

comment:17 by Christian Boos, 17 years ago

Component: ticket systemnotification
Milestone: 1.00.12

See also #2625 (preferred format)

in reply to:  12 comment:18 by Pedro Algarvio, aka, s0undt3ch <ufs@…>, 16 years ago

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.

comment:19 by Carsten Fuchs <CarstenFuchs@…>, 14 years ago

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").

comment:21 by Peter Suter, 10 years ago

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)

comment:22 by Peter Suter, 10 years ago

Milestone: next-major-releases1.1.2
Owner: changed from Alec Thomas to Peter Suter
Status: newassigned

comment:23 by Jun Omae, 10 years ago

Cc: Jun Omae added

comment:24 by Jun Omae, 10 years ago

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.

comment:25 by Peter Suter, 10 years ago

Excellent, thanks!

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

comment:26 by Jun Omae, 10 years ago

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

comment:27 by Peter Suter, 10 years ago

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

comment:28 by Jun Omae, 10 years ago

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

comment:29 by Peter Suter, 10 years ago

  • 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?)

in reply to:  29 comment:30 by Jun Omae, 10 years ago

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.

in reply to:  24 ; comment:31 by Peter Suter, 10 years ago

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?

in reply to:  31 comment:32 by Jun Omae, 10 years ago

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?

comment:33 by Ryan J Ollos, 10 years ago

Milestone: 1.1.21.1.3

comment:34 by Peter Suter, 9 years ago

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.

comment:35 by Peter Suter, 9 years ago

comment:36 by Peter Suter, 9 years ago

API Changes: modified (diff)
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to trunk in [13469].

Note: See TracTickets for help on using tickets.