Skip to content

Commit

Permalink
Enable admins to activate users
Browse files Browse the repository at this point in the history
Add an "Activate" button next to users on the /admin/users page so that
admins can activate user accounts.

Some refactoring to avoid code duplication between activating by users
clicking on an activation link and admins clicking the new activate button:
move the code that actually activates the user onto the User model where
both views can call it.
  • Loading branch information
seanh committed Feb 24, 2016
1 parent 4044d21 commit 8f43c6b
Show file tree
Hide file tree
Showing 8 changed files with 182 additions and 33 deletions.
6 changes: 6 additions & 0 deletions h/accounts/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from sqlalchemy.ext.hybrid import hybrid_property
from sqlalchemy.sql import expression

from h.accounts import events
from h.db import Base
from h._compat import text_type

Expand Down Expand Up @@ -132,6 +133,11 @@ def is_activated(self):

return False

def activate(self, request):
"""Activate the user by deleting any activation they have."""
request.db.delete(self.activation)
request.registry.notify(events.ActivationEvent(request, self))

# Hashed password
_password = sa.Column('password', sa.UnicodeText(), nullable=False)
# Password salt
Expand Down
60 changes: 60 additions & 0 deletions h/accounts/test/models_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

import re

import mock
from pyramid import testing
import pytest
from sqlalchemy import exc

Expand Down Expand Up @@ -132,3 +134,61 @@ def test_staff_members_does_not_return_non_staff_users():

for non_staff in non_staff:
assert non_staff not in staff


activate_fixtures = pytest.mark.usefixtures('events')


def mock_request():
"""Return a mock request object for User.activate() tests."""
class DBSpec(object):
def delete(self, activation):
pass

request = testing.DummyRequest(db=mock.Mock(spec=DBSpec))

def notify_spec(event):
pass

request.registry.notify = mock.Mock(spec=notify_spec)

return request


@activate_fixtures
def test_User_activate_deletes_activation():
request = mock_request()
user = factories.User()

user.activate(request)

request.db.delete.assert_called_once_with(user.activation)


@activate_fixtures
def test_User_activate_inits_ActivationEvent(events):
request = mock_request()
user = factories.User()

user.activate(request)

events.ActivationEvent.assert_called_once_with(request, user)


@activate_fixtures
def test_User_activate_notifies(events):
request = mock_request()
user = factories.User()

user.activate(request)

request.registry.notify.assert_called_once_with(
events.ActivationEvent.return_value)


