Edgewall Software
Modify

Opened 5 years ago

Closed 5 years ago

#11934 closed defect (fixed)

smtp_always_{cc,bcc} options don't work when no subscribers

Reported by: Jun Omae Owned by: Jun Omae
Priority: normal Milestone: 1.1.4
Component: notification Version: 1.1.3
Severity: normal Keywords:
Cc: Branch:
Release Notes:
  • Fix not working smtp_always_cc and smtp_always_bcc options in [notification] section.
  • Fix not working smtp_from_author option in [notification] section.
API Changes:

Description

Since 1.1.3, smtp_always_cc and smtp_always_bcc are implemented as IEmailDecorator in new notification system. It works if notification is subscribed by someone. However, EmailDistributor don't call decorators if no subscribers. Then, the options don't work.

Attachments (0)

Change History (9)

comment:1 by Jun Omae, 5 years ago

Summary: smtp_always_{cc,bcc} options don't work when disabling always_notify_{owner,reporter,updater}smtp_always_{cc,bcc} options don't work when no subscribers

comment:2 by Peter Suter, 5 years ago

Hm, good point. See also th:ticket:10676.

comment:3 by Jun Omae, 5 years ago

Owner: set to Jun Omae
Status: newassigned

Proposed changes in jomae.git@t11934, which implements those options as a not-configurable subscriber instead of a decorator.

I'm trying to replace uses of TicketNotifyEmail with NotificationSystem.notify(event). It seems that smtp_from_author and ignore_domains options are broken.

comment:4 by Jun Omae, 5 years ago

Another issue: in 1.1.3, even if both text/plain and text/html are enabled, smtp_always_{cc,bcc} options adds its addresses to Cc and Bcc headers to both notifications. As the result, the recipients which are listed in the options will receive two mails.

In the changes, the recipients would subscribe only text/plain format. See jomae.git/trac/notification/mail.py@t11934:530-538#L519.

I think it would be good to configure the default format. E.g. [notification] default_email_format = text/html.

Last edited 5 years ago by Jun Omae (previous) (diff)

comment:5 by Jun Omae, 5 years ago

Updated jomae.git@t11934 branch.

  • [f310a135f/jomae.git] fixed [notification] smtp_from_author option
  • [28513ae8/jomae.git] uses NotificationSystem.notify in unit tests
    • test_previous_cc_list and test_previous_owner still use TicketNotifyEmail

I misunderstood about ignore_domains option. The option works fine.

comment:6 by Peter Suter, 5 years ago

Oh, I forgot about smtp_from_author. It was on my list, but got lost somehow.

All your changes look good. Possibly smtp_from_author handling should go in EmailDistributor or a separate IEmailDecorator in trac.notification.mail, as it is not specific to just tickets, right?

I think it would be good to configure the default format. E.g. [notification] default_email_format = text/html.

What do you think about #11875? That would suggest something like:

[notification-subscriber]
always_cc = AlwaysEmailSubscriber
always_cc.format = text/html

in reply to:  6 ; comment:7 by Jun Omae, 5 years ago

Replying to psuter:

All your changes look good. Possibly smtp_from_author handling should go in EmailDistributor or a separate IEmailDecorator in trac.notification.mail, as it is not specific to just tickets, right?

Indeed. The option should work for all realms. Added FromAuthorEmailDecorator in [6099de83/jomae.git]. If no other problems in the changes, I'll push later.

I think it would be good to configure the default format. E.g. [notification] default_email_format = text/html.

What do you think about #11875? That would suggest something like:

[notification-subscriber]
always_cc = AlwaysEmailSubscriber
always_cc.format = text/html

Interested.

  1. Related to #11928, I am not sure whether a user wants to configure format per subscriber. It might be enough to configure format per transport like this.
    • default format per user: req.session['notification.default_format.email'] = 'text/html'
    • default format for all users: default_format.email = text/html in trac.ini.
  2. Unlike options in trac.ini, no way to known easily subscriber class's name. If the section is introduced, I think we should provide subscribers wiki page like TracIni and/or subscribers manager panel in admin page.

in reply to:  7 comment:8 by Peter Suter, 5 years ago

Replying to jomae:

If no other problems in the changes, I'll push later.

No other problems from my side. Please go ahead, thanks.

(I replied in #11928 and #11875 to the rest.)

comment:9 by Jun Omae, 5 years ago

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

Thanks for the reviewing! Committed in [13701-13703].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Jun Omae 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.