MR comments

app/handler/unsubscribe_encoder.py

@@ -1,10 +1,11 @@
+import enum
 from dataclasses import dataclass
 from typing import Optional
 
-from app.models import EnumE
+from app import config
 
 
-class UnsubscribeAction(EnumE):
+class UnsubscribeAction(enum.Enum):
     UnsubscribeNewsletter = 1
     DisableAlias = 2
     DisableContact = 3
@@ -16,18 +17,41 @@ class UnsubscribeData:
     data: int
 
 
+@dataclass
+class UnsubscribeLink:
+    link: str
+    via_email: bool
+
+
 class UnsubscribeEncoder:
     @staticmethod
-    def encode(unsub: UnsubscribeData) -> str:
-        if unsub.action == UnsubscribeAction.DisableAlias:
-            return f"{unsub.data}="
-        if unsub.action == UnsubscribeAction.DisableContact:
-            return f"{unsub.data}_"
-        if unsub.action == UnsubscribeAction.UnsubscribeNewsletter:
-            return f"{unsub.data}*"
-
-    @classmethod
-    def decode(cls, data: str) -> Optional[UnsubscribeData]:
+    def encode(action: UnsubscribeAction, data: int) -> UnsubscribeLink:
+        if config.UNSUBSCRIBER:
+            return UnsubscribeLink(
+                UnsubscribeEncoder.encode_subject(action, data), True
+            )
+        return UnsubscribeLink(UnsubscribeEncoder.encode_url(action, data), False)
+
+    @staticmethod
+    def encode_subject(action: UnsubscribeAction, data: int) -> str:
+        if action == UnsubscribeAction.DisableAlias:
+            return f"{data}="
+        if action == UnsubscribeAction.DisableContact:
+            return f"{data}_"
+        if action == UnsubscribeAction.UnsubscribeNewsletter:
+            return f"{data}*"
+
+    @staticmethod
+    def encode_url(action: UnsubscribeAction, data: int) -> str:
+        if action == UnsubscribeAction.DisableAlias:
+            return f"{config.URL}/dashboard/unsubscribe/{data}"
+        if action == UnsubscribeAction.DisableContact:
+            return f"{config.URL}/dashboard/block_contact/{data}"
+        if action == UnsubscribeAction.UnsubscribeNewsletter:
+            raise Exception("Cannot encode url to disable newsletter")
+
+    @staticmethod
+    def decode_subject(data: str) -> Optional[UnsubscribeData]:
         try:
             # subject has the format {alias.id}=
             if data.endswith("="):

app/handler/unsubscribe_generator.py

@@ -2,6 +2,10 @@ from email.message import Message
 
 from app.email import headers
 from app.email_utils import add_or_replace_header
+from app.handler.unsubscribe_encoder import (
+    UnsubscribeEncoder,
+    UnsubscribeAction,
+)
 from app.models import Alias, Contact
 
 
