launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06972
[Merge] lp:~jtv/maas/bug-946878 into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-946878 into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/bug-946878/+merge/100553
This allows an administrator to set a user's password. In the process I had to convert the page from using one of Django's ready-made view to a TemplateView.
As per discussion with Matthew, the admin fields are a separate form on the same page. We felt this was more sensible. If people feel that this works, we can make the page where users edit their own profiles do the same thing. Or if we decide to go with a single form after all, we'll have to change this new form instead.
I extracted some helpers to make it easier to add tests for these views. In particular, a POST must submit all of the field's forms and so I thought it helpful to be able to pre-fill the parameters dict based on current values from a User object.
--
https://code.launchpad.net/~jtv/maas/bug-946878/+merge/100553
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-946878 into lp:maas.
=== modified file 'src/maasserver/templates/maasserver/user_edit.html'
--- src/maasserver/templates/maasserver/user_edit.html 2012-03-02 06:06:51 +0000
+++ src/maasserver/templates/maasserver/user_edit.html 2012-04-03 04:54:18 +0000
@@ -5,13 +5,29 @@
{% block page-title %}Edit User{% endblock %}
{% block content %}
- <form action="." method="post" class="block auto-width">
- <ul>
- {% for field in form %}
- {% include "maasserver/form_field.html" %}
- {% endfor %}
- </ul>
- <input type="submit" value="Save user" class="right" />
- <a class="link-button" href="{% url 'settings' %}">Cancel</a>
- </form>
+ <div id="profile" class="block size7 first">
+ <form action="." method="post" class="block auto-width">
+ <ul>
+ {% for field in profile_form %}
+ {% include "maasserver/form_field.html" %}
+ {% endfor %}
+ </ul>
+ <input type="hidden" name="profile_submit" value="1" />
+ <input type="submit" value="Save user" class="button right" />
+ <a class="link-button" href="{% url 'settings' %}">Cancel</a>
+ </form>
+ </div>
+
+ <div id="password" class="block size7 first">
+ <form action="." method="post" class="block auto-width">
+ <ul>
+ {% for field in password_form %}
+ {% include "maasserver/form_field.html" %}
+ {% endfor %}
+ </ul>
+ <input type="hidden" name="password_submit" value="1" />
+ <input type="submit" value="Change password" class="button right" />
+ <a class="link-button" href="{% url 'settings' %}">Cancel</a>
+ </form>
+ </div>
{% endblock %}
=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py 2012-04-02 16:38:56 +0000
+++ src/maasserver/tests/test_views.py 2012-04-03 04:54:18 +0000
@@ -733,50 +733,118 @@
new_choices)
+def prefix_form_param(param, prefix=None):
+ """Prefix form parameter with form prefix, if given."""
+ if prefix is None:
+ return param
+ else:
+ return '%s-%s' % (prefix, param)
+
+
+def prefix_form_params(params, prefix=None):
+ """Prefix a dict of form parameters with form prefix, if given.
+
+ If a prefix is given, a "*_submit" parameter is added to indicate that
+ the form with this prefix has been submitted..
+ """
+ if prefix is None:
+ return params
+ else:
+ params = {
+ prefix_form_param(key, prefix): value
+ for key, value in params.items()}
+ params['%s_submit' % prefix] = '1'
+ return params
+
+
+# Settable attributes on User.
+user_attributes = [
+ 'email',
+ 'is_superuser',
+ 'last_name',
+ 'username',
+ ]
+
+
+def make_user_attribute_params(user):
+ """Compose a dict of form parameters for a user's account data.
+
+ By default, each attribute in the dict maps to the user's existing value
+ for that atrribute.
+ """
+ return {
+ attr: getattr(user, attr)
+ for attr in user_attributes
+ }
+
+
+def make_password_params(password):
+ """Create a dict of parameters for setting a given password."""
+ return {
+ 'password1': password,
+ 'password2': password,
+ }
+
+
+def get_dict_items(input_dict, keys):
+ """List values from a dict, in the given keys' order."""
+ return [input_dict[key] for key in keys]
+
+
+def get_obj_attrs(input_object, attrs):
+ """List attributes of an object, in the given order."""
+ return [getattr(input_object, attr) for attr in attrs]
+
+
class UserManagementTest(AdminLoggedInTestCase):
def test_add_user_POST(self):
- new_last_name = factory.getRandomString(30)
+ params = {
+ 'username': factory.getRandomString(),
+ 'last_name': factory.getRandomString(30),
+ 'email': factory.getRandomEmail(),
+ 'is_superuser': factory.getRandomBoolean(),
+ }
+ password = factory.getRandomString()
+ params.update(make_password_params(password))
+
+ response = self.client.post(reverse('accounts-add'), params)
+ self.assertEqual(httplib.FOUND, response.status_code)
+ user = User.objects.get(username=params['username'])
+ self.assertEqual(
+ get_dict_items(params, user_attributes),
+ get_obj_attrs(user, user_attributes))
+ self.assertTrue(user.check_password(password))
+
+ def test_edit_user_POST_profile_updates_attributes(self):
+ user = factory.make_user()
+ params = make_user_attribute_params(user)
+ params.update({
+ 'last_name': 'Newname-%s' % factory.getRandomString(),
+ 'email': 'new-%s@xxxxxxxxxxx' % factory.getRandomString(),
+ 'is_superuser': True,
+ 'username': 'newname-%s' % factory.getRandomString(),
+ })
+
+ response = self.client.post(
+ reverse('accounts-edit', args=[user.username]),
+ prefix_form_params(params, 'profile'))
+
+ self.assertEqual(httplib.FOUND, response.status_code)
+ user = reload_object(user)
+ self.assertEqual(
+ get_dict_items(params, user_attributes),
+ get_obj_attrs(user, user_attributes))
+
+ def test_edit_user_POST_updates_password(self):
+ user = factory.make_user()
new_password = factory.getRandomString()
- new_email = factory.getRandomEmail()
- new_admin_status = factory.getRandomBoolean()
- response = self.client.post(
- reverse('accounts-add'),
- {
- 'username': 'my_user',
- 'last_name': new_last_name,
- 'password1': new_password,
- 'password2': new_password,
- 'is_superuser': new_admin_status,
- 'email': new_email,
- })
- self.assertEqual(httplib.FOUND, response.status_code)
- users = list(User.objects.filter(username='my_user'))
- self.assertEqual(1, len(users))
- self.assertEqual(new_last_name, users[0].last_name)
- self.assertEqual(new_admin_status, users[0].is_superuser)
- self.assertEqual(new_email, users[0].email)
- self.assertTrue(users[0].check_password(new_password))
-
- def test_edit_user_POST(self):
- user = factory.make_user(username='user')
- user_id = user.id
- new_last_name = factory.getRandomString(30)
- new_admin_status = factory.getRandomBoolean()
- response = self.client.post(
- reverse('accounts-edit', args=['user']),
- {
- 'username': 'new_user',
- 'last_name': new_last_name,
- 'email': 'new_test@xxxxxxxxxxx',
- 'is_superuser': new_admin_status,
- })
- self.assertEqual(httplib.FOUND, response.status_code)
- users = list(User.objects.filter(username='new_user'))
- self.assertEqual(1, len(users))
- self.assertEqual(user_id, users[0].id)
- self.assertEqual(new_last_name, users[0].last_name)
- self.assertEqual(new_admin_status, users[0].is_superuser)
+ params = make_password_params(new_password)
+ response = self.client.post(
+ reverse('accounts-edit', args=[user.username]),
+ prefix_form_params(params, 'password'))
+ self.assertEqual(httplib.FOUND, response.status_code)
+ self.assertTrue(reload_object(user).check_password(new_password))
def test_delete_user_GET(self):
# The user delete page displays a confirmation page with a form.
=== modified file 'src/maasserver/views.py'
--- src/maasserver/views.py 2012-04-02 16:38:56 +0000
+++ src/maasserver/views.py 2012-04-03 04:54:18 +0000
@@ -38,7 +38,10 @@
)
from django.conf import settings as django_settings
from django.contrib import messages
-from django.contrib.auth.forms import PasswordChangeForm as PasswordForm
+from django.contrib.auth.forms import (
+ AdminPasswordChangeForm,
+ PasswordChangeForm,
+ )
from django.contrib.auth.models import User
from django.contrib.auth.views import (
login as dj_login,
@@ -63,6 +66,9 @@
ListView,
UpdateView,
)
+from django.views.generic.base import TemplateView
+from django.views.generic.detail import SingleObjectTemplateResponseMixin
+from django.views.generic.edit import ModelFormMixin
from maasserver.exceptions import (
CannotDeleteUserException,
NoRabbit,
@@ -267,7 +273,7 @@
# Process the password change form.
password_form, response = process_form(
- request, PasswordForm, reverse('prefs'), 'password',
+ request, PasswordChangeForm, reverse('prefs'), 'password',
"Password updated.", {'user': user})
if response is not None:
return response
@@ -282,6 +288,7 @@
class AccountsView(DetailView):
+ """Read-only view of user's account information."""
template_name = 'maasserver/user_view.html'
@@ -294,6 +301,7 @@
class AccountsAdd(CreateView):
+ """Add-user view."""
form_class = NewUserCreationForm
@@ -312,7 +320,6 @@
class AccountsDelete(DeleteView):
template_name = 'maasserver/user_confirm_delete.html'
-
context_object_name = 'user_to_delete'
def get_object(self):
@@ -334,19 +341,50 @@
return HttpResponseRedirect(self.get_next_url())
-class AccountsEdit(UpdateView):
-
- form_class = EditUserForm
-
+class AccountsEdit(TemplateView, ModelFormMixin,
+ SingleObjectTemplateResponseMixin):
+
+ model = User
template_name = 'maasserver/user_edit.html'
def get_object(self):
username = self.kwargs.get('username', None)
- user = get_object_or_404(User, username=username)
- return user
-
- def get_success_url(self):
- return reverse('settings')
+ return get_object_or_404(User, username=username)
+
+ def respond(self, request, profile_form, password_form):
+ """Generate a response."""
+ return self.render_to_response({
+ 'profile_form': profile_form,
+ 'password_form': password_form,
+ })
+
+ def get(self, request, *args, **kwargs):
+ """Called by `TemplateView`: handle a POST request."""
+ self.object = user = self.get_object()
+ profile_form = EditUserForm(instance=user, prefix='profile')
+ password_form = AdminPasswordChangeForm(user=user, prefix='password')
+ return self.respond(request, profile_form, password_form)
+
+ def post(self, request, *args, **kwargs):
+ """Called by `TemplateView`: handle a POST request."""
+ self.object = user = self.get_object()
+ next_page = reverse('settings')
+
+ # Process the profile-editing form, if that's what was submitted.
+ profile_form, response = process_form(
+ request, EditUserForm, next_page, 'profile', "Profile updated.",
+ {'instance': user})
+ if response is not None:
+ return response
+
+ # Process the password change form, if that's what was submitted.
+ password_form, response = process_form(
+ request, AdminPasswordChangeForm, next_page, 'password',
+ "Password updated.", {'user': user})
+ if response is not None:
+ return response
+
+ return self.respond(request, profile_form, password_form)
def proxy_to_longpoll(request):
