Better error validation.

see #491

modoboa/extensions/admin/__init__.py

@@ -21,6 +21,7 @@ admin_events = [
     "GetDomainActions",
     "GetDomainModifyLink",
     "GetDomainAliasQuerySet",
+    "CheckDomainName",
 
     "DomainAliasCreated",
     "DomainAliasDeleted",

modoboa/extensions/admin/forms/domain.py

@@ -16,7 +16,10 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm):
     aliases = DomainNameField(
         label=ugettext_lazy("Alias(es)"),
         required=False,
-        help_text=ugettext_lazy("Alias(es) of this domain. Indicate only one name per input, press ENTER to add a new input.")
+        help_text=ugettext_lazy(
+            "Alias(es) of this domain. Indicate only one name per input, "
+            "press ENTER to add a new input."
+        )
     )
 
     class Meta:
@@ -41,35 +44,48 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm):
                 self._create_field(forms.CharField, name, dalias.name, 3)
 
     def clean(self):
+        """Custom fields validation.
+
+        We want to prevent duplicate names between domains and domain
+        aliases. Extensions have the possibility to declare other
+        objects (see *CheckDomainName* event).
+
+        The validation way is not very smart...
+        """
         super(DomainFormGeneral, self).clean()
-        if len(self._errors):
+        if self._errors:
             raise forms.ValidationError(self._errors)
 
         cleaned_data = self.cleaned_data
         name = cleaned_data["name"]
-
-        try:
-            DomainAlias.objects.get(name=name)
-        except DomainAlias.DoesNotExist:
-            pass
-        else:
-            self._errors["name"] = self.error_class([_("An alias with this name already exists")])
-            del cleaned_data["name"]
-
+        dtypes = events.raiseQueryEvent('CheckDomainName')
+        for dtype, label in [(DomainAlias, _('domain alias'))] + dtypes:
+            try:
+                dtype.objects.get(name=name)
+            except dtype.DoesNotExist:
+                pass
+            else:
+                self._errors["name"] = self.error_class(
+                    [_("A %s with this name already exists" % unicode(label))]
+                )
+                del cleaned_data["name"]
+                break
         for k in cleaned_data.keys():
             if not k.startswith("aliases"):
                 continue
             if cleaned_data[k] == "":
                 del cleaned_data[k]
                 continue
-            try:
-                Domain.objects.get(name=cleaned_data[k])
-            except Domain.DoesNotExist:
-                pass
-            else:
-                self._errors[k] = self.error_class([_("A domain with this name already exists")])
+            for dtype, label in [(Domain, _('domain'))] + dtypes:
+                try:
+                    dtype.objects.get(name=cleaned_data[k])
+                except dtype.DoesNotExist:
+                    continue
+                self._errors[k] = self.error_class(
+                    [_("A %s with this name already exists" % unicode(label))]
+                )
                 del cleaned_data[k]
-
+                break
         return cleaned_data
 
     def update_mailbox_quotas(self, domain):
@@ -148,7 +164,10 @@ class DomainFormOptions(forms.Form):
     dom_admin_username = forms.CharField(
         label=ugettext_lazy("Name"),
         initial="admin",
-        help_text=ugettext_lazy("The administrator's name. Don't include the domain's name here, it will be automatically appended."),
+        help_text=ugettext_lazy(
+            "The administrator's name. Don't include the domain's name here, "
+            "it will be automatically appended."
+        ),
         widget=forms.widgets.TextInput(attrs={"class": "input-small"}),
         required=False
     )

modoboa/extensions/admin/lib.py

@@ -49,6 +49,16 @@ def get_sort_order(qdict, default, allowed_values=None):
 
 
 def get_listing_page(objects, pagenum):
+    """Return specific a listing page.
+
+    A page contains a limited number of elements (see
+    ITEMS_PER_PAGE). If the given page number is wrong, the first page
+    will be always returned.
+
+    :param list objects: object list to paginate
+    :param int pagenum: page number
+    :return: a ``Page`` object
+    """
     paginator = Paginator(
         objects, int(parameters.get_admin("ITEMS_PER_PAGE", app="core"))
     )

modoboa/extensions/admin/views/domain.py

