From d7a2e5c30b196cf62d112e06c610b91a4604ae97 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Sat, 19 Oct 2013 18:32:11 +0200 Subject: [PATCH] Better error validation. see #491 --- modoboa/extensions/admin/__init__.py | 1 + modoboa/extensions/admin/forms/domain.py | 57 ++++++++++++------ modoboa/extensions/admin/lib.py | 10 ++++ modoboa/extensions/admin/views/domain.py | 2 +- .../postfix_relay_domains/__init__.py | 10 +++- .../extensions/postfix_relay_domains/forms.py | 59 +++++++++++++------ .../postfix_relay_domains/models.py | 4 +- modoboa/static/js/global.js | 2 + 8 files changed, 104 insertions(+), 41 deletions(-) diff --git a/modoboa/extensions/admin/__init__.py b/modoboa/extensions/admin/__init__.py index f5737f288..22e84f0c7 100644 --- a/modoboa/extensions/admin/__init__.py +++ b/modoboa/extensions/admin/__init__.py @@ -21,6 +21,7 @@ admin_events = [ "GetDomainActions", "GetDomainModifyLink", "GetDomainAliasQuerySet", + "CheckDomainName", "DomainAliasCreated", "DomainAliasDeleted", diff --git a/modoboa/extensions/admin/forms/domain.py b/modoboa/extensions/admin/forms/domain.py index 2bc9a842a..59682ae6c 100644 --- a/modoboa/extensions/admin/forms/domain.py +++ b/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 ) diff --git a/modoboa/extensions/admin/lib.py b/modoboa/extensions/admin/lib.py index bf142a297..7f8ad30bc 100644 --- a/modoboa/extensions/admin/lib.py +++ b/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")) ) diff --git a/modoboa/extensions/admin/views/domain.py b/modoboa/extensions/admin/views/domain.py index 302fdc5af..812cc4928 100644 --- a/modoboa/extensions/admin/views/domain.py +++ b/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, diff --git a/modoboa/extensions/postfix_relay_domains/__init__.py b/modoboa/extensions/postfix_relay_domains/__init__.py index 7b77ca7b8..912da41f4 100644 --- a/modoboa/extensions/postfix_relay_domains/__init__.py +++ b/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')) + ] diff --git a/modoboa/extensions/postfix_relay_domains/forms.py b/modoboa/extensions/postfix_relay_domains/forms.py index ed60012da..f3100bd68 100644 --- a/modoboa/extensions/postfix_relay_domains/forms.py +++ b/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: diff --git a/modoboa/extensions/postfix_relay_domains/models.py b/modoboa/extensions/postfix_relay_domains/models.py index ac2a167c0..2900b5c35 100644 --- a/modoboa/extensions/postfix_relay_domains/models.py +++ b/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) diff --git a/modoboa/static/js/global.js b/modoboa/static/js/global.js index b3b0ef4e9..1016c0672 100644 --- a/modoboa/static/js/global.js +++ b/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);