@pytest.fixture
def events(request):
patcher = mock.patch('h.accounts.models.events', autospec=True)
module = patcher.start()
request.addfinalizer(patcher.stop)
return module
33 changes: 4 additions & 29 deletions h/accounts/test/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -687,19 +687,17 @@ def test_get_when_not_logged_in_404s_if_user_id_does_not_match_hash(
with pytest.raises(httpexceptions.HTTPNotFound):
views.ActivateController(request).get_when_not_logged_in()

def test_get_when_not_logged_in_successful_deletes_activation(
self,
user_model,
activation_model):
def test_get_when_not_logged_in_successful_activates_user(self,
user_model):
request = DummyRequest(matchdict={'id': '123', 'code': 'abc456'})
request.db.delete = mock.create_autospec(request.db.delete,
return_value=None)
user_model.get_by_activation.return_value.id = 123

views.ActivateController(request).get_when_not_logged_in()

request.db.delete.assert_called_once_with(
activation_model.get_by_code.return_value)
user_model.get_by_activation.return_value.activate.assert_called_once_with(
request)

def test_get_when_not_logged_in_successful_flashes_message(self,
user_model):
Expand All @@ -713,29 +711,6 @@ def test_get_when_not_logged_in_successful_flashes_message(self,
assert request.session.flash.call_args[0][0].startswith(
"Your account has been activated")

def test_get_when_not_logged_in_successful_creates_ActivationEvent(
self,
user_model,
ActivationEvent):
request = DummyRequest(matchdict={'id': '123', 'code': 'abc456'})
user_model.get_by_activation.return_value.id = 123

views.ActivateController(request).get_when_not_logged_in()

ActivationEvent.assert_called_once_with(
request, user_model.get_by_activation.return_value)

def test_get_when_not_logged_in_successful_notifies(self,
user_model,
notify,
ActivationEvent):
request = DummyRequest(matchdict={'id': '123', 'code': 'abc456'})
user_model.get_by_activation.return_value.id = 123

views.ActivateController(request).get_when_not_logged_in()

notify.assert_called_once_with(ActivationEvent.return_value)

def test_get_when_logged_in_already_logged_in_when_id_not_an_int(self):
request = DummyRequest(matchdict={
'id': 'abc', # Not an int.
Expand Down
4 changes: 1 addition & 3 deletions h/accounts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,15 +422,13 @@ def get_when_not_logged_in(self):
if user is None or user.id != id_:
raise httpexceptions.HTTPNotFound()

# Activate the user (by deleting the activation)
self.request.db.delete(activation)
user.activate(self.request)

self.request.session.flash(jinja2.Markup(_(
'Your account has been activated! '
'You can now <a href="{url}">sign in</a> using the password you '
'provided.').format(url=self.request.route_url('login'))),
'success')
self.request.registry.notify(ActivationEvent(self.request, user))

return httpexceptions.HTTPFound(
location=self.request.route_url('index'))
Expand Down
28 changes: 28 additions & 0 deletions h/admin.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
import jinja2
from pyramid import session
from pyramid import view
from pyramid import httpexceptions
Expand Down Expand Up @@ -195,6 +196,32 @@ def users_index(request):
return {'username': username, 'user': user, 'user_meta': user_meta}


@view.view_config(route_name='admin_users_activate',
request_method='POST',
request_param='username',
permission='admin_users')
def users_activate(request):
username = request.params['username']
user = models.User.get_by_username(username)

if user is None:
request.session.flash(jinja2.Markup(_(
"User {name} doesn't exist!".format(name=username))),
'error')
return httpexceptions.HTTPFound(
location=request.route_path('admin_users'))

user.activate(request)

request.session.flash(jinja2.Markup(_(
'User {name} has been activated!'.format(name=user.username))),
'success')

return httpexceptions.HTTPFound(
location=request.route_path('admin_users',
_query=(('username', user.username),)))


@view.view_config(route_name='admin_users_delete',
request_method='POST',
permission='admin_users')
Expand Down Expand Up @@ -320,6 +347,7 @@ def includeme(config):
config.add_route('admin_admins', '/admins')
config.add_route('admin_staff', '/staff')
config.add_route('admin_users', '/users')
config.add_route('admin_users_activate', '/users/activate')
config.add_route('admin_users_delete', '/users/delete')
config.add_route('admin_groups', '/groups')
config.add_route('admin_groups_csv', '/groups.csv')
Expand Down
4 changes: 4 additions & 0 deletions h/static/styles/admin.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ body {
table-layout: auto;
width: auto;
}

.users-activate-form {
display: inline;
}
18 changes: 17 additions & 1 deletion h/templates/admin/users.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,23 @@
<tr><th>Last login</th><td>{{ user.last_login_date }}</td></tr>
<tr>
<th>Is activated?</th>
<td>{% if user.is_activated %}&#x2714;{% else %}&#x2718;{% endif %}</td>
<td>
{% if user.is_activated %}
&#x2714;
{% else %}
&#x2718;
<form action="{{request.route_path('admin_users_activate')}}"
class="users-activate-form"
method="POST">
<input type="hidden"
name="username"
value="{{user.username}}">
<button class="btn btn-primary btn-xs" type="submit">
Activate
</button>
</form>
{% endif %}
</td>
</tr>
<tr>
<th>Is admin?</th>
Expand Down
62 changes: 62 additions & 0 deletions h/test/admin_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,68 @@ def test_users_index_user_found(User):
}


users_activate_fixtures = pytest.mark.usefixtures('routes_mapper', 'User')


@users_activate_fixtures
def test_users_activate_gets_user(User):
request = DummyRequest(params={"username": "bob"})

admin.users_activate(request)

User.get_by_username.assert_called_once_with("bob")


@users_activate_fixtures
def test_users_activate_flashes_error_if_no_user(User):
request = DummyRequest(params={"username": "bob"})
request.session.flash = Mock()
User.get_by_username.return_value = None

admin.users_activate(request)

assert request.session.flash.call_count == 1
assert request.session.flash.call_args[0][1] == 'error'


@users_activate_fixtures
def test_users_activate_redirects_if_no_user(User):
request = DummyRequest(params={"username": "bob"})
User.get_by_username.return_value = None

result = admin.users_activate(request)

assert isinstance(result, httpexceptions.HTTPFound)


@users_activate_fixtures
def test_users_activate_activates_user(User):
request = DummyRequest(params={"username": "bob"})

admin.users_activate(request)

User.get_by_username.return_value.activate.assert_called_once_with(request)


@users_activate_fixtures
def test_users_activate_flashes_success(User):
request = DummyRequest(params={"username": "bob"})
request.session.flash = Mock()

admin.users_activate(request)

assert request.session.flash.call_count == 1
assert request.session.flash.call_args[0][1] == 'success'


@users_activate_fixtures
def test_users_activate_redirects(User):
request = DummyRequest(params={"username": "bob"})

result = admin.users_activate(request)

assert isinstance(result, httpexceptions.HTTPFound)

users_delete_fixtures = pytest.mark.usefixtures('routes_mapper', 'User',
'delete_user')

Expand Down

0 comments on commit 8f43c6b

Please sign in to comment.