diff --git a/opentech/apply/users/forms.py b/opentech/apply/users/forms.py index 174d827ba6d736dc059fe951ec9699256df9506c..88e7bdbc44a63114c5929a66055b805d56fc51b8 100644 --- a/opentech/apply/users/forms.py +++ b/opentech/apply/users/forms.py @@ -1,8 +1,12 @@ from django import forms +from django.contrib.auth import get_user_model from wagtail.users.forms import UserEditForm, UserCreationForm +User = get_user_model() + + class CustomUserAdminFormBase(): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -18,3 +22,26 @@ class CustomUserEditForm(CustomUserAdminFormBase, UserEditForm): class CustomUserCreationForm(CustomUserAdminFormBase, UserCreationForm): pass + + +class ProfileForm(forms.ModelForm): + class Meta: + model = User + fields = ['full_name', 'email', 'slack'] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if not self.instance.is_apply_staff: + del self.fields['slack'] + + def clean_slack(self): + slack = self.cleaned_data['slack'] + if slack: + slack = slack.strip() + if ' ' in slack: + raise forms.ValidationError('Slack names must not include spaces') + + if not slack.startswith('@'): + slack = '@' + slack + + return slack diff --git a/opentech/apply/users/migrations/0007_user_slack.py b/opentech/apply/users/migrations/0007_user_slack.py new file mode 100644 index 0000000000000000000000000000000000000000..177e18168a780bfa9cb95f21b84f57a66a49f4f7 --- /dev/null +++ b/opentech/apply/users/migrations/0007_user_slack.py @@ -0,0 +1,18 @@ +# Generated by Django 2.0.2 on 2018-07-18 16:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('users', '0006_update_the_ordering_of_users'), + ] + + operations = [ + migrations.AddField( + model_name='user', + name='slack', + field=models.CharField(blank=True, help_text='This is the name we should "@mention" when sending notifications', max_length=50, verbose_name='Slack name'), + ), + ] diff --git a/opentech/apply/users/models.py b/opentech/apply/users/models.py index 16b87687b11ec6aad42a805da9a799e56dd5df99..62e753ef3dc3fbf270a0873add6ebc4364477ded 100644 --- a/opentech/apply/users/models.py +++ b/opentech/apply/users/models.py @@ -56,6 +56,12 @@ class UserManager(BaseUserManager.from_queryset(UserQuerySet)): class User(AbstractUser): email = models.EmailField(_('email address'), unique=True) full_name = models.CharField(verbose_name='Full name', max_length=255, blank=True) + slack = models.CharField( + verbose_name='Slack name', + blank=True, + help_text='This is the name we should "@mention" when sending notifications', + max_length=50, + ) # Meta: used for migration purposes only drupal_id = models.IntegerField(null=True, blank=True, editable=False) diff --git a/opentech/apply/users/templates/users/account.html b/opentech/apply/users/templates/users/account.html index 6064bb0670e7036fa21cc62cf863226cace4d7d0..3177476547d1df21cd0c82c377be0603ca29504c 100644 --- a/opentech/apply/users/templates/users/account.html +++ b/opentech/apply/users/templates/users/account.html @@ -11,11 +11,25 @@ </div> </div> +<div class="wrapper wrapper--large wrapper--inner-space-medium"> + <h3>Profile</h3> + <form action="" method="post" class="form"> + {% csrf_token %} + {% for field in form %} + {% include "funds/includes/field.html" %} + {% endfor %} + <button class="button button--primary" type="submit">{% trans "Update Profile" %}</button> + </form> +</div> + <div class="wrapper wrapper--large wrapper--inner-space-medium"> {% if show_change_password and user.has_usable_password and not backends.associated %} - <a href="{% url 'users:password_change' %}">{% trans "Change password" %}</a> + <h3>Change password</h3> + <a class="button button--primary" href="{% url 'users:password_change' %}">{% trans "Update password" %}</a> {% endif %} +</div> +<div class="wrapper wrapper--large wrapper--inner-space-medium"> {% if swappable %} <form action="{% url 'users:become' %}" method="post" class="form"> {% csrf_token %} diff --git a/opentech/apply/users/templates/users/change_password.html b/opentech/apply/users/templates/users/change_password.html index 2903587ae2bd825dd42a4e5683f0d5ee7ac7a17a..ba6419db4311229e6748f2e098ded299657fe6cc 100644 --- a/opentech/apply/users/templates/users/change_password.html +++ b/opentech/apply/users/templates/users/change_password.html @@ -1,40 +1,37 @@ -{% extends 'base.html' %} +{% extends 'base-apply.html' %} {% load i18n %} {% block header_modifier %}header--light-bg{% endblock %} {% block page_title %}Set a password{% endblock %} {% block title %}{% trans "Set a password" %}{% endblock %} {% block content %} -<div class="wrapper wrapper--small wrapper--bottom-space"> +<div class="wrapper wrapper--small wrapper--inner-space-medium"> + <form class="form" action="" method="POST" novalidate> + {% if form.non_field_errors %} + <ul class="errorlist"> + {% for error in form.non_field_errors %} + <li>{{ error }}</li> + {% endfor %} + </ul> + {% endif %} - {% if form.non_field_errors %} - <ul class="errorlist"> - {% for error in form.non_field_errors %} - <li>{{ error }}</li> - {% endfor %} - </ul> - {% endif %} + {% if form.errors %} + <ul class="errorlist"> + {% blocktrans count counter=form.errors.items|length %} + <li>Please correct the error below.</li> + {% plural %} + <li>Please correct the errors below.</li> + {% endblocktrans %} + </ul> + {% endif %} - {% if form.errors %} - <ul class="errorlist"> - {% blocktrans count counter=form.errors.items|length %} - <li>Please correct the error below.</li> - {% plural %} - <li>Please correct the errors below.</li> - {% endblocktrans %} - </ul> - {% endif %} - - <form class="form form--with-p-tags" action="" method="POST" novalidate> {% csrf_token %} {% for field in form %} - {{ field.errors }} - <p>{{ field.label_tag }}</p> - <p>{{ field }}</p> + {% include "funds/includes/field.html" %} {% endfor %} - <button class="link link--button-secondary" type="submit">{% trans 'Reset password' %}</button> + <button class="button button--primary" type="submit">{% trans 'Reset password' %}</button> </form> </div> {% endblock %} diff --git a/opentech/apply/users/tests/factories.py b/opentech/apply/users/tests/factories.py index e0aa38ba2f713f4f81a55ccf51b13e7bc5be069b..bbeead33536945666cad7f8c4a5955d8f35c99f4 100644 --- a/opentech/apply/users/tests/factories.py +++ b/opentech/apply/users/tests/factories.py @@ -20,6 +20,7 @@ class UserFactory(factory.DjangoModelFactory): email = factory.Sequence('email{}@email.com'.format) full_name = factory.Faker('name') + password = factory.PostGenerationMethodCall('set_password', 'defaultpassword') @factory.post_generation def groups(self, create, extracted, **kwargs): @@ -32,6 +33,10 @@ class UserFactory(factory.DjangoModelFactory): self.groups.add(groups) +class OAuthUserFactory(UserFactory): + password = factory.PostGenerationMethodCall('set_unusable_password') + + class AdminFactory(UserFactory): is_admin = True diff --git a/opentech/apply/users/tests/test_forms.py b/opentech/apply/users/tests/test_forms.py new file mode 100644 index 0000000000000000000000000000000000000000..03860d17eeba33dca8ee4453aaab88c05c56a083 --- /dev/null +++ b/opentech/apply/users/tests/test_forms.py @@ -0,0 +1,85 @@ +from django.forms.models import model_to_dict +from django.test import TestCase + +from ..forms import ProfileForm +from .factories import StaffFactory, UserFactory + + +class BaseTestProfileForm(TestCase): + def form_data(self, user, **values): + fields = ProfileForm.Meta.fields + data = model_to_dict(user, fields) + data.update(**values) + return data + + def submit_form(self, instance, **extra_data): + form = ProfileForm(instance=instance, data=self.form_data(instance, **extra_data)) + if form.is_valid(): + form.save() + + return form + + +class TestProfileForm(BaseTestProfileForm): + def setUp(self): + self.user = UserFactory() + + def test_email_unique(self): + other_user = UserFactory() + form = self.submit_form(self.user, email=other_user.email) + self.assertFalse(form.is_valid()) + self.user.refresh_from_db() + self.assertNotEqual(self.user.email, other_user.email) + + def test_can_change_email(self): + new_email = 'me@another.com' + self.submit_form(self.user, email=new_email) + self.user.refresh_from_db() + self.assertEqual(self.user.email, new_email) + + def test_cant_set_slack_name(self): + slack_name = '@foobar' + self.submit_form(self.user, slack=slack_name) + self.user.refresh_from_db() + self.assertNotEqual(self.user.slack, slack_name) + + +class TestStaffProfileForm(BaseTestProfileForm): + def setUp(self): + self.staff = StaffFactory() + + def test_can_set_slack_name(self): + slack_name = '@foobar' + self.submit_form(self.staff, slack=slack_name) + + self.staff.refresh_from_db() + self.assertEqual(self.staff.slack, slack_name) + + def test_can_set_slack_name_with_trailing_space(self): + slack_name = '@foobar' + self.submit_form(self.staff, slack=slack_name) + + self.staff.refresh_from_db() + self.assertEqual(self.staff.slack, slack_name) + + def test_cant_set_slack_name_with_space(self): + slack_name = '@ foobar' + form = self.submit_form(self.staff, slack=slack_name) + self.assertFalse(form.is_valid()) + + self.staff.refresh_from_db() + self.assertNotEqual(self.staff.slack, slack_name) + + def test_auto_prepend_at(self): + slack_name = 'foobar' + self.submit_form(self.staff, slack=slack_name) + + self.staff.refresh_from_db() + self.assertEqual(self.staff.slack, '@' + slack_name) + + def test_can_clear_slack_name(self): + slack_name = '' + self.submit_form(self.staff, slack=slack_name) + + self.staff.refresh_from_db() + self.assertEqual(self.staff.slack, slack_name) diff --git a/opentech/apply/users/tests/test_views.py b/opentech/apply/users/tests/test_views.py new file mode 100644 index 0000000000000000000000000000000000000000..935dcdc981e7301ad4f28cd84a204fb775c4ee3e --- /dev/null +++ b/opentech/apply/users/tests/test_views.py @@ -0,0 +1,45 @@ +from django.test import TestCase +from django.urls import reverse + +from .factories import OAuthUserFactory, StaffFactory, UserFactory + + +class BaseTestProfielView(TestCase): + @classmethod + def setUpTestData(cls): + cls.url = reverse('users:account') + + +class TestProfileView(BaseTestProfielView): + def setUp(self): + self.user = UserFactory() + self.client.force_login(self.user) + + def test_cant_acces_if_not_logged_in(self): + self.client.logout() + response = self.client.get(self.url, follow=True) + # Initial redirect will be via to https through a 301 + self.assertRedirects(response, reverse('users:login') + '?next=' + self.url, status_code=301) + + def test_includes_change_password(self): + response = self.client.get(self.url, follow=True) + self.assertContains(response, reverse('users:password_change')) + + def test_doesnt_includes_change_password_for_oauth(self): + self.client.force_login(OAuthUserFactory()) + response = self.client.get(self.url, follow=True) + self.assertNotContains(response, reverse('users:password_change')) + + def test_cant_set_slack_name(self): + response = self.client.get(self.url, follow=True) + self.assertNotContains(response, 'Slack name') + + +class TestStaffProfileView(BaseTestProfielView): + def setUp(self): + self.staff = StaffFactory() + self.client.force_login(self.staff) + + def test_can_set_slack_name(self): + response = self.client.get(self.url, follow=True) + self.assertContains(response, 'Slack name') diff --git a/opentech/apply/users/urls.py b/opentech/apply/users/urls.py index 30dc4c0206e371f3a82583da74df77f43d446070..ddb5fb2634500e85f9f4382dba4354aabbaf32ef 100644 --- a/opentech/apply/users/urls.py +++ b/opentech/apply/users/urls.py @@ -2,13 +2,13 @@ from django.urls import path from django.contrib.auth import views as auth_views from django.urls import reverse_lazy -from opentech.apply.users.views import account, become, oauth, ActivationView, create_password +from opentech.apply.users.views import AccountView, become, oauth, ActivationView, create_password app_name = 'users' urlpatterns = [ - path('', account, name='account'), + path('', AccountView.as_view(), name='account'), path('become/', become, name='become'), path( 'login/', diff --git a/opentech/apply/users/views.py b/opentech/apply/users/views.py index 7669165d9879dc243b4c841f0bd2cf1bc2493671..8f8e3dab59ca43446f461283062957c5c640f132 100644 --- a/opentech/apply/users/views.py +++ b/opentech/apply/users/views.py @@ -6,8 +6,10 @@ from django.contrib.auth.tokens import PasswordResetTokenGenerator from django.shortcuts import redirect, render from django.template.response import TemplateResponse from django.urls import reverse_lazy +from django.utils.decorators import method_decorator from django.utils.encoding import force_text from django.utils.http import urlsafe_base64_decode +from django.views.generic import UpdateView from django.views.generic.base import TemplateView from hijack.views import login_with_id @@ -15,22 +17,36 @@ from hijack.views import login_with_id from wagtail.admin.views.account import password_management_enabled from .decorators import require_oauth_whitelist +from .forms import ProfileForm + User = get_user_model() -@login_required(login_url=reverse_lazy('users:login')) -def account(request): - """Account page placeholder view""" - if request.user.is_superuser: - swappable = User.objects.filter(is_active=True, is_superuser=False) - else: - swappable = [] +@method_decorator(login_required, name='dispatch') +class AccountView(UpdateView): + form_class = ProfileForm + template_name = 'users/account.html' - return render(request, 'users/account.html', { - 'show_change_password': password_management_enabled() and request.user.has_usable_password(), - 'swappable': swappable, - }) + def get_object(self): + return self.request.user + + def get_success_url(self,): + return reverse_lazy('users:account') + + def get_context_data(self, **kwargs): + if self.request.user.is_superuser: + swappable = User.objects.filter(is_active=True, is_superuser=False) + else: + swappable = [] + + show_change_password = password_management_enabled() and self.request.user.has_usable_password(), + + return super().get_context_data( + swappable=swappable, + show_change_password=show_change_password, + **kwargs, + ) @login_required(login_url=reverse_lazy('users:login')) diff --git a/opentech/settings/production.py b/opentech/settings/production.py index 410aff444a003c4c5869931637dabb8cf3635864..131e636384ec8591fa81ad050380abb9a9629fd7 100644 --- a/opentech/settings/production.py +++ b/opentech/settings/production.py @@ -11,12 +11,6 @@ from .base import * # noqa # Disable debug mode DEBUG = False -# Raven (sentry) configuration. See local settings for DSN - -INSTALLED_APPS += ( - 'raven.contrib.django.raven_compat', -) - # Cache everything for 10 minutes # This only applies to pages that do not have a more specific cache-control @@ -29,6 +23,16 @@ CACHE_CONTROL_MAX_AGE = 600 env = os.environ.copy() +# Raven (sentry) configuration. See local settings for DSN + +if 'SENTRY_DSN' in env: + INSTALLED_APPS += ( + 'raven.contrib.django.raven_compat', + ) + + SENTRY_DSN = env['SENTRY_DSN'] + + # On Torchbox servers, many environment variables are prefixed with "CFG_" for key, value in os.environ.items(): if key.startswith('CFG_'): diff --git a/requirements.txt b/requirements.txt index 7c0bdc343e2fcdfa710e513beed3cd7b5ea4f354..c70c8301fb265f72ceedb8649edfec45da1ac284 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,4 +29,4 @@ dj-database-url==0.4.1 whitenoise==2.0.4 uwsgi==2.0.15 ConcurrentLogHandler==0.9.1 -raven==6.1.0 +raven==6.9.0