@@ -54,7 +54,7 @@ def _domains(request):
     page = get_listing_page(domainlist, request.GET.get("page", 1))
     return ajax_simple_response({
         "table": _render_to_string(request, 'admin/domains_table.html', {
-            'domains': domainlist,
+            'domains': page.object_list,
             'tableid': 'domains'
         }),
         "page": page.number,

modoboa/extensions/postfix_relay_domains/__init__.py

@@ -6,7 +6,7 @@ from django.utils.translation import ugettext_lazy
 from django.db.models import Q
 from modoboa.lib import events, parameters
 from modoboa.core.extensions import ModoExtension, exts_pool
-from .models import RelayDomain, Service
+from .models import RelayDomain, RelayDomainAlias, Service
 
 extension_events = [
     "RelayDomainCreated",
@@ -124,3 +124,11 @@ def rdomain_actions(user, domain):
         "title": ugettext_lazy("Delete %s?" % domain.name),
         "img": "icon-trash"
     }]
+
+
+@events.observe('CheckDomainName')
+def check_domain_name():
+    return [
+        (RelayDomain, ugettext_lazy('relay domain')),
+        (RelayDomainAlias, ugettext_lazy('relay domain alias'))
+    ]

modoboa/extensions/postfix_relay_domains/forms.py

@@ -1,8 +1,8 @@
 from django import forms
 from django.http import QueryDict
-from django.utils.translation import ugettext_lazy
+from django.utils.translation import ugettext as _, ugettext_lazy
 from modoboa.lib.formutils import DynamicForm, DomainNameField
-from modoboa.extensions.admin.models import Domain
+from modoboa.extensions.admin.models import Domain, DomainAlias
 from .models import RelayDomain, RelayDomainAlias
 
 
@@ -34,37 +34,60 @@ class RelayDomainForm(forms.ModelForm, DynamicForm):
                 self._create_field(forms.CharField, name, rdalias.name, 3)
 
     def clean(self):
+        """Custom fields validaton.
+
+        We want to prevent duplicate names between domains, relay
+        domains, domain aliases and relay domain aliases.
+
+        The validation way is not very smart...
+        """
         super(RelayDomainForm, self).clean()
         if self._errors:
             raise forms.ValidationError(self._errors)
         cleaned_data = self.cleaned_data
-        # try:
-        #     DomainAlias.objects.get(name=name)
-        # except DomainAlias.DoesNotExist:
-        #     pass
-        # else:
-        #     self._errors["name"] = self.error_class([_("An alias with this name already exists")])
-        #     del cleaned_data["name"]
+        for dtype, label in [(Domain, _('domain')),
+                             (DomainAlias, _('domain alias')),
+                             (RelayDomainAlias, _('relay domain alias'))]:
+            try:
+                dtype.objects.get(name=cleaned_data['name'])
+            except dtype.DoesNotExist:
+                pass
+            else:
+                self._errors["name"] = self.error_class(
+                    [_("A %s with this name already exists" % label)]
+                )
+                del cleaned_data["name"]
+                break
 
         for k in cleaned_data.keys():
             if not k.startswith("aliases"):
                 continue
-            if cleaned_data[k] == "":
+            if not cleaned_data[k]:
                 del cleaned_data[k]
                 continue
-            try:
-                Domain.objects.get(name=cleaned_data[k])
-            except Domain.DoesNotExist:
-                pass
-            else:
-                self._errors[k] = self.error_class([_("A domain with this name already exists")])
-                del cleaned_data[k]
+            for dtype, name in [(RelayDomain, _('relay domain')),
+                                (DomainAlias, _('domain alias')),
+                                (Domain, _('domain'))]:
+                try:
+                    dtype.objects.get(name=cleaned_data[k])
+                except dtype.DoesNotExist:
+                    pass
+                else:
+                    self._errors[k] = self.error_class(
+                        [_("A %s with this name already exists" % name)]
+                    )
+                    del cleaned_data[k]
+                    break
 
         return cleaned_data
 
     def save(self, user, commit=True):
-        """Custom save method
+        """Custom save method.
 
+        As relay domain aliases are defined using the same form as
+        relay domains, we need to save them manually.
+
+        :param ``User`` user: connected user
         """
         rd = super(RelayDomainForm, self).save(commit=False)
         if commit:

modoboa/extensions/postfix_relay_domains/models.py

@@ -76,7 +76,7 @@ class RelayDomain(AdminObject):
     )
     target_host = models.CharField(
         ugettext_lazy('target host'), max_length=255,
-        help_text=ugettext_lazy('')
+        help_text=ugettext_lazy('Remote destination of this domain')
     )
     service = models.ForeignKey(Service, default='relay')
     enabled = models.BooleanField(
@@ -85,7 +85,7 @@ class RelayDomain(AdminObject):
     )
     verify_recipients = models.BooleanField(
         ugettext_lazy('verify recipients'),
-        help_text=ugettext_lazy('')
+        help_text=ugettext_lazy('Check for valid recipients')
     )
 
     owners = generic.GenericRelation(ObjectAccess)

modoboa/static/js/global.js

@@ -196,6 +196,8 @@ function simple_ajax_form_post2(e, options) {
             return;
         }
         var result = $.parseJSON(jqxhr.responseText);
+        $("div.error").removeClass("error");
+        $("span.help-inline").remove();
         $.each(result, function(id, value) {
             var fullid = "id_" + id;
             var $widget = $("#" + fullid);