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

[Jun Omae]

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.

[Peter Suter]

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

[Jun Omae]

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.

[Jun Omae]

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.

[Jun Omae]

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.

[Peter Suter]

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

[Jun Omae]

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.

[Peter Suter]

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

[Jun Omae]

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