From fadbfde980805578dc2679e093bf39e569239bb4 Mon Sep 17 00:00:00 2001 From: Maik Hoepfel Date: Fri, 10 May 2013 13:47:18 +0100 Subject: [PATCH] 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 | 1 - oscar/apps/dashboard/partners/app.py | 21 ++-- oscar/apps/dashboard/partners/forms.py | 62 +++++++++++- oscar/apps/dashboard/partners/views.py | 97 ++++++++++++++----- .../partners/messages/user_unlinked.html | 5 + .../dashboard/partners/partner_delete.html | 2 +- .../dashboard/partners/partner_list.html | 12 +-- .../dashboard/partners/partner_user_form.html | 6 +- .../dashboard/partners/partner_user_list.html | 25 +++-- .../partners/partner_user_select.html | 84 ++++++++++++++++ 10 files changed, 259 insertions(+), 56 deletions(-) create mode 100644 oscar/templates/oscar/dashboard/partners/messages/user_unlinked.html create mode 100644 oscar/templates/oscar/dashboard/partners/partner_user_select.html diff --git a/oscar/apps/customer/forms.py b/oscar/apps/customer/forms.py index e1ecf5fbb90..169645b001e 100644 --- a/oscar/apps/customer/forms.py +++ b/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 diff --git a/oscar/apps/dashboard/partners/app.py b/oscar/apps/dashboard/partners/app.py index c613cadebb1..b488cf46092 100644 --- a/oscar/apps/dashboard/partners/app.py +++ b/oscar/apps/dashboard/partners/app.py @@ -10,13 +10,13 @@ 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): @@ -24,21 +24,22 @@ def get_urls(self): url(r'^$', self.list_view.as_view(), name='partner-list'), url(r'^create/$', self.create_view.as_view(), name='partner-create'), - url(r'^(?P\d+)/update/$', self.update_view.as_view(), - name='partner-update'), + url(r'^(?P\d+)/$', self.manage_view.as_view(), + name='partner-manage'), url(r'^(?P\d+)/delete/$', self.delete_view.as_view(), name='partner-delete'), - url(r'^(?P\d+)/users/$', - self.user_list_view.as_view(), - name='partner-user-list'), url(r'^(?P\d+)/users/add/$', self.user_create_view.as_view(), name='partner-user-create'), + url(r'^(?P\d+)/users/select/$', + self.user_select_view.as_view(), + name='partner-user-select'), + url(r'^(?P\d+)/users/(?P\d+)/link/$', + self.user_link_view.as_view(), name='partner-user-link'), url(r'^(?P\d+)/users/(?P\d+)/unlink/$', self.user_unlink_view.as_view(), name='partner-user-unlink'), - - url(r'^users/(?P\d+)/update/$', + url(r'^(?P\d+)/users/(?P\d+)/update/$', self.user_update_view.as_view(), name='partner-user-update'), ) diff --git a/oscar/apps/dashboard/partners/forms.py b/oscar/apps/dashboard/partners/forms.py index db9c3c82cb3..f098b3e37a8 100644 --- a/oscar/apps/dashboard/partners/forms.py +++ b/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') \ No newline at end of file + 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) diff --git a/oscar/apps/dashboard/partners/views.py b/oscar/apps/dashboard/partners/views.py index 24396b53777..e62cddcfed3 100644 --- a/oscar/apps/dashboard/partners/views.py +++ b/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})) diff --git a/oscar/templates/oscar/dashboard/partners/messages/user_unlinked.html b/oscar/templates/oscar/dashboard/partners/messages/user_unlinked.html new file mode 100644 index 00000000000..00688b62d29 --- /dev/null +++ b/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 %} +{% trans 'Undo' %}. diff --git a/oscar/templates/oscar/dashboard/partners/partner_delete.html b/oscar/templates/oscar/dashboard/partners/partner_delete.html index dcc308f4151..8401b1c2713 100644 --- a/oscar/templates/oscar/dashboard/partners/partner_delete.html +++ b/oscar/templates/oscar/dashboard/partners/partner_delete.html @@ -20,7 +20,7 @@ /
  • - {{ partner.name }} + {{ partner.name }} /
  • {% trans "Delete?" %}
    • diff --git a/oscar/templates/oscar/dashboard/partners/partner_list.html b/oscar/templates/oscar/dashboard/partners/partner_list.html index b400c84b6fd..5eff580ef22 100644 --- a/oscar/templates/oscar/dashboard/partners/partner_list.html +++ b/oscar/templates/oscar/dashboard/partners/partner_list.html @@ -49,7 +49,7 @@

    {% trans "Partner management" %}

    {% for partner in partners %} - {{ partner.name }} + {{ partner.name }} {% with users=partner.users.all %} @@ -57,7 +57,7 @@

    {% trans "Partner management" %}

    {% endblock %} -{% block headertext %}{% trans "Update partner user" %}{% endblock %} +{% block headertext %}{{ title }}{% endblock %} {% block dashboard_content %} {% include "partials/form.html" %} diff --git a/oscar/templates/oscar/dashboard/partners/partner_user_list.html b/oscar/templates/oscar/dashboard/partners/partner_user_list.html index 6d60b1b83a2..f86d078ca4f 100644 --- a/oscar/templates/oscar/dashboard/partners/partner_user_list.html +++ b/oscar/templates/oscar/dashboard/partners/partner_user_list.html @@ -17,16 +17,24 @@ /
  • - {{ partner.name }} - / + {{ partner.name }}
    • -
  • {% trans "Manage users" %}
    • {% endblock %} {% block header %}
    - {% trans "Link a new user" %} +
    + + + {% trans 'Link a user' %} + + + +

    {% trans "Manage users" %}

    {% endblock header %} @@ -45,11 +53,11 @@

    {% trans "Manage users" %}

    {% for user in users %} - {{ user.email }} + {{ user.email }} {{ user.first_name|default:"-" }} {{ user.last_name|default:"-" }} -
    + {% csrf_token %}
    @@ -59,7 +67,10 @@

    {% trans "Manage users" %}

    {% else %} - {% trans "No users linked." %} +

    {% trans "No users linked." %}

    {% endif %} {% include "partials/pagination.html" %} + +

    {% trans "Update details" %}

    + {% include 'partials/form.html' %} {% endblock dashboard_content %} diff --git a/oscar/templates/oscar/dashboard/partners/partner_user_select.html b/oscar/templates/oscar/dashboard/partners/partner_user_select.html new file mode 100644 index 00000000000..0a304e3b187 --- /dev/null +++ b/oscar/templates/oscar/dashboard/partners/partner_user_select.html @@ -0,0 +1,84 @@ +{% extends 'dashboard/layout.html' %} +{% load sorting_tags %} +{% load i18n %} + +{% block title %} + {% trans "Partner management" %} | {{ block.super }} +{% endblock %} + +{% block breadcrumbs %} + +{% endblock %} + +{% block header %} +
    +

    {% trans "Link an existing user" %}

    +
    +{% endblock header %} + +{% block dashboard_content %} +
    +
    + {% include 'partials/form_fields_inline.html' with form=form %} + + {% if is_filtered %} + {% trans "Reset" %} + {% endif %} +
    +
    + + {% if is_filtered %} + {% if users.count %} + {% with partner_users=partner.users.all %} + + + + + + + + + + + {% for user in users %} + + + + + + + {% endfor %} + +
    {% trans 'Email' %}{% trans 'First name' %}{% trans 'Last name' %} 
    {{ user.email }}{{ user.first_name|default:"-" }}{{ user.last_name|default:"-" }} + {% if user in partner_users %} + {% blocktrans with name=partner.name %} + User is already linked to {{ name }}. + {% endblocktrans %} + {% else %} +
    + {% csrf_token %} + +
    + {% endif %} +
    + {% endwith %} + {% include "partials/pagination.html" %} + {% else %} + {% trans "No users found." %} + {% endif %} + {% endif %} +{% endblock dashboard_content %}