From dfa1f2eecbdd3fdb8c34ee9ce4036c53f9182454 Mon Sep 17 00:00:00 2001 From: George Katsikas <giorgakis.katsikas@gmail.com> Date: Wed, 31 Jul 2024 16:04:29 +0200 Subject: [PATCH] add mail engine choice of from/to address --- scipost_django/mails/core.py | 11 +++- scipost_django/mails/forms.py | 110 +++++++++++++++++++++++++++++----- scipost_django/mails/views.py | 26 +++++++- 3 files changed, 127 insertions(+), 20 deletions(-) diff --git a/scipost_django/mails/core.py b/scipost_django/mails/core.py index 1225035c5..9ddf3a923 100644 --- a/scipost_django/mails/core.py +++ b/scipost_django/mails/core.py @@ -30,6 +30,8 @@ class MailEngine: "from_name", "cc", "bcc", + "html_message", + "message", ] _email_fields = ["recipient_list", "from_email", "cc", "bcc"] _processed_template = False @@ -203,18 +205,21 @@ class MailEngine: try: with open(json_location, "r") as f: - self.mail_data = json.loads(f.read()) + self.default_data = json.loads(f.read()) except OSError: raise ImportError( "No configuration file found. Mail code: %s" % self.mail_code ) # Check if configuration file is valid. - if "subject" not in self.mail_data: + if "subject" not in self.default_data: raise ConfigurationError('key "subject" is missing.') - if "recipient_list" not in self.mail_data: + if "recipient_list" not in self.default_data: raise ConfigurationError('key "recipient_list" is missing.') + # Set mail data to default data when keys are missing. + self.mail_data = {**self.default_data, **getattr(self, "mail_data", {})} + # Overwrite mail data if parameters are given. for key, val in self.extra_config.items(): if val or key not in self.mail_data: diff --git a/scipost_django/mails/forms.py b/scipost_django/mails/forms.py index 21f77127f..0f1220514 100644 --- a/scipost_django/mails/forms.py +++ b/scipost_django/mails/forms.py @@ -2,9 +2,11 @@ __copyright__ = "Copyright © Stichting SciPost (SciPost Foundation)" __license__ = "AGPL v3" +import re from django import forms from common.forms import MultiEmailField +from common.utils.text import split_strip from .core import MailEngine from .exceptions import ConfigurationError @@ -17,8 +19,12 @@ class EmailForm(forms.Form): the mail after editing. """ - subject = forms.CharField(max_length=255, label="Subject*") - text = forms.CharField(widget=SummernoteEditor, label="Text*") + required_css_class = "required-asterisk" + + from_address = forms.ChoiceField(label="From", required=True) + to_address = forms.MultipleChoiceField(label="To", required=True) + subject = forms.CharField(max_length=255, label="Subject") + text = forms.CharField(widget=SummernoteEditor, label="Text") cc_mail_field = MultiEmailField(label="Optional: cc this email to", required=False) bcc_mail_field = MultiEmailField( label="Optional: bcc this email to", required=False @@ -37,7 +43,7 @@ class EmailForm(forms.Form): ): raise KeyError("Not all `extra_config` parameters are accepted.") - # This form shouldn't be is_bound==True is there is any non-relavant POST data given. + # This form shouldn't be is_bound==True if there is any non-relevant POST data given. if len(args) > 0 and args[0]: data = args[0] elif "data" in kwargs: @@ -46,13 +52,19 @@ class EmailForm(forms.Form): data = {} if "%s-subject" % self.prefix in data.keys(): data = { - "%s-subject" % self.prefix: data.get("%s-subject" % self.prefix), - "%s-text" % self.prefix: data.get("%s-text" % self.prefix), - "%s-cc_mail_field" - % self.prefix: data.get("%s-cc_mail_field" % self.prefix), - "%s-bcc_mail_field" - % self.prefix: data.get("%s-bcc_mail_field" % self.prefix), + f"{self.prefix}-{key}": data.get(f"{self.prefix}-{key}") + for prefixed_key in data.keys() + if (key := re.sub(f"^{self.prefix}-", "", prefixed_key)) != prefixed_key + and key in self.base_fields } + + # Cast `to_address` to a list if it is a single string. + # This avoids the MultipleChoiceField raising an error, when the form is bound. + # This cannot happen in `clean_[field]` since it runs after the the default field validation. + to_address = data.get(f"{self.prefix}-to_address", None) + if to_address and not isinstance(to_address, list): + data[f"{self.prefix}-to_address"] = [to_address] + else: # Reset to prevent having a false-bound form. data = {} @@ -62,19 +74,86 @@ class EmailForm(forms.Form): self.engine = MailEngine(self.mail_code, **self.extra_config, **kwargs) self.engine.validate(render_template=True) self.fields["text"].widget = SummernoteEditor( - csp_nonce=kwargs.pop("csp_nonce", "Hello!") + csp_nonce=kwargs.pop("csp_nonce", None) ) self.fields["text"].initial = self.engine.mail_data["html_message"] self.fields["subject"].initial = self.engine.mail_data["subject"] + # 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.engine.mail_data["from_email"], self.engine.mail_data["from_name"]) + ] + + from_addresses = [] + for address, description in available_from_addresses: + if address not in [a[0] for a in from_addresses]: + from_addresses.append((address, f"{description} <{address}>")) + + self.fields["from_address"].choices = from_addresses + self.fields["from_address"].initial = self.engine.mail_data["from_email"] + + # Pass the recipient list to the form, changing the widget if there are multiple recipients. + self.fields["to_address"].choices = [ + (recipient, recipient) + for recipient in self.engine.mail_data["recipient_list"] + ] + self.fields["to_address"].initial = self.engine.mail_data["recipient_list"] + if len(self.fields["to_address"].choices) > 1: + self.fields["to_address"].widget.attrs["size"] = len( + self.fields["to_address"].choices + ) + else: + select_widget = forms.Select(choices=self.fields["to_address"].choices) + self.fields["to_address"].widget = select_widget + + def clean(self): + super().clean() + address = self.cleaned_data.get("to_address") + + if not address: + self.add_error("to_address", "You must select at least one recipient.") + + return self.cleaned_data + def is_valid(self): """Fallback used in CBVs.""" + if super().is_valid(): + # Check that to and from addresses are provided choices. + to_addresses = self.cleaned_data.get("to_address") + to_address_choices = dict(self.fields["to_address"].choices) + for to_address in to_addresses: + if to_address not in to_address_choices: + self.add_error( + f"to_address", + f"Recipient address {to_address} not in {to_address_choices}.", + ) + + from_address = self.cleaned_data.get("from_address") + from_address_choices = dict(self.fields["from_address"].choices) + if from_address not in from_address_choices: + self.add_error("from_address", "Sender address not in list.") + + # Push new data to the engine so it can be validated. + old_mail_data = self.engine.mail_data + self.engine.mail_data.update( + { + "from_name": from_address_choices.get(from_address, None), + "from_email": from_address, + "recipient_list": to_addresses, + } + ) + try: self.engine.validate(render_template=False) return True - except (ImportError, KeyError, ConfigurationError): - return False + except (ImportError, KeyError, ConfigurationError) as e: + self.add_error(None, "The mail could not be validated. " + str(e)) + pass # Fall through to the return False + self.engine.mail_data = old_mail_data + # Reset the mail data to the original state. return False def save(self): @@ -82,11 +161,12 @@ class EmailForm(forms.Form): self.engine.mail_data["subject"] = self.cleaned_data["subject"] if cc_mail_str := self.cleaned_data["cc_mail_field"]: if self.engine.mail_data["cc"]: - self.engine.mail_data["cc"] += [m.strip() for m in cc_mail_str.split(",")] + self.engine.mail_data["cc"] += split_strip(cc_mail_str) else: - self.engine.mail_data["cc"] = [m.strip() for m in cc_mail_str.split(",")] + self.engine.mail_data["cc"] = split_strip(cc_mail_str) if bcc_mail_str := self.cleaned_data["bcc_mail_field"]: - self.engine.mail_data["bcc"] += [m.strip() for m in bcc_mail_str.split(",")] + self.engine.mail_data["bcc"] += split_strip(bcc_mail_str) + self.engine.send_mail() return self.engine.template_variables["object"] diff --git a/scipost_django/mails/views.py b/scipost_django/mails/views.py index 974b28f09..56c321494 100644 --- a/scipost_django/mails/views.py +++ b/scipost_django/mails/views.py @@ -9,6 +9,9 @@ from django.shortcuts import render from django.utils.encoding import force_str from django.views.generic.edit import UpdateView +from common.utils.models import get_current_domain +from scipost.templatetags.user_groups import is_ed_admin, is_scipost_admin + from .forms import EmailForm, HiddenDataForm @@ -63,7 +66,7 @@ class MailFormView(MailViewBase, UpdateView): mail_code=self.mail_code, instance=self.object, **self.get_mail_config(), - **self.mail_variables + **self.mail_variables, ) if self.mail_form.is_valid(): return self.form_valid(form) @@ -125,11 +128,30 @@ 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.update(**self.get_mail_config()) kwargs.update(**self.mail_variables) return kwargs @@ -169,7 +191,7 @@ class MailEditorSubview: request.POST or None, csp_nonce=self.request.csp_nonce, mail_code=mail_code, - **kwargs + **kwargs, ) self._is_valid = False -- GitLab