Improved select account forms

orchestra/contrib/accounts/admin.py

@@ -333,7 +333,7 @@ class SelectAccountAdminMixin(AccountAdminMixin):
         info = opts.app_label, opts.model_name
         account_list = AccountListAdmin(Account, admin_site).changelist_view
         select_urls = [
-            url("/select-account/$",
+            url("add/select-account/$",
                 wrap_admin_view(self, account_list),
                 name='%s_%s_select_account' % info),
         ]
@@ -355,6 +355,7 @@ class SelectAccountAdminMixin(AccountAdminMixin):
                 context = {
                     'title': _("Add %s for %s") % (opts.verbose_name, self.account.username),
                     'from_account': bool(from_account_id),
+                    'from_select': True,
                     'account': self.account,
                     'account_opts': Account._meta,
                 }

orchestra/contrib/accounts/templates/admin/accounts/account/change_form.html

@@ -9,11 +9,13 @@
 &rsaquo; <a href="{% url 'admin:app_list' app_label=account_opts.app_label %}">{{ account_opts.app_config.verbose_name }}</a>
 &rsaquo; <a href="{% url account_opts|admin_urlname:'changelist' %}">{{ account_opts.verbose_name_plural|capfirst }}</a>
 &rsaquo; <a href="{% url account_opts|admin_urlname:'change' account.pk|admin_urlquote %}">{{ account|truncatewords:"18" }}</a>
-&rsaquo; {% if has_change_permission %}<a href="{% url opts|admin_urlname:'changelist' %}?account={{ account.pk }}">{{ opts.verbose_name_plural|capfirst }}</a>{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %}
 {% else %}
 &rsaquo; <a href="{% url 'admin:app_list' app_label=opts.app_label %}">{{ opts.app_config.verbose_name }}</a>
 &rsaquo; {% if has_change_permission %}<a href="{% url opts|admin_urlname:'changelist' %}">{{ opts.verbose_name_plural|capfirst }}</a>{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %}
 {% endif %}
+{% if from_select %}
+&rsaquo; <a href="{% url opts|admin_urlname:'select_account' %}">{% blocktrans with name=original_opts.verbose_name %}Select {{ name }} account{% endblocktrans %}</a>
+{% endif %}
 &rsaquo; {% if add %}{% trans 'Add' %} {{ opts.verbose_name }}{% else %}{{ original|truncatewords:"18" }}{% endif %}
 </div>
 {% endblock %}

orchestra/contrib/accounts/templates/admin/accounts/account/select_account_list.html

@@ -7,8 +7,7 @@
 <a href="{% url 'admin:index' %}">{% trans 'Home' %}</a>
 &rsaquo; <a href="{% url 'admin:app_list' app_label=cl.opts.app_label %}">{{ original_opts.app_config.verbose_name }}</a>
 &rsaquo; <a href="{% url opts|admin_urlname:'changelist' %}">{{ original_opts.verbose_name_plural|capfirst }}</a>
-&rsaquo; {% trans 'Add' %} {{ original_opts.verbose_name }}
+&rsaquo; {% blocktrans with name=original_opts.verbose_name %}Select {{ name }} account{% endblocktrans %}
-&rsaquo; {% trans 'Select account' %}
 </div>
 {% endblock %}
 

orchestra/contrib/mailboxes/admin.py

@@ -124,10 +124,19 @@ class MailboxAdmin(ChangePasswordAdminMixin, SelectAccountAdminMixin, ExtendedMo
         search_term = search_term.replace('@', ' ')
         return super(MailboxAdmin, self).get_search_results(request, queryset, search_term)
     
-    def render_change_form(self, request, context, *args, **kwargs):
+    def render_change_form(self, request, context, add=False, change=False, form_url='', obj=None):
+        if not add:
+            self.check_unrelated_address(request, obj)
+        return super(MailboxAdmin, self).render_change_form(
+            request, context, add, change, form_url, obj)
+    
+    def log_addition(self, request, object):
+        self.check_unrelated_address(request, object)
+        return super(MailboxAdmin, self).log_addition(request, object)
+    
+    def check_unrelated_address(self, request, obj):
         # Check if there exists an unrelated local Address for this mbox
         local_domain = settings.MAILBOXES_LOCAL_DOMAIN
-        obj = kwargs['obj']
         if local_domain and obj.name:
             non_mbox_addresses = Address.objects.exclude(mailboxes__name=obj.name).exclude(
                 forward__regex=r'.*(^|\s)+%s($|\s)+.*' % obj.name)
@@ -137,11 +146,14 @@ class MailboxAdmin(ChangePasswordAdminMixin, SelectAccountAdminMixin, ExtendedMo
                 pass
             else:
                 url = reverse('admin:mailboxes_address_change', args=(addr.pk,))
-                msg = _("Address <a href='{url}'>{addr}</a> clashes with this mailbox "
+                msg = mark_safe(
+                    _("Address <a href='{url}'>{addr}</a> clashes with '{mailbox}' mailbox "
                       "local address. Consider adding this mailbox to the address.").format(
-                    url=url, addr=addr)
+                      mailbox=obj.name, url=url, addr=addr)
-                self.message_user(request, mark_safe(msg), level=messages.WARNING)
+                )
-        return super(MailboxAdmin, self).render_change_form(request, context, *args, **kwargs)
+                # Prevent duplication (add_view+continue)
+                if msg not in (m.message for m in messages.get_messages(request)):
+                    self.message_user(request, msg, level=messages.WARNING)
     
     def save_model(self, request, obj, form, change):
         """ save hacky mailbox.addresses and local domain clashing """

orchestra/contrib/mailboxes/forms.py

@@ -57,7 +57,7 @@ class MailboxForm(forms.ModelForm):
             else:
                 if addr not in cleaned_data.get('addresses', []):
                     raise ValidationError({
-                        'addresses': _("This mailbox matches local address '%s', "
+                        'addresses': _("This mailbox local address matche '%s', "
                                        "please make explicit this fact by selecting it.") % addr
                     })
         return cleaned_data
@@ -98,7 +98,7 @@ class AddressForm(forms.ModelForm):
                     if mailbox.name == name:
                         return
                 raise ValidationError(
-                    _("This address matches mailbox '%s', please make explicit this fact "
+                    _("This address matches mailbox '%s' local address, please make explicit "
-                      "by adding the mailbox on the mailboxes or forward field.") % name
+                      "this fact by adding the mailbox on the mailboxes or forward field.") % name
                 )
         return cleaned_data