From 3924c2bd3da0fc2fa4a34c10fee4112bbb8b3b57 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Thu, 26 Apr 2012 16:44:35 +0200 Subject: [PATCH] wip: UI refactoring * We now make a distinction between aliases, fowards and distribution lists * Admin listing were updated see #204 --- admin/forms.py | 52 ++++++++++++ admin/models.py | 7 +- admin/tables.py | 36 ++++---- admin/templatetags/admin_extras.py | 38 ++++++--- admin/tests.py | 17 +++- admin/urls.py | 6 +- admin/views.py | 125 +++++++++++++++++----------- static/css/admin.css | 4 +- static/js/admin.js | 6 +- templates/admin/editdomainform.html | 3 + templates/admin/identities.html | 3 + 11 files changed, 210 insertions(+), 87 deletions(-) diff --git a/admin/forms.py b/admin/forms.py index f4000d841..e20fa46cc 100644 --- a/admin/forms.py +++ b/admin/forms.py @@ -195,6 +195,58 @@ class DlistForm(forms.ModelForm, DynamicForm): self.save_m2m() return dlist +class ForwardForm(forms.ModelForm): + email = forms.EmailField( + label=ugettext_lazy("Email address"), + help_text=ugettext_lazy("The forwarded address") + ) + recipient = forms.EmailField( + label=ugettext_lazy("Recipient"), + help_text=ugettext_lazy("The address to forward messages to") + ) + + class Meta: + model = Alias + fields = ("enabled",) + + def __init__(self, *args, **kwargs): + super(ForwardForm, self).__init__(*args, **kwargs) + self.fields.keyOrder = ['email', 'recipient', 'enabled'] + if kwargs.has_key("instance"): + forward = kwargs["instance"] + self.fields["email"].initial = forward.full_address + self.fields["recipient"].initial = forward.extmboxes + + def clean_email(self): + localpart, domname = split_mailbox(self.cleaned_data["email"]) + try: + Domain.objects.get(name=domname) + except Domain.DoesNotExist: + raise forms.ValidationError(_("Domain does not exist")) + return self.cleaned_data["email"] + + def clean_recipient(self): + local_part, domname = split_mailbox(self.cleaned_data["recipient"]) + try: + domain = Domain.objects.get(name=domname) + except Domain.DoesNotExist: + pass + else: + raise forms.ValidationError(_("Local addresses are forbidden")) + return self.cleaned_data["recipient"] + + def set_recipients(self, user): + pass + + def save(self, commit=True): + forward = super(ForwardForm, self).save(commit=False) + localpart, domname = split_mailbox(self.cleaned_data["email"]) + forward.address = localpart + forward.domain = Domain.objects.get(name=domname) + if commit: + forward.save([], [self.cleaned_data["recipient"]]) + return forward + class ImportDataForm(forms.Form): sourcefile = forms.FileField(label=ugettext_lazy("Select a file")) sepcar = forms.CharField(label=ugettext_lazy("Separator"), max_length=1, required=False) diff --git a/admin/models.py b/admin/models.py index 47b1f7449..c352d38d1 100644 --- a/admin/models.py +++ b/admin/models.py @@ -712,7 +712,10 @@ class Alias(DatesAware): @property def name_or_rcpt(self): - return "%s, ..." % self.get_recipients()[0] + rcpts = self.get_recipients() + if len(rcpts) > 1: + return "%s, ..." % rcpts[0] + return rcpts[0] def save(self, int_rcpts, ext_rcpts, *args, **kwargs): if len(ext_rcpts): @@ -729,7 +732,7 @@ class Alias(DatesAware): self.mboxes.remove(t) def get_recipients(self): - """Return a the recipients list + """Return the recipients list Internal and external addresses are mixed into a single list. """ diff --git a/admin/tables.py b/admin/tables.py index 34aaf4245..19934cc36 100644 --- a/admin/tables.py +++ b/admin/tables.py @@ -6,37 +6,32 @@ from templatetags.admin_extras import * class DomainsTable(tables.Table): tableid = "objects_table" idkey = "id" + styles = "table" name = tables.LinkColumn( "name", label=ugettext_lazy("Name"), urlpattern="modoboa.admin.views.editdomain", title=_("Edit domain"), modal=True, modalcb="domainform_cb" ) - domaliases = tables.Column("domainalias_count", label=ugettext_lazy("Domain aliases"), - width="100px", align="center") - mboxes = tables.Column("mailbox_count", label=ugettext_lazy("Mailboxes"), - width="100px", align="center") - mbaliases = tables.Column("mbalias_count", label=ugettext_lazy("Mailbox aliases"), - width="100px", align="center") - quota = tables.Column("quota", label=ugettext_lazy("Quota"), - width="50px", align="center") - enabled = tables.Column("enabled", label=gender("Enabled", "m"), width="50px", - align="center") - actions = tables.ActionColumn("actions", label=ugettext_lazy("Actions"), width="70px", - align="center", defvalue=domain_actions) + domaliases = tables.Column("domainalias_set", + label=ugettext_lazy("Alias(es)"), safe=True) + actions = tables.ActionColumn("actions", label=ugettext_lazy("Actions"), + defvalue=domain_actions) - cols_order = ["name", "domaliases", "mboxes", "mbaliases", - "quota", "actions"] + cols_order = ["name", "domaliases", "actions"] def __init__(self, request, doms): super(DomainsTable, self).__init__(request) self.populate(self._rows_from_model(doms)) - def parse_quota(self, value): - return "%s %s" % (value, _("MB")) - def parse_enabled(self, value): - return _("yes") if value else _("no") + def parse_domainalias_set(self, aliases): + if not len(aliases.all()): + return "---" + res = "" + for da in aliases.all(): + res += "%s
" % da.name + return res def row_class(self, request, domain): if not domain.enabled: @@ -54,12 +49,13 @@ class ExtensionsTable(tables.Table): class IdentitiesTable(tables.Table): idkey = "id" + styles = "table" identity = tables.LinkColumn( "identity", label=ugettext_lazy("Email/Username"), modal=True, urlpattern={"User" : "modoboa.admin.views.editaccount", - "Alias" : "modoboa.admin.views.editdlist"}, - modalcb={"User" : "editaccount_cb", "Alias" : "dlistform_cb"} + "Alias" : "modoboa.admin.views.editalias"}, + modalcb={"User" : "editaccount_cb", "Alias" : "aliasform_cb"} ) name_or_rcpt = tables.Column("name_or_rcpt", label=ugettext_lazy("Fullname/Recipient")) actions = tables.ActionColumn("actions", label=ugettext_lazy("Actions"), diff --git a/admin/templatetags/admin_extras.py b/admin/templatetags/admin_extras.py index ea8f3ac4a..de2bd07c4 100644 --- a/admin/templatetags/admin_extras.py +++ b/admin/templatetags/admin_extras.py @@ -64,7 +64,7 @@ def settings_menu(selection, user): def domains_menu(selection, user): entries = [ {"name" : "newdomain", - "label" : _("New domain"), + "label" : _("Add domain"), "img" : "icon-plus", "modal" : True, "modalcb" : "domainform_cb", @@ -87,16 +87,22 @@ def domains_menu(selection, user): def identities_menu(user): entries = [ {"name" : "newaccount", - "label" : _("New account"), + "label" : _("Add account"), "img" : "icon-plus", "modal" : True, "modalcb" : "newaccount_cb", "url" : reverse(admin.views.newaccount)}, - {"name" : "newdlist", - "label" : _("New distribution list"), + {"name" : "newforward", + "label" : _("Add forward"), "img" : "icon-plus", "modal" : True, - "modalcb" : "dlistform_cb", + "modalcb" : "aliasform_cb", + "url" : reverse(admin.views.newforward)}, + {"name" : "newdlist", + "label" : _("Add distribution list"), + "img" : "icon-plus", + "modal" : True, + "modalcb" : "aliasform_cb", "url" : reverse(admin.views.newdlist)}, {"name" : "import", "label" : _("Import"), @@ -133,12 +139,22 @@ def identity_actions(user, iid): "title" : _("Delete this account")}, ] else: - actions = [ - {"name" : "deldlist", - "url" : reverse(admin.views.deldlist) + "?selection=%s" % objid, - "img" : "icon-trash", - "title" : _("Delete this distribution list")}, - ] + from modoboa.admin.models import Alias + alias = Alias.objects.get(pk=objid) + if len(alias.get_recipients()) >= 2: + actions = [ + {"name" : "deldlist", + "url" : reverse(admin.views.deldlist) + "?selection=%s" % objid, + "img" : "icon-trash", + "title" : _("Delete this distribution list")}, + ] + else: + actions = [ + {"name" : "delforward", + "url" : reverse(admin.views.delforward) + "?selection=%s" % objid, + "img" : "icon-trash", + "title" : _("Delete this forward")}, + ] return render_actions(actions) @register.simple_tag diff --git a/admin/tests.py b/admin/tests.py index fa8e3ccf1..626db79b6 100644 --- a/admin/tests.py +++ b/admin/tests.py @@ -154,7 +154,7 @@ class AliasTestCase(ModoTestCase): dlist = Alias.objects.get(address="all", domain__name="test.com") self.assertEqual(len(dlist.get_recipients()), 3) del values["recipients_1"] - self.check_ajax_post("/modoboa/admin/distriblists/edit/%d/" % dlist.id, + self.check_ajax_post("/modoboa/admin/aliases/edit/%d/" % dlist.id, values) self.assertEqual(len(dlist.get_recipients()), 2) @@ -163,6 +163,21 @@ class AliasTestCase(ModoTestCase): self.assertRaises(Alias.DoesNotExist, Alias.objects.get, address="all", domain__name="test.com") + def test_forward(self): + values = dict(email="forward@test.com", recipient="rcpt@dest.com") + self.check_ajax_post("/modoboa/admin/forwards/new/", values) + fwd = Alias.objects.get(address="forward", domain__name="test.com") + self.assertEqual(len(fwd.get_recipients()), 1) + + values["recipient"] = "rcpt2@dest.com" + self.check_ajax_post("/modoboa/admin/aliases/edit/%d/" % fwd.id, + values) + self.assertEqual(len(fwd.get_recipients()), 1) + + self.check_ajax_get("/modoboa/admin/forwards/delete/?selection=%d" \ + % fwd.id, {}) + self.assertRaises(Alias.DoesNotExist, Alias.objects.get, + address="forward", domain__name="test.com") class PermissionsTestCase(ModoTestCase): fixtures = ["test_content.json"] diff --git a/admin/urls.py b/admin/urls.py index 2251bc5d3..d4494d584 100644 --- a/admin/urls.py +++ b/admin/urls.py @@ -30,6 +30,10 @@ urlpatterns = patterns( (r'^accounts/delete/$', 'delaccount'), (r'^distriblists/new/$', 'newdlist'), - (r'^distriblists/edit/(?P\d+)/$', 'editdlist'), (r'^distriblists/delete/$', 'deldlist'), + + (r'^forwards/new/$', 'newforward'), + (r'^forwards/delete/$', 'delforward'), + + (r'^aliases/edit/(?P\d+)/$', 'editalias') ) diff --git a/admin/views.py b/admin/views.py index 84afbe789..4a5e3e4c0 100644 --- a/admin/views.py +++ b/admin/views.py @@ -166,8 +166,8 @@ def mailboxes_search(request): result = map(lambda mb: mb.full_address, mboxes) return ajax_simple_response(result) -def _validate_dlist(request, form, successmsg, tplname, commonctx, callback=None): - """Distribution list validation +def _validate_alias(request, form, successmsg, tplname, commonctx, callback=None): + """Alias validation Common function shared between creation and modification actions. """ @@ -193,67 +193,96 @@ def _validate_dlist(request, form, successmsg, tplname, commonctx, callback=None commonctx["error"] = error return ajax_response(request, status="ko", template=tplname, **commonctx) +def _new_alias(request, tplname, formclass, ctx, successmsg): + events.raiseEvent("CanCreate", request.user, "mailbox_aliases") + ctx.update(action_label=_("Create"), action_classes="submit") + if request.method == "POST": + def callback(user, alias): + grant_access_to_object(user, alias, is_owner=True) + events.raiseEvent("MailboxAliasCreated", user, alias) + + form = formclass(request.POST) + return _validate_alias(request, form, successmsg, tplname, ctx, callback) + + form = formclass() + ctx["form"] = form + return _render(request, tplname, ctx) + @login_required @permission_required("admin.add_alias") @transaction.commit_on_success -def newdlist(request, tplname="common/generic_modal_form.html"): - events.raiseEvent("CanCreate", request.user, "mailbox_aliases") - commonctx = {"title" : _("New distribution list"), - "action_label" : _("Create"), - "action_classes" : "submit", - "action" : reverse(newdlist), - "formid" : "dlistform"} +def newdlist(request): + ctx = dict(title=_("New distribution list"), + action=reverse(newdlist), + formid="aliasform") + return _new_alias(request, "common/generic_modal_form.html", + DlistForm, ctx, _("Distribution list created")) +@login_required +@permission_required("admin.add_alias") +@transaction.commit_on_success +def newforward(request): + ctx = dict(title=_("New forward"), + action=reverse(newforward), + formid="aliasform") + return _new_alias(request, "common/generic_modal_form.html", + ForwardForm, ctx, _("Forward created")) + +def _edit_alias(request, alias, tplname, formclass, ctx, successmsg): + if not request.user.can_access(alias.domain): + raise PermDeniedException(_("You do not have access to this domain")) + ctx.update(title=alias.full_address, action_label=_("Update"), + action_classes="submit") if request.method == "POST": - def callback(user, dlist): - grant_access_to_object(user, dlist, is_owner=True) - events.raiseEvent("MailboxAliasCreated", user, dlist) + form = formclass(request.POST, instance=alias) + return _validate_alias(request, form, successmsg, tplname, ctx) - form = DlistForm(request.POST) - return _validate_dlist(request, form, _("Distribution list created"), - tplname, commonctx, callback) + form = formclass(instance=alias) + ctx["form"] = form + return _render(request, tplname, ctx) - form = DlistForm() - commonctx["form"] = form - return _render(request, "common/generic_modal_form.html", commonctx) +def editdlist(request, alias, ctx): + return _edit_alias(request, alias, "admin/dlistform.html", + DlistForm, ctx, _("Distribution list modified")) + +def editforward(request, alias, ctx): + return _edit_alias(request, alias, "common/generic_modal_form.html", + ForwardForm, ctx, _("Forward modified")) @login_required @permission_required("admin.change_alias") -def editdlist(request, dlist_id, tplname="admin/dlistform.html"): - dlist = Alias.objects.get(pk=dlist_id) - if not request.user.can_access(dlist.domain): - raise PermDeniedException(_("You do not have access to this domain")) - commonctx = {"title" : dlist.full_address, - "action_label" : _("Update"), - "action_classes" : "submit", - "action" : reverse(editdlist, args=[dlist.id]), - "formid" : "dlistform"} - if request.method == "POST": - form = DlistForm(request.POST, instance=dlist) - return _validate_dlist(request, form, _("Distribution list modified"), - tplname, commonctx) +def editalias(request, alid): + alias = Alias.objects.get(pk=alid) + ctx = dict(action=reverse(editalias, args=[alias.id]), formid="aliasform") + if len(alias.get_recipients()) >= 2: + return editdlist(request, alias, ctx) + return editforward(request, alias, ctx) - form = DlistForm(instance=dlist) - commonctx["form"] = form - return _render(request, tplname, commonctx) +def _del_alias(request, msg, msgs): + selection = request.GET["selection"].split(",") + for alid in selection: + alias = Alias.objects.get(pk=alid) + if not request.user.can_access(alias.domain): + raise PermDeniedException(_("You do not have access to this domain")) + events.raiseEvent("MailboxAliasDeleted", alias) + ungrant_access_to_object(alias) + alias.delete() + + msg = ungettext(msg, msgs, len(selection)) + messages.info(request, msg) + return ajax_response(request) @login_required @permission_required("admin.delete_alias") @transaction.commit_on_success def deldlist(request): - selection = request.GET["selection"].split(",") - for dlist_id in selection: - dlist = Alias.objects.get(pk=dlist_id) - if not request.user.can_access(dlist.domain): - raise PermDeniedException(_("You do not have access to this domain")) - events.raiseEvent("MailboxAliasDeleted", dlist) - ungrant_access_to_object(dlist) - dlist.delete() + return _del_alias(request, "Distribution list deleted", "Distribution lists deleted") - msg = ungettext("Distribution list deleted", - "Distribution lists deleted", len(selection)) - messages.info(request, msg) - return ajax_response(request) +@login_required +@permission_required("admin.delete_alias") +@transaction.commit_on_success +def delforward(request): + return _del_alias(request, "Forward deleted", "Forwards deleted") @login_required @user_passes_test(lambda u: u.has_perm("auth.add_user") or u.has_perm("admin.add_alias")) @@ -278,8 +307,10 @@ def identities(request, tplname='admin/identities.html'): account.has_mailbox and request.user.can_access(account.mailbox_set.all()[0].domain): accounts_list += [account] for mbalias in mbaliases: - if len(mbalias.get_recipients()) >= 2: + if len(mbalias.get_recipients()) >= 2 or \ + not len(mbalias.mboxes.all()): accounts_list += [mbalias] + return render_listing(request, "identities", tplname, objects=accounts_list, squery=squery) diff --git a/static/css/admin.css b/static/css/admin.css index 823c6087f..7f87f9f60 100644 --- a/static/css/admin.css +++ b/static/css/admin.css @@ -11,11 +11,11 @@ span.title { width: 180px; } -table#domadmins th#actions { +table th#actions { text-align: right; } -table#domadmins td[name="actions"] { +table td[name="actions"] { text-align: right; } diff --git a/static/js/admin.js b/static/js/admin.js index f45ab9df9..7a5117ccc 100644 --- a/static/js/admin.js +++ b/static/js/admin.js @@ -50,7 +50,7 @@ function editaccount_cb() { }); } -function dlistform_cb() { +function aliasform_cb() { $("#id_email").autocompleter({ from_character: "@", choices: get_domains_list @@ -58,8 +58,8 @@ function dlistform_cb() { $("#id_recipients").dynamic_input(); $(".submit").one('click', function(e) { simple_ajax_form_post(e, { - formid: "dlistform", - error_cb: dlistform_cb + formid: "aliasform", + error_cb: aliasform_cb }); }); } diff --git a/templates/admin/editdomainform.html b/templates/admin/editdomainform.html index 137140311..fa6bbf820 100644 --- a/templates/admin/editdomainform.html +++ b/templates/admin/editdomainform.html @@ -26,6 +26,9 @@
{% trans "Creation date" %}{{ domain.creation }}
{% trans "Last modification date" %}{{ domain.last_modification }}
+
{% trans "Domain aliases" %}{{ domain.domainalias_count }}
+
{% trans "Mailboxes" %}{{ domain.mailbox_count }}
+
{% trans "Mailbox aliase" %}{{ domain.mbalias_count }}
{% endblock %} diff --git a/templates/admin/identities.html b/templates/admin/identities.html index 7efb55da3..ec2cdbe69 100644 --- a/templates/admin/identities.html +++ b/templates/admin/identities.html @@ -25,6 +25,9 @@ $(document).ready(function() { $("a[name=deldlist]").confirm({ question: gettext("Delete this list?") }); + $("a[name=delforward]").confirm({ + question: gettext("Delete this forward?") + }); $("#pagination_bar").find(".disabled").click(function(e) { e.preventDefault(); });