Refactor address & mailbox views

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):
+class MailForm(forms.ModelForm):
-    name = forms.CharField()
+    class Meta:
-    domain = forms.ChoiceField()
+        model = Address
-    mailboxes = forms.MultipleChoiceField(required=False)
+        fields = ("name", "domain", "mailboxes", "forward")
-    forward = forms.EmailField(required=False)
 
     def __init__(self, *args, **kwargs):
-        self.instance = kwargs.pop('instance', None)
+        self.user = kwargs.pop('user')
-        if self.instance is not None:
-            kwargs['initial'] = self.instance.deserialize()
-
-        domains = kwargs.pop('domains')
-        mailboxes = kwargs.pop('mailboxes')
-
         super().__init__(*args, **kwargs)
-        self.fields['domain'].choices = [(d.url, d.name) for d in domains]
+        self.fields['domain'].queryset = Domain.objects.filter(account=self.user)
-        self.fields['mailboxes'].choices = [(m.url, m.name) for m in mailboxes]
+        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):
+    def save(self, commit=True):
-        assert hasattr(self, 'cleaned_data')
+        instance = super().save(commit=False)
-        serialized_data = {
+        instance.account = self.user
-            "name": self.cleaned_data["name"],
+        if commit:
-            "domain": {"url": self.cleaned_data["domain"]},
+            super().save(commit=True)
-            "mailboxes": [{"url": mbox} for mbox in self.cleaned_data["mailboxes"]],
+        return instance
-            "forward": self.cleaned_data["forward"],
-        }
-        return serialized_data
 
 
-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.ModelForm):
-class MailboxCreateForm(forms.Form):
     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):
+    def save(self, commit=True):
-        assert self.is_valid()
+        instance = super().save(commit=False)
-        serialized_data = {
+        instance.account = self.user
-            "name": self.cleaned_data["name"],
+        if commit:
-            "password": self.cleaned_data["password2"],
+            super().save(commit=True)
-            "addresses": self.cleaned_data["addresses"],
+        return instance
-        }
-        return serialized_data
 
 
-class MailboxUpdateForm(forms.Form):
+class MailboxUpdateForm(forms.ModelForm):
     addresses = forms.MultipleChoiceField(required=False)
-
+    class Meta:
-    def __init__(self, *args, **kwargs):
+        fields = ('addresses',)
-        self.instance = kwargs.pop('instance', None)
+        model = Mailbox
-        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

orchestra/contrib/musician/templates/musician/address_form.html

@@ -10,7 +10,7 @@
   {% buttons %}
   <a class="btn btn-light mr-2" href="{% url 'musician:address-list' %}">{% trans "Cancel" %}</a>
   <button type="submit" class="btn btn-secondary">{% trans "Save" %}</button>
-  {% if form.instance %}
+  {% if form.instance.pk %}
   <div class="float-right">
     <a class="btn btn-danger" href="{% url 'musician:address-delete' view.kwargs.pk %}">{% trans "Delete" %}</a>
   </div>

orchestra/contrib/musician/templates/musician/addresses.html

@@ -21,10 +21,10 @@
     <tbody>
       {% for obj in object_list %}
       <tr>
-        <td><a href="{% url 'musician:address-update' obj.id %}">{{ obj.full_address_name }}</a></td>
+        <td><a href="{% url 'musician:address-update' obj.id %}">{{ obj.email }}</a></td>
         <td>{{ obj.domain.name }}</td>
         <td>
-          {% for mailbox in obj.mailboxes %}
+          {% for mailbox in obj.mailboxes.all %}
             <a href="{% url 'musician:mailbox-update' mailbox.id %}">{{ mailbox.name }}</a>
             {% if not forloop.last %}<br/> {% endif %}
           {% endfor %}

orchestra/contrib/musician/templates/musician/mailbox_form.html

@@ -19,10 +19,10 @@
   {% buttons %}
   <a class="btn btn-light mr-2" href="{% url 'musician:mailbox-list' %}">{% trans "Cancel" %}</a>
   <button type="submit" class="btn btn-secondary">{% trans "Save" %}</button>
-  {% if form.instance %}
+  {% if form.instance.pk %}
   <div class="float-right">
-    <a class="btn btn-outline-warning" href="{% url 'musician:mailbox-password' view.kwargs.pk %}"><i class="fas fa-key"></i> {% trans "Change password" %}</a>
+    <a class="btn btn-outline-warning" href="{% url 'musician:mailbox-password' form.instance.pk %}"><i class="fas fa-key"></i> {% trans "Change password" %}</a>
-    <a class="btn btn-danger" href="{% url 'musician:mailbox-delete' view.kwargs.pk %}">{% trans "Delete" %}</a>
+    <a class="btn btn-danger" href="{% url 'musician:mailbox-delete' form.instance.pk %}">{% trans "Delete" %}</a>
   </div>
   {% endif %}
   {% endbuttons %}

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):
+class MailCreateView(CustomContextMixin, UserTokenRequiredMixin, CreateView):
-    service_class = Address
+    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['user'] = self.request.user
-        kwargs['mailboxes'] = self.orchestra.retrieve_mailbox_list()
         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, UpdateView):
-
+    service_class = AddressService
-
+    model = Address
-class MailUpdateView(CustomContextMixin, UserTokenRequiredMixin, FormView):
-    service_class = 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["user"] = self.request.user
-
-        kwargs.update({
-            'instance': instance,
-            'domains': self.orchestra.retrieve_domain_list(),
-            'mailboxes': self.orchestra.retrieve_mailbox_list(),
-        })
-
         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):
+class MailboxUpdateView(CustomContextMixin, UserTokenRequiredMixin, UpdateView):
-        serialized_data = form.serialize()
+    service_class = MailboxService
-        status, response = self.orchestra.create_mailbox(serialized_data)
+    model = Mailbox
-
-        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
     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"