diff --git a/bugsink/urls.py b/bugsink/urls.py index 9eeeb7e..000d641 100644 --- a/bugsink/urls.py +++ b/bugsink/urls.py @@ -34,6 +34,9 @@ urlpatterns = [ path("accounts/preferences/", preferences, name="preferences"), + # many user-related views are directly exposed above (/accounts/), the rest is here: + path("users/", include("users.urls")), + path('api/', include('ingest.urls')), path('projects/', include('projects.urls')), diff --git a/theme/templates/base.html b/theme/templates/base.html index 495b8a4..ee3f2e3 100644 --- a/theme/templates/base.html +++ b/theme/templates/base.html @@ -31,6 +31,10 @@
Admin
{% endif %} + {% if user.is_superuser %} +
Users
+ {% endif %} + {% if logged_in_user.is_anonymous %}
Login
{# I don't think this is actually ever shown in practice, because you must always be logged in #} {% else %} diff --git a/users/forms.py b/users/forms.py index 090d837..b563ec9 100644 --- a/users/forms.py +++ b/users/forms.py @@ -26,7 +26,6 @@ UserModel = get_user_model() class UserCreationForm(BaseUserCreationForm): - # Our UserCreationForm is the place where the "use email for usernames" logic is implemented. # We could instead push such logic in the model, and do it more thoroughly (i.e. remove either field, and point the # USERNAME_FIELD to the other). But I'm not sure that this is the most future-proof way forward. In particular, @@ -82,6 +81,34 @@ class UserCreationForm(BaseUserCreationForm): return user +class UserEditForm(ModelForm): + # See notes in UserCreationForm about the "use email for usernames" logic; it's the same here. + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields['username'].validators = [EmailValidator()] + self.fields['username'].label = "Email" + + self.fields['username'].help_text = None # "Email" is descriptive enough + + class Meta: + model = UserModel + fields = ("username",) + + def clean_username(self): + if UserModel.objects.exclude(pk=self.instance.pk).filter(username=self.cleaned_data['username']).exists(): + raise ValidationError(mark_safe("This email is already registered by another user.")) + return self.cleaned_data['username'] + + def save(self, **kwargs): + commit = kwargs.pop("commit", True) + user = super().save(commit=False) + user.email = user.username + if commit: + user.save() + return user + + class ResendConfirmationForm(forms.Form): email = forms.EmailField() diff --git a/users/templates/users/reset_password.html b/users/templates/users/reset_password.html index efd798c..e41753e 100644 --- a/users/templates/users/reset_password.html +++ b/users/templates/users/reset_password.html @@ -22,7 +22,7 @@ - + diff --git a/users/templates/users/user_edit.html b/users/templates/users/user_edit.html new file mode 100644 index 0000000..1ab0c4d --- /dev/null +++ b/users/templates/users/user_edit.html @@ -0,0 +1,33 @@ +{% extends "base.html" %} +{% load static %} +{% load tailwind_forms %} + +{% block title %}Edit {{ form.instance.username }} · {{ site_title }}{% endblock %} + +{% block content %} + + +
+ +
+
+ {% csrf_token %} + +
+

{{ form.instance.username }}

+
+ +
+ Settings for "{{ form.instance.username }}". +
+ + {% tailwind_formfield form.username %} + + + Cancel +
+ +
+
+ +{% endblock %} diff --git a/users/templates/users/user_list.html b/users/templates/users/user_list.html new file mode 100644 index 0000000..b9d93d6 --- /dev/null +++ b/users/templates/users/user_list.html @@ -0,0 +1,86 @@ +{% extends "base.html" %} +{% load static %} + +{% block title %}Users · {{ site_title }}{% endblock %} + +{% block content %} + +
+ +
+ + {% if messages %} + + {% endif %} + +
+

Users

+ + {% comment %} + Our current invite-system is tied to either a team or a project; no "global" invites (yet). + + {% endcomment %} +
+ +
+
+ {% csrf_token %} + + + + + + + + + {% for user in users %} + + + + + + + {% endfor %} + + {#% empty %} not needed, a site without users cannot be visited by a user #} + +
Users
+
+ {{ user.username }} {# "best name" perhaps later? #} + {# Invitation pending #} {# perhaps useful for "not active"? #} + {% if member.is_superuser %} + Superuser + {% endif %} +
+
+
+ {% if not request.user == user %} + {% if user.is_active %} + + {% else %} + + {% endif %} + {% endif %} +
+
+ +
+
+ + {% comment %} +
+
+ Back to Xxxx {# perhaps once this is part of some other flow #} +
+
+ {% endcomment %} +
+ +{% endblock %} diff --git a/users/urls.py b/users/urls.py new file mode 100644 index 0000000..1c1e2a5 --- /dev/null +++ b/users/urls.py @@ -0,0 +1,8 @@ +from django.urls import path + +from .views import user_list, user_edit + +urlpatterns = [ + path('', user_list, name="user_list"), + path('/edit/', user_edit, name="user_edit"), +] diff --git a/users/views.py b/users/views.py index 833a9f8..d1c4615 100644 --- a/users/views.py +++ b/users/views.py @@ -6,12 +6,13 @@ from django.contrib.auth import get_user_model from django.http import Http404 from django.utils import timezone from django.contrib import messages -from django.contrib.auth.decorators import login_required +from django.contrib.auth.decorators import login_required, user_passes_test from bugsink.app_settings import get_settings, CB_ANYBODY from bugsink.decorators import atomic_for_request_method -from .forms import UserCreationForm, ResendConfirmationForm, RequestPasswordResetForm, SetPasswordForm, PreferencesForm +from .forms import ( + UserCreationForm, ResendConfirmationForm, RequestPasswordResetForm, SetPasswordForm, PreferencesForm, UserEditForm) from .models import EmailVerification from .tasks import send_confirm_email, send_reset_email @@ -19,6 +20,52 @@ from .tasks import send_confirm_email, send_reset_email UserModel = get_user_model() +@atomic_for_request_method +@user_passes_test(lambda u: u.is_superuser) +def user_list(request): + users = UserModel.objects.all().order_by('username') + + if request.method == 'POST': + full_action_str = request.POST.get('action') + action, user_pk = full_action_str.split(":", 1) + if action == "deactivate": + user = UserModel.objects.get(pk=user_pk) + user.is_active = False + user.save() + + messages.success(request, 'User %s deactivated' % user.username) + return redirect('user_list') + + if action == "activate": + user = UserModel.objects.get(pk=user_pk) + user.is_active = True + user.save() + + messages.success(request, 'User %s activated' % user.username) + return redirect('user_list') + + return render(request, 'users/user_list.html', { + 'users': users, + }) + + +@atomic_for_request_method +@user_passes_test(lambda u: u.is_superuser) +def user_edit(request, user_pk): + user = UserModel.objects.get(pk=user_pk) + if request.method == 'POST': + form = UserEditForm(request.POST, instance=user) + + if form.is_valid(): + form.save() + return redirect("user_list") + + else: + form = UserEditForm(instance=user) + + return render(request, "users/user_edit.html", {"form": form}) + + @atomic_for_request_method def signup(request): if get_settings().USER_REGISTRATION != CB_ANYBODY: @@ -62,7 +109,7 @@ def confirm_email(request, token=None): if request.method == 'POST': # We insist on POST requests to do the actual confirmation (at the cost of an extra click). See: # https://softwareengineering.stackexchange.com/a/422579/168778 - # there's no form, the'res just a button to generate the post request + # there's no Django form (fields), there's just a button to generate the post request verification.user.is_active = True verification.user.save() @@ -159,8 +206,8 @@ def reset_password(request, token=None): @atomic_for_request_method -# in the general case this is done by Middleware but we're under /accounts/. not security-critical because we simply -# get a failure on request.user if this wasn't there, but still the "right thing" +# in the general case this is done by Middleware but we're under /accounts/, so we need it back. +# not security-critical because we simply get a failure on request.user if this wasn't there, but still the right thing. @login_required def preferences(request): user = request.user