@@ -14,14 +18,16 @@ class UnsubscribeGenerator:
         """
         user = alias.user
         if user.one_click_unsubscribe_block_sender:
-            unsubscribe_link, via_email = alias.unsubscribe_link(contact)
+            unsub_link = UnsubscribeEncoder.encode(
+                UnsubscribeAction.DisableContact, contact.id
+            )
         else:
-            unsubscribe_link, via_email = alias.unsubscribe_link()
+            unsub_link = UnsubscribeEncoder.encode(
+                UnsubscribeAction.DisableAlias, alias.id
+            )
 
-        add_or_replace_header(
-            message, headers.LIST_UNSUBSCRIBE, f"<{unsubscribe_link}>"
-        )
-        if not via_email:
+        add_or_replace_header(message, headers.LIST_UNSUBSCRIBE, f"<{unsub_link.link}>")
+        if not unsub_link.via_email:
             add_or_replace_header(
                 message, headers.LIST_UNSUBSCRIBE_POST, "List-Unsubscribe=One-Click"
             )

app/handler/unsubscribe_handler.py

@@ -23,7 +23,7 @@ class UnsubscribeHandler:
         header_value = message[headers.SUBJECT]
         if not header_value:
             return None
-        return UnsubscribeEncoder.decode(header_value)
+        return UnsubscribeEncoder.decode_subject(header_value)
 
     def handle_unsubscribe_from_message(self, envelope: Envelope, msg: Message) -> str:
         unsub_data = self._extract_unsub_info_from_message(msg)

app/models.py

@@ -35,6 +35,7 @@ from app.errors import (
     SubdomainInTrashError,
     CannotCreateContactForReverseAlias,
 )
+from app.handler.unsubscribe_encoder import UnsubscribeAction, UnsubscribeEncoder
 from app.log import LOG
 from app.oauth_models import Scope
 from app.pw_models import PasswordOracle
@@ -220,11 +221,6 @@ class BlockBehaviourEnum(EnumE):
     return_5xx = 1
 
 
-class UnsubscribeBehaviourEnum(EnumE):
-    DisableAlias = 0
-    PreserveOriginal = 1
-
-
 class AuditLogActionEnum(EnumE):
     create_object = 0
     update_object = 1
@@ -883,8 +879,10 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle):
             if self.newsletter_alias_id:
                 alias = Alias.get(self.newsletter_alias_id)
                 if alias.enabled:
-                    unsubscribe_link, via_email = alias.unsubscribe_link()
-                    return alias.email, unsubscribe_link, via_email
+                    unsub = UnsubscribeEncoder.encode(
+                        UnsubscribeAction.DisableAlias, alias.id
+                    )
+                    return alias.email, unsub.link, unsub.via_email
                 # alias disabled -> user doesn't want to receive newsletter
                 else:
                     return None, None, False

templates/dashboard/setting.html

@@ -504,38 +504,6 @@
     </div>
     <!-- END One-click subscribe -->
 
-      <div class="card" id="unsub-behaviour">
-          <div class="card-body">
-              <div class="card-title">Unsubscribe list header behaviour</div>
-              <div class="mb-3">
-                  When an email is sent to your mailbox you can decide what do you want the action to be performed when
-                  you unsubscribe from the list. <br>
-                  <b>Original action</b> means that if a message sent to your alias has an unsubscribe header, that same action
-                  will be preserved in the email forwarded to your mailbox.<br>
-                  <b>Disable alias or block contact</b> means that the unsubscribe action will disable that alias and you will stop receiving emails
-                  to it, or the contact will be disabled based on your preferences.<br>
-              </div>
-              <form method="post" action="#change-unsubscribe-behaviour" class="form-inline">
-                  <input type="hidden" name="form-name" value="change-unsubscribe-behaviour">
-
-                  <select class="form-control mr-sm-2" name="unsubscribe-behaviour">
-                      <option value="{{ UnsubscribeBehaviourEnum.PreserveOriginal.name }}"
-                              {% if current_user.unsub_behaviour.value == UnsubscribeBehaviourEnum.PreserveOriginal.value %}
-                              selected="selected" {% endif %}>
-                        Original action
-                      </option>
-                      <option value="{{ UnsubscribeBehaviourEnum.DisableAlias.name }}"
-                              {% if current_user.unsub_behaviour.value == UnsubscribeBehaviourEnum.DisableAlias.value %}
-                              selected="selected" {% endif %}>
-                          Disable alias or block contact.
-                      </option>
-
-                  </select>
-                  <button class="btn btn-outline-primary">Update</button>
-              </form>
-          </div>
-      </div>
-
     <div class="card">
       <div class="card-body">
         <div class="card-title">Quarantine</div>

tests/dashboard/test_directory.py

@@ -62,7 +62,7 @@ def test_create_directory_in_trash(flask_client):
 def test_create_directory_out_of_quota(flask_client):
     user = login(flask_client)
 
-    for i in range(MAX_NB_DIRECTORY):
+    for i in range(MAX_NB_DIRECTORY - Directory.count()):
         Directory.create(name=f"test{i}", user_id=user.id, commit=True)
 
     assert Directory.count() == MAX_NB_DIRECTORY

tests/handler/test_unsubscribe_encoder.py

@@ -1,34 +1,79 @@
 import pytest
 
+from app import config
 from app.handler.unsubscribe_encoder import (
     UnsubscribeData,
     UnsubscribeAction,
     UnsubscribeEncoder,
 )
 
-legacy_test_data = [
+legacy_subject_test_data = [
     ("3=", UnsubscribeData(UnsubscribeAction.DisableAlias, 3)),
     ("438_", UnsubscribeData(UnsubscribeAction.DisableContact, 438)),
     ("4325*", UnsubscribeData(UnsubscribeAction.UnsubscribeNewsletter, 4325)),
 ]
 
 
-@pytest.mark.parametrize("serialized_data, expected", legacy_test_data)
-def test_decode_legacy_unsub(serialized_data, expected):
-    info = UnsubscribeEncoder.decode(serialized_data)
-    assert expected == info
+@pytest.mark.parametrize("expected_subject, expected_deco", legacy_subject_test_data)
+def test_legacy_unsub_subject(expected_subject, expected_deco):
+    info = UnsubscribeEncoder.decode_subject(expected_subject)
+    assert expected_deco == info
+    subject = UnsubscribeEncoder.encode_subject(
+        expected_deco.action, expected_deco.data
+    )
+    assert expected_subject == subject
 
 
-encode_decode_test_data = [
-    UnsubscribeData(UnsubscribeAction.DisableContact, 3),
-    UnsubscribeData(UnsubscribeAction.DisableContact, 10),
-    UnsubscribeData(UnsubscribeAction.DisableAlias, 101),
+legacy_url_test_data = [
+    (
+        f"{config.URL}/dashboard/unsubscribe/3",
+        UnsubscribeData(UnsubscribeAction.DisableAlias, 3),
+    ),
+    (
+        f"{config.URL}/dashboard/block_contact/5",
+        UnsubscribeData(UnsubscribeAction.DisableContact, 5),
+    ),
 ]
 
 
-@pytest.mark.parametrize("unsub_data", encode_decode_test_data)
-def test_encode_decode_unsub(unsub_data):
-    encoded = UnsubscribeEncoder.encode(unsub_data)
-    decoded = UnsubscribeEncoder.decode(encoded)
-    assert unsub_data.action == decoded.action
-    assert unsub_data.data == decoded.data
+@pytest.mark.parametrize("expected_url, unsub_data", legacy_url_test_data)
+def test_encode_decode_unsub_subject(expected_url, unsub_data):
+    url = UnsubscribeEncoder.encode_url(unsub_data.action, unsub_data.data)
+    assert expected_url == url
+
+
+legacy_mail_or_link_test_data = [
+    (
+        f"{config.URL}/dashboard/unsubscribe/3",
+        False,
+        UnsubscribeData(UnsubscribeAction.DisableAlias, 3),
+    ),
+    (
+        "9=",
+        True,
+        UnsubscribeData(UnsubscribeAction.DisableAlias, 9),
+    ),
+    (
+        f"{config.URL}/dashboard/block_contact/8",
+        False,
+        UnsubscribeData(UnsubscribeAction.DisableContact, 8),
+    ),
+    (
+        "8_",
+        True,
+        UnsubscribeData(UnsubscribeAction.DisableContact, 8),
+    ),
+]
+
+
+@pytest.mark.parametrize(
+    "expected_link, via_mail, unsub_data", legacy_mail_or_link_test_data
+)
+def test_encode_legacy_link(expected_link, via_mail, unsub_data):
+    if via_mail:
+        config.UNSUBSCRIBER = "me@nowhere.net"
+    else:
+        config.UNSUBSCRIBER = None
+    link_info = UnsubscribeEncoder.encode(unsub_data.action, unsub_data.data)
+    assert via_mail == link_info.via_email
+    assert expected_link == link_info.link