make notification email more configurable + add tests on sender name build process

tracim/development.ini.base

@@ -183,7 +183,10 @@ website.base_url = http://127.0.0.1:8080
 website.server_name = 127.0.0.1
 
 email.notification.activated = False
-email.notification.from.email = noreply@trac.im
+# email notifications can be sent with the user_id added as an identifier
+# this way email clients like Thunderbird will be able to distinguish
+# notifications generated by a user or another one
+email.notification.from.email = noreply+{user_id}@trac.im
 email.notification.from.default_label = Tracim Notifications
 email.notification.content_update.template.html = ./tracim/templates/mail/content_update_body_html.mak
 email.notification.content_update.template.text = ./tracim/templates/mail/content_update_body_text.mak

tracim/tracim/lib/notifications.py

@@ -168,6 +168,7 @@ class EST(object):
             cls.WORKSPACE_LABEL
         ]
 
+
 class EmailNotifier(object):
 
     """
@@ -188,26 +189,25 @@ class EmailNotifier(object):
         :param user: user to extract display name
         :return: sender string
         """
-        if user is None:
-            return '{0} <{1}>'.format(
-                self._global_config.EMAIL_NOTIFICATION_FROM_DEFAULT_LABEL,
-                self._global_config.EMAIL_NOTIFICATION_FROM_EMAIL,
-            )
 
-        # We add a suffix to email to prevent client like Thunderbird to
-        # display personal adressbook label.
-        email = self._global_config.EMAIL_NOTIFICATION_FROM_EMAIL
-        email_name, domain = email.split('@')
-        arranged_email = '{0}+{1}@{2}'.format(
-            email_name,
-            str(user.user_id),
-            domain,
-        )
-
-        return '{0} {1} <{2}>'.format(
-            Header(user.display_name).encode(),
-            'via Tracim',
-            arranged_email,
+        email_template = self._global_config.EMAIL_NOTIFICATION_FROM_EMAIL
+        mail_sender_name = self._global_config.EMAIL_NOTIFICATION_FROM_DEFAULT_LABEL  # nopep8
+        if user:
+            mail_sender_name = '{name} via Tracim'.format(name=user.display_name)
+            email_address = email_template.replace('{user_id}', str(user.user_id))
+            # INFO - D.A. - 2017-08-04
+            # We use email_template.replace() instead of .format() because this
+            # method is more robust to errors in config file.
+            #
+            # For example, if the email is info+{userid}@tracim.fr
+            # email.format(user_id='bob') will raise an exception
+            # email.replace('{user_id}', 'bob') will just ignore {userid}
+        else:
+            email_address = email_template.replace('{user_id}', '0')
+
+        return '{label} <{email_address}>'.format(
+            label = Header(mail_sender_name).encode(),
+            email_address = email_address
         )
 
     def notify_content_update(self, event_actor_id: int, event_content_id: int):

tracim/tracim/tests/library/test_notification.py

@@ -10,6 +10,7 @@ import transaction
 
 from tracim.config.app_cfg import CFG
 from tracim.lib.notifications import DummyNotifier
+from tracim.lib.notifications import EmailNotifier
 from tracim.lib.notifications import EST
 from tracim.lib.notifications import NotifierFactory
 from tracim.lib.notifications import RealNotifier
@@ -47,3 +48,51 @@ class TestDummyNotifier(TestStandard):
         ok_('{workspace_label}' in tags)
         ok_('{content_label}' in tags)
         ok_('{content_status_label}' in tags)
+
+
+class TestEmailNotifier(TestStandard):
+    def test_email_notifier__build_name_with_user_id(self):
+        u = User()
+        u.user_id = 3
+        u.display_name = 'François Michâlié'
+
+        config = CFG.get_instance()
+        config.EMAIL_NOTIFICATION_FROM_EMAIL = 'noreply+{user_id}@tracim.io'
+
+        notifier = EmailNotifier(smtp_config=None, global_config=config)
+        email = notifier._get_sender(user=u)
+        eq_('=?utf-8?q?Fran=C3=A7ois_Mich=C3=A2li=C3=A9_via_Tracim?= <noreply+3@tracim.io>', email)  # nopep8
+
+    def test_email_notifier__build_name_without_user_id(self):
+        u = User()
+        u.user_id = 3
+        u.display_name = 'François Michâlié'
+
+        config = CFG.get_instance()
+        config.EMAIL_NOTIFICATION_FROM_EMAIL = 'noreply@tracim.io'
+
+        notifier = EmailNotifier(smtp_config=None, global_config=config)
+        email = notifier._get_sender(user=u)
+        eq_('=?utf-8?q?Fran=C3=A7ois_Mich=C3=A2li=C3=A9_via_Tracim?= <noreply@tracim.io>', email)  # nopep8
+
+    def test_email_notifier__build_name_with_user_id_wrong_syntax(self):
+        u = User()
+        u.user_id = 3
+        u.display_name = 'François Michâlié'
+
+        config = CFG.get_instance()
+        config.EMAIL_NOTIFICATION_FROM_EMAIL = 'noreply+{userid}@tracim.io'
+
+        notifier = EmailNotifier(smtp_config=None, global_config=config)
+        email = notifier._get_sender(user=u)
+        eq_('=?utf-8?q?Fran=C3=A7ois_Mich=C3=A2li=C3=A9_via_Tracim?= <noreply+{userid}@tracim.io>', email)  # nopep8
+
+    def test_email_notifier__build_name_with_no_user(self):
+        config = CFG.get_instance()
+        config.EMAIL_NOTIFICATION_FROM_DEFAULT_LABEL = 'Robot'
+        config.EMAIL_NOTIFICATION_FROM_EMAIL = 'noreply@tracim.io'
+
+        notifier = EmailNotifier(smtp_config=None, global_config=config)
+        email = notifier._get_sender()
+        eq_('Robot <noreply@tracim.io>', email)
+