From a921e0f648ba531e53bc9872e9d9809e1bb510a6 Mon Sep 17 00:00:00 2001 From: Santiago Lamora Date: Thu, 23 Nov 2023 12:50:55 +0100 Subject: [PATCH] Refactor address & mailbox views --- orchestra/contrib/musician/forms.py | 101 +++++-------- .../templates/musician/address_form.html | 2 +- .../templates/musician/addresses.html | 4 +- .../templates/musician/mailbox_form.html | 6 +- orchestra/contrib/musician/views.py | 139 +++--------------- 5 files changed, 69 insertions(+), 183 deletions(-) diff --git a/orchestra/contrib/musician/forms.py b/orchestra/contrib/musician/forms.py index eae90a37..0a2065f6 100644 --- a/orchestra/contrib/musician/forms.py +++ b/orchestra/contrib/musician/forms.py @@ -1,9 +1,11 @@ - from django import forms from django.contrib.auth.forms import AuthenticationForm from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ +from orchestra.contrib.domains.models import Domain +from orchestra.contrib.mailboxes.models import Address, Mailbox + from . import api @@ -25,23 +27,16 @@ class LoginForm(AuthenticationForm): return self.cleaned_data -class MailForm(forms.Form): - name = forms.CharField() - domain = forms.ChoiceField() - mailboxes = forms.MultipleChoiceField(required=False) - forward = forms.EmailField(required=False) +class MailForm(forms.ModelForm): + class Meta: + model = Address + fields = ("name", "domain", "mailboxes", "forward") def __init__(self, *args, **kwargs): - self.instance = kwargs.pop('instance', None) - if self.instance is not None: - kwargs['initial'] = self.instance.deserialize() - - domains = kwargs.pop('domains') - mailboxes = kwargs.pop('mailboxes') - + self.user = kwargs.pop('user') super().__init__(*args, **kwargs) - self.fields['domain'].choices = [(d.url, d.name) for d in domains] - self.fields['mailboxes'].choices = [(m.url, m.name) for m in mailboxes] + self.fields['domain'].queryset = Domain.objects.filter(account=self.user) + self.fields['mailboxes'].queryset = Mailbox.objects.filter(account=self.user) def clean(self): cleaned_data = super().clean() @@ -49,18 +44,15 @@ class MailForm(forms.Form): raise ValidationError("A mailbox or forward address should be provided.") return cleaned_data - def serialize(self): - assert hasattr(self, 'cleaned_data') - serialized_data = { - "name": self.cleaned_data["name"], - "domain": {"url": self.cleaned_data["domain"]}, - "mailboxes": [{"url": mbox} for mbox in self.cleaned_data["mailboxes"]], - "forward": self.cleaned_data["forward"], - } - return serialized_data + def save(self, commit=True): + instance = super().save(commit=False) + instance.account = self.user + if commit: + super().save(commit=True) + return instance -class MailboxChangePasswordForm(forms.Form): +class MailboxChangePasswordForm(forms.ModelForm): error_messages = { 'password_mismatch': _('The two password fields didn’t match.'), } @@ -76,6 +68,10 @@ class MailboxChangePasswordForm(forms.Form): help_text=_("Enter the same password as before, for verification."), ) + class Meta: + fields = ("password",) + model = Mailbox + def clean_password2(self): password = self.cleaned_data.get("password") password2 = self.cleaned_data.get("password2") @@ -86,15 +82,8 @@ class MailboxChangePasswordForm(forms.Form): ) return password2 - def serialize(self): - assert self.is_valid() - serialized_data = { - "password": self.cleaned_data["password2"], - } - return serialized_data - -class MailboxCreateForm(forms.Form): +class MailboxCreateForm(forms.ModelForm): error_messages = { 'password_mismatch': _('The two password fields didn’t match.'), } @@ -110,12 +99,17 @@ class MailboxCreateForm(forms.Form): strip=False, help_text=_("Enter the same password as before, for verification."), ) - addresses = forms.MultipleChoiceField(required=False) + addresses = forms.ModelMultipleChoiceField(queryset=Address.objects.none(), required=False) + + class Meta: + fields = ("name", "password", "password2", "addresses") + model = Mailbox def __init__(self, *args, **kwargs): - addresses = kwargs.pop('addresses') + user = kwargs.pop('user') super().__init__(*args, **kwargs) - self.fields['addresses'].choices = [(addr.url, addr.full_address_name) for addr in addresses] + self.fields['addresses'].queryset = Address.objects.filter(account=user) + self.user = user def clean_password2(self): password = self.cleaned_data.get("password") @@ -127,31 +121,16 @@ class MailboxCreateForm(forms.Form): ) return password2 - def serialize(self): - assert self.is_valid() - serialized_data = { - "name": self.cleaned_data["name"], - "password": self.cleaned_data["password2"], - "addresses": self.cleaned_data["addresses"], - } - return serialized_data + def save(self, commit=True): + instance = super().save(commit=False) + instance.account = self.user + if commit: + super().save(commit=True) + return instance -class MailboxUpdateForm(forms.Form): +class MailboxUpdateForm(forms.ModelForm): addresses = forms.MultipleChoiceField(required=False) - - def __init__(self, *args, **kwargs): - self.instance = kwargs.pop('instance', None) - if self.instance is not None: - kwargs['initial'] = self.instance.deserialize() - - addresses = kwargs.pop('addresses') - super().__init__(*args, **kwargs) - self.fields['addresses'].choices = [(addr.url, addr.full_address_name) for addr in addresses] - - def serialize(self): - assert self.is_valid() - serialized_data = { - "addresses": self.cleaned_data["addresses"], - } - return serialized_data + class Meta: + fields = ('addresses',) + model = Mailbox diff --git a/orchestra/contrib/musician/templates/musician/address_form.html b/orchestra/contrib/musician/templates/musician/address_form.html index de210674..0e8ff78c 100644 --- a/orchestra/contrib/musician/templates/musician/address_form.html +++ b/orchestra/contrib/musician/templates/musician/address_form.html @@ -10,7 +10,7 @@ {% buttons %} {% trans "Cancel" %} - {% if form.instance %} + {% if form.instance.pk %}
{% trans "Delete" %}
diff --git a/orchestra/contrib/musician/templates/musician/addresses.html b/orchestra/contrib/musician/templates/musician/addresses.html index 1ebc8b72..9a6714a2 100644 --- a/orchestra/contrib/musician/templates/musician/addresses.html +++ b/orchestra/contrib/musician/templates/musician/addresses.html @@ -21,10 +21,10 @@ {% for obj in object_list %} - {{ obj.full_address_name }} + {{ obj.email }} {{ obj.domain.name }} - {% for mailbox in obj.mailboxes %} + {% for mailbox in obj.mailboxes.all %} {{ mailbox.name }} {% if not forloop.last %}
{% endif %} {% endfor %} diff --git a/orchestra/contrib/musician/templates/musician/mailbox_form.html b/orchestra/contrib/musician/templates/musician/mailbox_form.html index 5fb9465e..34922037 100644 --- a/orchestra/contrib/musician/templates/musician/mailbox_form.html +++ b/orchestra/contrib/musician/templates/musician/mailbox_form.html @@ -19,10 +19,10 @@ {% buttons %} {% trans "Cancel" %} - {% if form.instance %} + {% if form.instance.pk %}
- {% trans "Change password" %} - {% trans "Delete" %} + {% trans "Change password" %} + {% trans "Delete" %}
{% endif %} {% endbuttons %} diff --git a/orchestra/contrib/musician/views.py b/orchestra/contrib/musician/views.py index d0c40ec2..76704642 100644 --- a/orchestra/contrib/musician/views.py +++ b/orchestra/contrib/musician/views.py @@ -16,7 +16,8 @@ from django.utils.translation import gettext_lazy as _ from django.views import View from django.views.generic.base import RedirectView, TemplateView from django.views.generic.detail import DetailView -from django.views.generic.edit import DeleteView, FormView +from django.views.generic.edit import (CreateView, DeleteView, FormView, + UpdateView) from django.views.generic.list import ListView from requests.exceptions import HTTPError @@ -256,8 +257,9 @@ class MailView(ServiceListView): return context -class MailCreateView(CustomContextMixin, UserTokenRequiredMixin, FormView): - service_class = Address +class MailCreateView(CustomContextMixin, UserTokenRequiredMixin, CreateView): + service_class = AddressService + model = Address template_name = "musician/address_form.html" form_class = MailForm success_url = reverse_lazy("musician:address-list") @@ -265,24 +267,13 @@ class MailCreateView(CustomContextMixin, UserTokenRequiredMixin, FormView): def get_form_kwargs(self): kwargs = super().get_form_kwargs() - kwargs['domains'] = self.orchestra.retrieve_domain_list() - kwargs['mailboxes'] = self.orchestra.retrieve_mailbox_list() + kwargs['user'] = self.request.user return kwargs - def form_valid(self, form): - # handle request errors e.g. 400 validation - try: - serialized_data = form.serialize() - self.orchestra.create_mail_address(serialized_data) - except HTTPError as e: - form.add_error(field='__all__', error=e) - return self.form_invalid(form) - return super().form_valid(form) - - -class MailUpdateView(CustomContextMixin, UserTokenRequiredMixin, FormView): - service_class = Address +class MailUpdateView(CustomContextMixin, UserTokenRequiredMixin, UpdateView): + service_class = AddressService + model = Address template_name = "musician/address_form.html" form_class = MailForm success_url = reverse_lazy("musician:address-list") @@ -290,27 +281,9 @@ class MailUpdateView(CustomContextMixin, UserTokenRequiredMixin, FormView): def get_form_kwargs(self): kwargs = super().get_form_kwargs() - instance = self.orchestra.retrieve_mail_address(self.kwargs['pk']) - - kwargs.update({ - 'instance': instance, - 'domains': self.orchestra.retrieve_domain_list(), - 'mailboxes': self.orchestra.retrieve_mailbox_list(), - }) - + kwargs["user"] = self.request.user return kwargs - def form_valid(self, form): - # handle request errors e.g. 400 validation - try: - serialized_data = form.serialize() - self.orchestra.update_mail_address(self.kwargs['pk'], serialized_data) - except HTTPError as e: - form.add_error(field='__all__', error=e) - return self.form_invalid(form) - - return super().form_valid(form) - class AddressDeleteView(CustomContextMixin, UserTokenRequiredMixin, DeleteView): template_name = "musician/address_check_delete.html" @@ -355,9 +328,9 @@ class MailingListsView(ServiceListView): # doesn't support filtering by domain domain_id = self.request.GET.get('domain') if domain_id: - return "domain={}".format(domain_id) + return {"domain": domain_id} - return '' + return {} class MailboxesView(ServiceListView): @@ -370,7 +343,7 @@ class MailboxesView(ServiceListView): } -class MailboxCreateView(CustomContextMixin, UserTokenRequiredMixin, FormView): +class MailboxCreateView(CustomContextMixin, UserTokenRequiredMixin, CreateView): service_class = MailboxService model = Mailbox template_name = "musician/mailbox_form.html" @@ -387,68 +360,27 @@ class MailboxCreateView(CustomContextMixin, UserTokenRequiredMixin, FormView): def is_extra_mailbox(self, profile): number_of_mailboxes = len(self.orchestra.retrieve_mailbox_list()) - return number_of_mailboxes >= profile.allowed_resources('mailbox') + # TODO(@slamora): how to retrieve allowed mailboxes? + allowed_mailboxes = 2 # TODO(@slamora): harcoded value + return number_of_mailboxes >= allowed_mailboxes + # return number_of_mailboxes >= profile.allowed_resources('mailbox') def get_form_kwargs(self): kwargs = super().get_form_kwargs() kwargs.update({ - 'addresses': self.orchestra.retrieve_mail_address_list(), + 'user': self.request.user, }) return kwargs - def form_valid(self, form): - serialized_data = form.serialize() - status, response = self.orchestra.create_mailbox(serialized_data) - - if status >= 400: - if status == 400: - # handle errors & add to form (they will be rendered) - form.add_error(field=None, error=response) - else: - logger.error("{}: {}".format(status, response[:120])) - msg = "Sorry, an error occurred while processing your request ({})".format(status) - form.add_error(field='__all__', error=msg) - return self.form_invalid(form) - - return super().form_valid(form) - - -class MailboxUpdateView(CustomContextMixin, UserTokenRequiredMixin, FormView): - service_class = Mailbox +class MailboxUpdateView(CustomContextMixin, UserTokenRequiredMixin, UpdateView): + service_class = MailboxService + model = Mailbox template_name = "musician/mailbox_form.html" form_class = MailboxUpdateForm success_url = reverse_lazy("musician:mailbox-list") extra_context = {'service': service_class} - def get_form_kwargs(self): - kwargs = super().get_form_kwargs() - instance = self.orchestra.retrieve_mailbox(self.kwargs['pk']) - - kwargs.update({ - 'instance': instance, - 'addresses': self.orchestra.retrieve_mail_address_list(), - }) - - return kwargs - - def form_valid(self, form): - serialized_data = form.serialize() - status, response = self.orchestra.update_mailbox(self.kwargs['pk'], serialized_data) - - if status >= 400: - if status == 400: - # handle errors & add to form (they will be rendered) - form.add_error(field=None, error=response) - else: - logger.error("{}: {}".format(status, response[:120])) - msg = "Sorry, an error occurred while processing your request ({})".format(status) - form.add_error(field='__all__', error=msg) - - return self.form_invalid(form) - - return super().form_valid(form) - class MailboxDeleteView(CustomContextMixin, UserTokenRequiredMixin, DeleteView): template_name = "musician/mailbox_check_delete.html" @@ -485,37 +417,12 @@ class MailboxDeleteView(CustomContextMixin, UserTokenRequiredMixin, DeleteView): logger.error("Error sending email to managers", exc_info=True) -class MailboxChangePasswordView(CustomContextMixin, UserTokenRequiredMixin, FormView): +class MailboxChangePasswordView(CustomContextMixin, UserTokenRequiredMixin, UpdateView): template_name = "musician/mailbox_change_password.html" + model = Mailbox form_class = MailboxChangePasswordForm success_url = reverse_lazy("musician:mailbox-list") - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - self.object = self.get_object() - context.update({ - 'object': self.object, - }) - return context - - def get_object(self, queryset=None): - obj = self.orchestra.retrieve_mailbox(self.kwargs['pk']) - return obj - - def form_valid(self, form): - data = { - 'password': form.cleaned_data['password2'] - } - status, response = self.orchestra.set_password_mailbox(self.kwargs['pk'], data) - - if status < 400: - messages.success(self.request, _('Password updated!')) - else: - messages.error(self.request, _('Cannot process your request, please try again later.')) - logger.error("{}: {}".format(status, str(response)[:100])) - - return super().form_valid(form) - class DatabasesView(ServiceListView): template_name = "musician/databases.html"