Browse Source

Fix website_send_to: prefer using name instead of website_from

Son NK 5 years ago
parent
commit
6c68b3cda7
4 changed files with 79 additions and 18 deletions
  1. 0 1
      app/email_utils.py
  2. 30 11
      app/models.py
  3. 18 5
      tests/test_email_utils.py
  4. 31 1
      tests/test_models.py

+ 0 - 1
app/email_utils.py

@@ -447,4 +447,3 @@ def parseaddr_unicode(addr) -> (str, str):
             return decoded_string.decode(charset), email
         else:
             return decoded_string, email
-

+ 30 - 11
app/models.py

@@ -728,7 +728,9 @@ class Contact(db.Model, ModelMixin):
     user_id = db.Column(db.ForeignKey(User.id, ondelete="cascade"), nullable=False)
     alias_id = db.Column(db.ForeignKey(Alias.id, ondelete="cascade"), nullable=False)
 
-    name = db.Column(db.String(512), nullable=True, default=None, server_default=text("NULL"))
+    name = db.Column(
+        db.String(512), nullable=True, default=None, server_default=text("NULL")
+    )
 
     # used to be envelope header, should be mail header from instead
     website_email = db.Column(db.String(512), nullable=False)
@@ -750,21 +752,38 @@ class Contact(db.Model, ModelMixin):
 
     def website_send_to(self):
         """return the email address with name.
-        to use when user wants to send an email from the alias"""
+        to use when user wants to send an email from the alias
+        Return
+        "First Last | email at example.com" <ra+random_string@SL>
+        """
 
-        name = self.website_email.replace("@", " at ")
+        # Prefer using contact name if possible
+        name = self.name
 
-        if self.website_from:
-            website_name, _ = parseaddr(self.website_from)
+        # if no name, try to parse it from website_from
+        if not name and self.website_from:
+            try:
+                from app.email_utils import parseaddr_unicode
 
-            if website_name:
-                # remove all double quote
-                website_name = website_name.replace('"', "")
-                name = website_name + " | " + name
+                name, _ = parseaddr_unicode(self.website_from)
+            except Exception:
+                # Skip if website_from is wrongly formatted
+                LOG.warning(
+                    "Cannot parse contact %s website_from %s", self, self.website_from
+                )
+                name = ""
+
+        # remove all double quote
+        if name:
+            name = name.replace('"', "")
+
+        if name:
+            name = name + " | " + self.website_email.replace("@", " at ")
+        else:
+            name = self.website_email.replace("@", " at ")
 
-        return f'"{name}" <{self.reply_email}>'
         # cannot use formataddr here as this field is for email client, not for MTA
-        # return formataddr((self.website_email.replace("@", " at "), self.reply_email))
+        return f'"{name}" <{self.reply_email}>'
 
     def last_reply(self) -> "EmailLog":
         """return the most recent reply"""

+ 18 - 5
tests/test_email_utils.py

@@ -6,7 +6,8 @@ from app.email_utils import (
     can_be_used_as_personal_email,
     delete_header,
     add_or_replace_header,
-    parseaddr_unicode)
+    parseaddr_unicode,
+)
 from app.extensions import db
 from app.models import User, CustomDomain
 
@@ -65,13 +66,25 @@ def test_add_or_replace_header():
 
 def test_parseaddr_unicode():
     # ascii address
-    assert parseaddr_unicode("First Last <abcd@gmail.com>") == ("First Last", "abcd@gmail.com")
+    assert parseaddr_unicode("First Last <abcd@gmail.com>") == (
+        "First Last",
+        "abcd@gmail.com",
+    )
 
     # Handle quote
-    assert parseaddr_unicode('"First Last" <abcd@gmail.com>') == ("First Last", "abcd@gmail.com")
+    assert parseaddr_unicode('"First Last" <abcd@gmail.com>') == (
+        "First Last",
+        "abcd@gmail.com",
+    )
 
     # UTF-8 charset
-    assert parseaddr_unicode("=?UTF-8?B?TmjGoW4gTmd1eeG7hW4=?= <abcd@gmail.com>") == ('Nhơn Nguyễn', "abcd@gmail.com")
+    assert parseaddr_unicode("=?UTF-8?B?TmjGoW4gTmd1eeG7hW4=?= <abcd@gmail.com>") == (
+        "Nhơn Nguyễn",
+        "abcd@gmail.com",
+    )
 
     # iso-8859-1 charset
-    assert parseaddr_unicode("=?iso-8859-1?q?p=F6stal?= <abcd@gmail.com>") == ('pöstal', "abcd@gmail.com")
+    assert parseaddr_unicode("=?iso-8859-1?q?p=F6stal?= <abcd@gmail.com>") == (
+        "pöstal",
+        "abcd@gmail.com",
+    )

+ 31 - 1
tests/test_models.py

@@ -5,7 +5,7 @@ import pytest
 
 from app.config import EMAIL_DOMAIN, MAX_NB_EMAIL_FREE_PLAN
 from app.extensions import db
-from app.models import generate_email, User, Alias
+from app.models import generate_email, User, Alias, Contact
 
 
 def test_generate_email(flask_client):
@@ -63,3 +63,33 @@ def test_alias_create_random(flask_client):
 
     alias = Alias.create_new_random(user)
     assert alias.email.endswith(EMAIL_DOMAIN)
+
+
+def test_website_send_to(flask_client):
+    user = User.create(
+        email="a@b.c", password="password", name="Test User", activated=True
+    )
+    db.session.commit()
+
+    alias = Alias.create_new_random(user)
+    db.session.commit()
+
+    # non-empty name
+    c1 = Contact.create(
+        user_id=user.id,
+        alias_id=alias.id,
+        website_email="abcd@example.com",
+        reply_email="rep@SL",
+        name="First Last",
+    )
+    assert c1.website_send_to() == '"First Last | abcd at example.com" <rep@SL>'
+
+    # empty name, ascii website_from, easy case
+    c1.name = None
+    c1.website_from = "First Last <abcd@example.com>"
+    assert c1.website_send_to() == '"First Last | abcd at example.com" <rep@SL>'
+
+    # empty name, RFC 2047 website_from
+    c1.name = None
+    c1.website_from = "=?UTF-8?B?TmjGoW4gTmd1eeG7hW4=?= <abcd@example.com>"
+    assert c1.website_send_to() == '"Nhơn Nguyễn | abcd at example.com" <rep@SL>'