Refactor partner dashboard views

- Merged PartnerUpdate and PartnerUserList view for a better UX
- Created new ExistingUserForm to allow for updating user details
  without changing password
- PartnerUserUpdateView now enforces user being associated to partner
- Added Undo link when unlinking user
- Enrich user details when creating in form instead of view

oscar/apps/customer/forms.py

@@ -2,7 +2,6 @@
 import random
 
 from django.contrib.auth.forms import AuthenticationForm
-from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext_lazy as _
 from django.core.exceptions import ObjectDoesNotExist
 from django import forms

oscar/apps/dashboard/partners/app.py

@@ -10,35 +10,36 @@ class PartnersDashboardApplication(Application):
 
     list_view = views.PartnerListView
     create_view = views.PartnerCreateView
-    update_view = views.PartnerUpdateView
+    manage_view = views.PartnerManageView
     delete_view = views.PartnerDeleteView
 
-    user_list_view = views.PartnerUserListView
+    user_link_view = views.PartnerUserLinkView
     user_unlink_view = views.PartnerUserUnlinkView
     user_create_view = views.PartnerUserCreateView
-
+    user_select_view = views.PartnerUserSelectView
     user_update_view = views.PartnerUserUpdateView
 
     def get_urls(self):
         urlpatterns = patterns('',
             url(r'^$', self.list_view.as_view(), name='partner-list'),
             url(r'^create/$', self.create_view.as_view(),
                 name='partner-create'),
-            url(r'^(?P<pk>\d+)/update/$', self.update_view.as_view(),
-                name='partner-update'),
+            url(r'^(?P<pk>\d+)/$', self.manage_view.as_view(),
+                name='partner-manage'),
             url(r'^(?P<pk>\d+)/delete/$', self.delete_view.as_view(),
                 name='partner-delete'),
 
-            url(r'^(?P<pk>\d+)/users/$',
-                self.user_list_view.as_view(),
-                name='partner-user-list'),
             url(r'^(?P<partner_pk>\d+)/users/add/$',
                 self.user_create_view.as_view(),
                 name='partner-user-create'),
+            url(r'^(?P<partner_pk>\d+)/users/select/$',
+                self.user_select_view.as_view(),
+                name='partner-user-select'),
+            url(r'^(?P<partner_pk>\d+)/users/(?P<user_pk>\d+)/link/$',
+                self.user_link_view.as_view(), name='partner-user-link'),
             url(r'^(?P<partner_pk>\d+)/users/(?P<user_pk>\d+)/unlink/$',
                 self.user_unlink_view.as_view(), name='partner-user-unlink'),
-
-            url(r'^users/(?P<pk>\d+)/update/$',
+            url(r'^(?P<partner_pk>\d+)/users/(?P<user_pk>\d+)/update/$',
                 self.user_update_view.as_view(),
                 name='partner-user-update'),
         )

oscar/apps/dashboard/partners/forms.py

@@ -2,7 +2,8 @@
 from django.utils.translation import ugettext_lazy as _
 from django.db.models import get_model
 from django import forms
-from oscar.apps.customer.forms import EmailUserCreationForm
+from oscar.apps.customer.forms import EmailUserCreationForm, CommonPasswordValidator
+from django.core import validators
 
 
 Partner = get_model('partner', 'Partner')
@@ -18,7 +19,62 @@ class Meta:
         fields = ('name',)
 
 
-class UserForm(EmailUserCreationForm):
+class NewUserForm(EmailUserCreationForm):
+
+    def __init__(self, partner, *args, **kwargs):
+        self.partner = partner
+        super(NewUserForm, self).__init__(*args, **kwargs)
+
+    def save(self):
+        user = super(EmailUserCreationForm, self).save(commit=False)
+        user.is_staff = True
+        user.save()
+        self.partner.users.add(user)
+        return user
+
     class Meta:
         model = User
-        fields = ('first_name', 'last_name', 'email', 'password1', 'password2')
+        fields = ('first_name', 'last_name', 'email', 'password1', 'password2')
+
+
+class ExistingUserForm(forms.ModelForm):
+    """
+    Slightly different form that makes
+    * makes saving password optional
+    * doesn't regenerate username
+    * doesn't allow changing email till #668 is resolved
+    """
+    password1 = forms.CharField(
+        label=_('Password'),
+        widget=forms.PasswordInput,
+        required=False,
+        validators=[validators.MinLengthValidator(6),
+                    CommonPasswordValidator()])
+    password2 = forms.CharField(
+        required=False,
+        label=_('Confirm Password'),
+        widget=forms.PasswordInput)
+
+    def clean_password2(self):
+        password1 = self.cleaned_data.get('password1', '')
+        password2 = self.cleaned_data.get('password2', '')
+
+        if password1 != password2:
+            raise forms.ValidationError(_("The two password fields didn't match."))
+        return password2
+
+    def save(self, commit=True):
+        user = super(ExistingUserForm, self).save(commit=False)
+        if self.cleaned_data['password1']:
+            user.set_password(self.cleaned_data['password1'])
+        if commit:
+            user.save()
+        return user
+
+    class Meta:
+        model = User
+        fields = ('first_name', 'last_name', 'password1', 'password2')
+
+
+class UserEmailForm(forms.Form):
+    email = forms.CharField(max_length=100)

oscar/apps/dashboard/partners/views.py

@@ -5,9 +5,10 @@
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.utils.translation import ugettext_lazy as _
+from django.template.loader import render_to_string
 from django.views import generic
 
-from oscar.apps.dashboard.partners.forms import UserForm
+from oscar.apps.dashboard.partners.forms import UserEmailForm, ExistingUserForm, NewUserForm
 from oscar.core.loading import get_classes
 from oscar.views.generic import BulkEditMixin
 
@@ -79,15 +80,20 @@ def get_success_url(self):
         return reverse_lazy('dashboard:partner-list')
 
 
-class PartnerUpdateView(generic.UpdateView):
+class PartnerManageView(generic.UpdateView):
+    """
+    Is a merge of an UpdateView to update the name of the partner,
+    and a ListView to list associated users.
+    """
     model = Partner
-    template_name = 'dashboard/partners/partner_form.html'
+    template_name = 'dashboard/partners/partner_user_list.html'
     form_class = PartnerCreateForm
     context_object_name = 'partner'
 
     def get_context_data(self, **kwargs):
-        ctx = super(PartnerUpdateView, self).get_context_data(**kwargs)
+        ctx = super(PartnerManageView, self).get_context_data(**kwargs)
         ctx['title'] = self.object.name
+        ctx['users'] = self.object.users.all()
         return ctx
 
     def get_success_url(self):
@@ -116,7 +122,7 @@ def get_success_url(self):
 class PartnerUserCreateView(generic.CreateView):
     model = User
     template_name = 'dashboard/partners/partner_user_form.html'
-    form_class = UserForm
+    form_class = NewUserForm
 
     def dispatch(self, request, *args, **kwargs):
         self.partner = get_object_or_404(
@@ -127,12 +133,13 @@ def dispatch(self, request, *args, **kwargs):
     def get_context_data(self, **kwargs):
         ctx = super(PartnerUserCreateView, self).get_context_data(**kwargs)
         ctx['partner'] = self.partner
+        ctx['title'] = _('Create user')
         return ctx
 
-    def form_valid(self, form):
-        ret = super(PartnerUserCreateView, self).form_valid(form)
-        self.partner.users.add(self.object)
-        return ret
+    def get_form_kwargs(self):
+        kwargs = super(PartnerUserCreateView, self).get_form_kwargs()
+        kwargs['partner'] = self.partner
+        return kwargs
 
     def get_success_url(self):
         name = self.object.get_full_name() or self.object.email
@@ -142,9 +149,14 @@ def get_success_url(self):
 
 
 class PartnerUserUpdateView(generic.UpdateView):
-    model = User
     template_name = 'dashboard/partners/partner_user_form.html'
-    form_class = UserForm
+    form_class = ExistingUserForm
+
+    def get_object(self, queryset=None):
+        return get_object_or_404(User,
+                                 pk=self.kwargs['user_pk'],
+                                 partners__pk=self.kwargs['partner_pk'],
+                                 is_staff=True)
 
     def get_context_data(self, **kwargs):
         name = self.object.get_full_name() or self.object.email
@@ -159,24 +171,57 @@ def get_success_url(self):
         return reverse_lazy('dashboard:partner-list')
 
 
-class PartnerUserListView(generic.ListView):
-    model = User
-    template_name = 'dashboard/partners/partner_user_list.html'
+class PartnerUserSelectView(generic.ListView):
+    template_name = 'dashboard/partners/partner_user_select.html'
+    form_class = UserEmailForm
     context_object_name = 'users'
 
     def dispatch(self, request, *args, **kwargs):
-        self.partner = get_object_or_404(
-            Partner, pk=kwargs.get('pk', None))
-        return super(PartnerUserListView, self).dispatch(
+        self.partner = get_object_or_404(Partner,
+                                         pk=kwargs.get('partner_pk', None))
+        if 'email' in request.GET:
+            data = request.GET
+        else:
+            data = None
+        self.form = UserEmailForm(data)
+        return super(PartnerUserSelectView, self).dispatch(
             request, *args, **kwargs)
 
     def get_context_data(self, **kwargs):
-        ctx = super(PartnerUserListView, self).get_context_data(**kwargs)
+        ctx = super(PartnerUserSelectView, self).get_context_data(**kwargs)
         ctx['partner'] = self.partner
+        ctx['form'] = self.form
+        ctx['is_filtered'] = self.is_filtered
         return ctx
 
     def get_queryset(self):
-        return self.partner.users.all()
+        email = self.form.data.get('email', '')
+        self.is_filtered = bool(email)
+        if self.is_filtered:
+            return User.objects.filter(is_staff=True, email__icontains=email)
+        else:
+            return User.objects.none()
+
+
+class PartnerUserLinkView(generic.View):
+
+    def get(self, request, user_pk, partner_pk):
+        # need to allow GET to make Undo link in PartnerUserUnlinkView work
+        return self.post(request, user_pk, partner_pk)
+
+    def post(self, request, user_pk, partner_pk):
+        user = get_object_or_404(User, pk=user_pk)
+        partner = get_object_or_404(Partner, pk=partner_pk)
+        name = user.get_full_name() or user.email
+        if not partner.users.filter(pk=user_pk).exists():
+            partner.users.add(user)
+            messages.success(request, _("User '%s' was linked to '%s'") %
+                             (name, partner.name))
+        else:
+            messages.error(request, _("User '%s' is already linked to '%s'") %
+                           (name, partner.name))
+        return HttpResponseRedirect(reverse('dashboard:partner-manage',
+                                            kwargs={'pk': partner_pk}))
 
 
 class PartnerUserUnlinkView(generic.View):
@@ -185,13 +230,17 @@ def post(self, request, user_pk, partner_pk):
         user = get_object_or_404(User, pk=user_pk)
         name = user.get_full_name() or user.email
         partner = get_object_or_404(Partner, pk=partner_pk)
-        users = partner.users.all()
-        if user in users:
+        if partner.users.filter(pk=user_pk).exists():
             partner.users.remove(user)
-            messages.success(request, _("User '%s' was unlinked from '%s'") %
-                             (name, partner.name))
+            msg = render_to_string(
+                'dashboard/partners/messages/user_unlinked.html',
+                {'user_name': name,
+                 'partner_name': partner.name,
+                 'user_pk': user_pk,
+                 'partner_pk': partner_pk })
+            messages.success(self.request, msg, extra_tags='safe')
         else:
             messages.error(request, _("User '%s' is not linked to '%s'") %
                            (name, partner.name))
-        return HttpResponseRedirect(reverse('dashboard:partner-manage-users',
+        return HttpResponseRedirect(reverse('dashboard:partner-manage',
                                             kwargs={'pk': partner_pk}))

oscar/templates/oscar/dashboard/partners/messages/user_unlinked.html

@@ -0,0 +1,5 @@
+{% load i18n %}
+{% blocktrans %}
+User {{ user_name }} was unlinked from {{ partner_name }}.
+{% endblocktrans %}
+<a href="{% url dashboard:partner-user-link partner_pk user_pk %}">{% trans 'Undo' %}.</a>

oscar/templates/oscar/dashboard/partners/partner_delete.html

@@ -20,7 +20,7 @@
             <span class="divider">/</span>
         </li>
         <li>
-            <a href="{% url dashboard:partner-update partner.pk %}">{{ partner.name }}</a>
+            <a href="{% url dashboard:partner-manage partner.pk %}">{{ partner.name }}</a>
             <span class="divider">/</span>
         </li>
         <li class="active">{% trans "Delete?" %}</li>

oscar/templates/oscar/dashboard/partners/partner_list.html

@@ -49,15 +49,15 @@ <h1>{% trans "Partner management" %}</h1>
                 {% for partner in partners %}
                     <tr>
                         <td>
-                            <a href="{% url dashboard:partner-update partner.pk %}">{{ partner.name }}</a>
+                            <a href="{% url dashboard:partner-manage partner.pk %}">{{ partner.name }}</a>
                         </td>
                         <td>
                             {% with users=partner.users.all %}
                                 {% if users %}
                                     <ul>
                                         {% for user in users %}
                                             <li>
-                                                <a href="{% url dashboard:partner-user-update user.id %}" title="{% trans "Edit the user's details" %}">
+                                                <a href="{% url dashboard:partner-user-update partner.pk user.id %}" title="{% trans "Edit the user's details" %}">
                                                     {{ user.get_full_name|default:user.email }}
                                                 </a>
                                             </li>
@@ -76,11 +76,9 @@ <h1>{% trans "Partner management" %}</h1>
                                         <span class="caret"></span>
                                     </a>
                                     <ul class="dropdown-menu pull-right">
-                                        <li><a href="{% url dashboard:partner-update partner.pk %}">{% trans "Edit" %}</a></li>
-                                        <li><a href="{% url dashboard:partner-delete partner.pk %}">{% trans "Delete" %}</a></li>
-                                        {% if partner.users.count %}
-                                            <li><a href="{% url dashboard:partner-user-list partner.pk %}">{% trans "Manage users" %}</a></li>
-                                        {% endif %}
+                                        <li><a href="{% url dashboard:partner-manage partner.pk %}">{% trans "Manage partner and users" %}</a></li>
+                                        <li><a href="{% url dashboard:partner-delete partner.pk %}">{% trans "Delete partner" %}</a></li>
+                                        <li><a href="{% url dashboard:partner-user-select partner.pk %}">{% trans "Link an existing user" %}</a></li>
                                         <li><a href="{% url dashboard:partner-user-create partner.pk %}">{% trans "Link a new user" %}</a></li>
                                     </ul>
                                 </div>

oscar/templates/oscar/dashboard/partners/partner_user_form.html

@@ -5,7 +5,7 @@
 {% block body_class %}create-page partner{% endblock %}
 
 {% block title %}
-    {% trans "Update user" %} | {{ partner.name }} {% trans "Partner management" %} | {{ block.super }}
+    {{ title }} | {{ partner.name }} {% trans "Partner management" %} | {{ block.super }}
 {% endblock %}
 
 {% block breadcrumbs %}
@@ -18,11 +18,11 @@
             <a href="{% url dashboard:partner-list %}">{% trans "Partner management" %}</a>
             <span class="divider">/</span>
         </li>
-        <li class="active">{% trans "Update user" %}</li>
+        <li class="active">{{ title }}</li>
     </ul>
 {% endblock %}
 
-{% block headertext %}{% trans "Update partner user" %}{% endblock %}
+{% block headertext %}{{ title }}{% endblock %}
 
 {% block dashboard_content %}
     {% include "partials/form.html" %}