[patch] Refactoring of general `NotifyEmail` class to make it easier to subclass
|Reported by:||Owned by:|
|Cc:||Thijs Triemstra, Ryan J Ollos||Branch:|
NotifyEmail class in
trac.notification is a great starting-point for any custom email classes.
I have made a simple class that does my special things in
send(), but where 98% of the logic it is left to
Notify classes to handle the practical things. Among other things I use my custom Email-sending class to send registration confirmations and similar system mail.
However, for any kind of reuse of this class there is one snag: Notification needs to be enabled for the project. So, if you want to send you own mail or have some other module that sends email there is no easy way to do this based on general classes without copy & paste of logic logic from original class (in notify() as it would need to be 'skipped' in the call-chain). My system email sending new passwords based on 'forgotten password' form should not depend on ticket notification being enabled. As it is now I have to trick the class in my notify() by enabling notification, calling
NotifyEmail.notify(), and then setting it back at the end… Not pretty, and potentially a source of application misbehavior.
So much for the argument of why. Here is a small patch that extracts the enable-check logic into a method that can be easily overridden for any subclass of
Index: trac/trac/notification.py =================================================================== --- trac/trac/notification.py (revision 5803) +++ trac/trac/notification.py (working copy) @@ -209,11 +209,14 @@ self._charset.output_charset = 'ascii' else: raise TracError, 'Invalid email encoding setting: %s' % pref + + def notification_enabled(self, resid): + return self.config.getbool('notification', 'smtp_enabled') def notify(self, resid, subject): self.subject = subject - if not self.config.getbool('notification', 'smtp_enabled'): + if not self.notification_enabled(resid): return self.smtp_server = self.config['notification'].get('smtp_server') self.smtp_port = self.config['notification'].getint('smtp_port')
It should not be able to break any existing code anywhere as far as I can deduct, and would be a great bonus for reuse of Trac code.
Change History (11)
comment:6 by , 9 years ago
|Summary:||[patch] Minor refactoring of general `NotifyEmail` class to make it easier to subclass → [patch] Refactoring of general `NotifyEmail` class to make it easier to subclass|