Edgewall Software
Modify

Opened 12 years ago

Closed 4 years ago

Last modified 3 years ago

#4056 closed enhancement (fixed)

Notification Preferences

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
Release Notes:

Added a Notification preferences panel.

API Changes:

Added new extension point INotificationSubscriber.

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.

Attachments (1)

SubscriptionRulesAsAnonymous.png (25.0 KB ) - added by Ryan J Ollos 4 years ago.

Download all attachments as: .zip

Change History (49)

comment:1 Changed 12 years ago by sid

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

comment:2 Changed 12 years ago by Christian Boos

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 Changed 12 years ago by 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.

comment:4 Changed 12 years ago by 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.

comment:5 Changed 12 years ago by Alec Thomas

Owner: changed from Jonas Borgström to Alec Thomas

comment:6 Changed 12 years ago by 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.

comment:7 Changed 12 years ago by Matthew Good

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

comment:8 Changed 12 years ago by Emmanuel Blot

Keywords: notification email added

comment:9 Changed 12 years ago by Andy Schlaikjer <hazen@…>

Cc: hazen@… added

comment:10 Changed 12 years ago by sid

#1640 was marked as duplicate of this ticket.

comment:11 Changed 12 years ago by 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.

comment:12 Changed 12 years ago by 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.

comment:13 in reply to:  12 Changed 12 years ago by 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.

comment:14 Changed 12 years ago by hazen@…

Cc: hazen@… removed

comment:15 Changed 12 years ago by 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).

comment:16 Changed 12 years ago by ThurnerRupert

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

comment:17 Changed 12 years ago by Christian Boos

Component: ticket systemnotification
Milestone: 1.00.12

See also #2625 (preferred format)

comment:18 in reply to:  12 Changed 10 years ago by 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.

comment:19 Changed 9 years ago by 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").

comment:20 Changed 8 years ago by rcorsaro

comment:21 Changed 5 years ago by 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)

comment:22 Changed 5 years ago by Peter Suter

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

comment:23 Changed 5 years ago by Jun Omae

Cc: Jun Omae added

comment:24 Changed 5 years ago by 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.

comment:25 Changed 5 years ago by Peter Suter

Excellent, thanks!

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

comment:26 Changed 5 years ago by Jun Omae

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

comment:27 Changed 5 years ago by Peter Suter

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

comment:28 Changed 5 years ago by 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

comment:29 Changed 5 years ago by 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?)

comment:30 in reply to:  29 Changed 5 years ago by 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.

comment:31 in reply to:  24 ; Changed 5 years ago by 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?

comment:32 in reply to:  31 Changed 5 years ago by 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?

comment:33 Changed 5 years ago by Ryan J Ollos

Milestone: 1.1.21.1.3

comment:34 Changed 4 years ago by 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.

comment:35 Changed 4 years ago by Peter Suter

comment:36 Changed 4 years ago by Peter Suter

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

Committed to trunk in [13469].

comment:37 Changed 4 years ago by Christian Boos

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

comment:38 in reply to:  37 Changed 4 years ago by 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.

comment:39 Changed 4 years ago by 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.

comment:40 in reply to:  39 Changed 4 years ago by 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?

Last edited 4 years ago by Peter Suter (previous) (diff)

comment:41 Changed 4 years ago by Ryan J Ollos

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

comment:42 Changed 4 years ago by Jun Omae

In [13673], fixed missing id attribute.

Changed 4 years ago by Ryan J Ollos

comment:43 Changed 4 years ago by 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?

comment:44 in reply to:  43 Changed 4 years ago by 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.

comment:45 Changed 4 years ago by Ryan J Ollos

Thanks, I created ticket #11990.

comment:46 Changed 3 years ago by 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!

comment:47 Changed 3 years ago by 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.

comment:48 Changed 3 years ago by Ryan J Ollos

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Peter Suter.
The resolution will be deleted.
to The owner will be changed from Peter Suter to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.