From a3d5e9e2880b5b71703af17331a4f708439f466e Mon Sep 17 00:00:00 2001
From: George Katsikas <giorgakis.katsikas@gmail.com>
Date: Sat, 10 Aug 2024 11:52:18 +0200
Subject: [PATCH] refactor email view address picker

fix from address with double angled brackets
---
 scipost_django/mails/forms.py | 40 ++++++++++++++++++++++++++++++-----
 scipost_django/mails/views.py | 21 ++----------------
 2 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/scipost_django/mails/forms.py b/scipost_django/mails/forms.py
index 0f1220514..847c818ff 100644
--- a/scipost_django/mails/forms.py
+++ b/scipost_django/mails/forms.py
@@ -3,15 +3,21 @@ __license__ = "AGPL v3"
 
 
 import re
+from typing import TYPE_CHECKING
 from django import forms
 
 from common.forms import MultiEmailField
+from common.utils.models import get_current_domain
 from common.utils.text import split_strip
+from scipost.templatetags.user_groups import is_ed_admin, is_scipost_admin
 
 from .core import MailEngine
 from .exceptions import ConfigurationError
 from .widgets import SummernoteEditor
 
+if TYPE_CHECKING:
+    from django.contrib.auth.models import User
+
 
 class EmailForm(forms.Form):
     """
@@ -37,6 +43,9 @@ class EmailForm(forms.Form):
         # Check if all exta configurations are valid.
         self.extra_config.update(kwargs.pop("mail_config", {}))
 
+        # Pop out user to prevent saving it as a form field.
+        user = kwargs.pop("user", None)
+
         if not all(
             key in MailEngine._possible_parameters
             for key, val in self.extra_config.items()
@@ -82,12 +91,12 @@ class EmailForm(forms.Form):
         # Determine the available from addresses passed to the form.
         # Append the default from address from the mail_code json.
         # Remove duplicates and select the default from address as initial.
-        available_from_addresses = kwargs.pop("available_from_addresses", []) + [
+        self.available_from_addresses = [
             (self.engine.mail_data["from_email"], self.engine.mail_data["from_name"])
-        ]
+        ] + EmailForm.get_available_from_addresses_for_user(user)
 
         from_addresses = []
-        for address, description in available_from_addresses:
+        for address, description in self.available_from_addresses:
             if address not in [a[0] for a in from_addresses]:
                 from_addresses.append((address, f"{description} <{address}>"))
 
@@ -132,7 +141,7 @@ class EmailForm(forms.Form):
                     )
 
             from_address = self.cleaned_data.get("from_address")
-            from_address_choices = dict(self.fields["from_address"].choices)
+            from_address_choices = dict(self.available_from_addresses)
             if from_address not in from_address_choices:
                 self.add_error("from_address", "Sender address not in list.")
 
@@ -140,7 +149,7 @@ class EmailForm(forms.Form):
             old_mail_data = self.engine.mail_data
             self.engine.mail_data.update(
                 {
-                    "from_name": from_address_choices.get(from_address, None),
+                    "from_name": from_address_choices.get(from_address, ""),
                     "from_email": from_address,
                     "recipient_list": to_addresses,
                 }
@@ -170,6 +179,27 @@ class EmailForm(forms.Form):
         self.engine.send_mail()
         return self.engine.template_variables["object"]
 
+    @staticmethod
+    def get_available_from_addresses_for_user(user: "User | None") -> list:
+        """Determine the available from addresses based on the request user's permissions.
+
+        Returns a list of tuples with the email address and the human readable name.
+        """
+
+        if user is None:
+            return []
+
+        emails = []
+        domain = get_current_domain()
+
+        if is_ed_admin(user):
+            emails.append(("edadmin@" + domain, "SciPost Editorial Administration"))
+
+        if is_scipost_admin(user):
+            emails.append(("admin@" + domain, "SciPost Administration"))
+
+        return emails
+
 
 class HiddenDataForm(forms.Form):
     """
diff --git a/scipost_django/mails/views.py b/scipost_django/mails/views.py
index 56c321494..d45f358d8 100644
--- a/scipost_django/mails/views.py
+++ b/scipost_django/mails/views.py
@@ -128,30 +128,12 @@ class MailView(MailViewBase, UpdateView):
     form_class = EmailForm
     template_name = "mails/mail_form.html"
 
-    def get_available_from_addresses(self):
-        """Determine the available from addresses based on the request user's permissions.
-
-        Returns a list of tuples with the email address and the human readable name.
-        """
-
-        user = self.request.user
-        emails = []
-        domain = get_current_domain()
-
-        if is_ed_admin(user):
-            emails.append(("edadmin@" + domain, "SciPost Editorial Administration"))
-
-        if is_scipost_admin(user):
-            emails.append(("admin@" + domain, "SciPost Administration"))
-
-        return emails
-
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
         kwargs["mail_code"] = self.mail_code
         kwargs["instance"] = self.get_object()
         kwargs["csp_nonce"] = self.request.csp_nonce
-        kwargs["available_from_addresses"] = self.get_available_from_addresses()
+        kwargs["user"] = self.request.user
         kwargs.update(**self.get_mail_config())
         kwargs.update(**self.mail_variables)
         return kwargs
@@ -191,6 +173,7 @@ class MailEditorSubview:
             request.POST or None,
             csp_nonce=self.request.csp_nonce,
             mail_code=mail_code,
+            user=request.user,
             **kwargs,
         )
         self._is_valid = False
-- 
GitLab