From 9640d2242f9e868f765ed8c6e1b05b86385f6aa1 Mon Sep 17 00:00:00 2001 From: swoga Date: Sat, 24 Jul 2021 18:38:59 +0200 Subject: [PATCH] feat: add manager role --- hc/accounts/forms.py | 4 +- .../migrations/0043_add_role_manager.py | 17 +++++++ hc/accounts/models.py | 6 +-- hc/accounts/tests/test_project.py | 45 +++++++++++++++---- hc/accounts/views.py | 13 ++++-- hc/test.py | 2 +- templates/accounts/project.html | 25 ++++++++--- 7 files changed, 87 insertions(+), 25 deletions(-) create mode 100644 hc/accounts/migrations/0043_add_role_manager.py diff --git a/hc/accounts/forms.py b/hc/accounts/forms.py index 16f3f336..397b8320 100644 --- a/hc/accounts/forms.py +++ b/hc/accounts/forms.py @@ -6,7 +6,7 @@ from django import forms from django.core.exceptions import ValidationError from django.contrib.auth import authenticate from django.contrib.auth.models import User -from hc.accounts.models import REPORT_CHOICES +from hc.accounts.models import REPORT_CHOICES, Member from hc.api.models import TokenBucket import pytz @@ -136,7 +136,7 @@ class ChangeEmailForm(forms.Form): class InviteTeamMemberForm(forms.Form): email = LowercaseEmailField(max_length=254) - rw = forms.BooleanField(required=False) + role = forms.ChoiceField(choices=Member.Role.choices) class RemoveTeamMemberForm(forms.Form): diff --git a/hc/accounts/migrations/0043_add_role_manager.py b/hc/accounts/migrations/0043_add_role_manager.py new file mode 100644 index 00000000..ea9e2a6d --- /dev/null +++ b/hc/accounts/migrations/0043_add_role_manager.py @@ -0,0 +1,17 @@ +from django.db import migrations, models +from hc.accounts.models import Member + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0042_remove_member_rw'), + ] + + operations = [ + migrations.AlterField( + model_name='member', + name='role', + field=models.CharField(choices=Member.Role.choices, default=Member.Role.REGULAR, max_length=1), + ), + ] diff --git a/hc/accounts/models.py b/hc/accounts/models.py index 17b7ecbe..67afdd6c 100644 --- a/hc/accounts/models.py +++ b/hc/accounts/models.py @@ -351,14 +351,13 @@ class Project(models.Model): used = q.distinct().count() return used < self.owner_profile.team_limit - def invite(self, user, rw): + def invite(self, user, role): if Member.objects.filter(user=user, project=self).exists(): return False if self.owner_id == user.id: return False - role = Member.Role.REGULAR if rw else Member.Role.READONLY Member.objects.create(user=user, project=self, role=role) checks_url = reverse("hc-checks", args=[self.code]) user.profile.send_instant_login_link(self, redirect_url=checks_url) @@ -423,6 +422,7 @@ class Member(models.Model): class Role(models.TextChoices): READONLY = "r", "Read-only" REGULAR = "w", "Member" + MANAGER = "m", "Manager" user = models.ForeignKey(User, models.CASCADE, related_name="memberships") project = models.ForeignKey(Project, models.CASCADE) @@ -441,7 +441,7 @@ class Member(models.Model): @property def is_rw(self): - return self.role in (Member.Role.REGULAR,) + return self.role in (Member.Role.REGULAR, Member.Role.MANAGER) class Credential(models.Model): diff --git a/hc/accounts/tests/test_project.py b/hc/accounts/tests/test_project.py index 9f9d3fd6..2152ecac 100644 --- a/hc/accounts/tests/test_project.py +++ b/hc/accounts/tests/test_project.py @@ -66,7 +66,7 @@ class ProjectTestCase(BaseTestCase): def test_it_adds_team_member(self): self.client.login(username="alice@example.org", password="password") - form = {"invite_team_member": "1", "email": "frank@example.org", "rw": "1"} + form = {"invite_team_member": "1", "email": "frank@example.org", "role": "w"} r = self.client.post(self.url, form) self.assertEqual(r.status_code, 200) @@ -90,7 +90,7 @@ class ProjectTestCase(BaseTestCase): def test_it_adds_readonly_team_member(self): self.client.login(username="alice@example.org", password="password") - form = {"invite_team_member": "1", "email": "frank@example.org"} + form = {"invite_team_member": "1", "email": "frank@example.org", "role": "r"} r = self.client.post(self.url, form) self.assertEqual(r.status_code, 200) @@ -100,6 +100,20 @@ class ProjectTestCase(BaseTestCase): self.assertEqual(member.role, member.Role.READONLY) + def test_it_adds_manager_team_member(self): + self.client.login(username="alice@example.org", password="password") + + form = {"invite_team_member": "1", "email": "frank@example.org", "role": "m"} + r = self.client.post(self.url, form) + self.assertEqual(r.status_code, 200) + + member = Member.objects.get( + project=self.project, user__email="frank@example.org" + ) + + # The new user should have role manager + self.assertEqual(member.role, member.Role.MANAGER) + def test_it_adds_member_from_another_team(self): # With team limit at zero, we should not be able to invite any new users self.profile.team_limit = 0 @@ -111,7 +125,7 @@ class ProjectTestCase(BaseTestCase): Member.objects.create(user=self.charlie, project=p2) self.client.login(username="alice@example.org", password="password") - form = {"invite_team_member": "1", "email": "charlie@example.org"} + form = {"invite_team_member": "1", "email": "charlie@example.org", "role": "r"} r = self.client.post(self.url, form) self.assertEqual(r.status_code, 200) @@ -125,7 +139,7 @@ class ProjectTestCase(BaseTestCase): def test_it_rejects_duplicate_membership(self): self.client.login(username="alice@example.org", password="password") - form = {"invite_team_member": "1", "email": "bob@example.org"} + form = {"invite_team_member": "1", "email": "bob@example.org", "role": "r"} r = self.client.post(self.url, form) self.assertContains(r, "bob@example.org is already a member") @@ -135,7 +149,7 @@ class ProjectTestCase(BaseTestCase): def test_it_rejects_owner_as_a_member(self): self.client.login(username="alice@example.org", password="password") - form = {"invite_team_member": "1", "email": "alice@example.org"} + form = {"invite_team_member": "1", "email": "alice@example.org", "role": "r"} r = self.client.post(self.url, form) self.assertContains(r, "alice@example.org is already a member") @@ -146,7 +160,7 @@ class ProjectTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") aaa = "a" * 300 - form = {"invite_team_member": "1", "email": f"frank+{aaa}@example.org"} + form = {"invite_team_member": "1", "email": f"frank+{aaa}@example.org", "role": "r"} r = self.client.post(self.url, form) self.assertEqual(r.status_code, 200) @@ -161,7 +175,7 @@ class ProjectTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") - form = {"invite_team_member": "1", "email": "frank@example.org"} + form = {"invite_team_member": "1", "email": "frank@example.org", "role": "r"} r = self.client.post(self.url, form) self.assertContains(r, "Too Many Requests") @@ -170,7 +184,7 @@ class ProjectTestCase(BaseTestCase): def test_it_requires_owner_to_add_team_member(self): self.client.login(username="bob@example.org", password="password") - form = {"invite_team_member": "1", "email": "frank@example.org"} + form = {"invite_team_member": "1", "email": "frank@example.org", "role": "r"} r = self.client.post(self.url, form) self.assertEqual(r.status_code, 403) @@ -180,7 +194,7 @@ class ProjectTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") - form = {"invite_team_member": "1", "email": "frank@example.org"} + form = {"invite_team_member": "1", "email": "frank@example.org", "role": "r"} r = self.client.post(self.url, form) self.assertEqual(r.status_code, 403) @@ -200,6 +214,19 @@ class ProjectTestCase(BaseTestCase): r = self.client.post(self.url, form) self.assertEqual(r.status_code, 403) + def test_it_rejects_manager_remove_self(self): + self.bobs_membership.role = "m" + self.bobs_membership.save() + + self.client.login(username="bob@example.org", password="password") + + form = {"remove_team_member": "1", "email": "bob@example.org"} + r = self.client.post(self.url, form) + self.assertEqual(r.status_code, 400) + + # The number of memberships should have not decreased + self.assertEqual(self.project.member_set.count(), 1) + def test_it_checks_membership_when_removing_team_member(self): self.client.login(username="charlie@example.org", password="password") diff --git a/hc/accounts/views.py b/hc/accounts/views.py index 7cccb0ff..021ac6b7 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -284,9 +284,11 @@ def project(request, code): is_owner = project.owner_id == request.user.id if request.user.is_superuser or is_owner: + is_manager = True rw = True else: membership = get_object_or_404(Member, project=project, user=request.user) + is_manager = membership.role == Member.Role.MANAGER rw = membership.is_rw ctx = { @@ -294,6 +296,7 @@ def project(request, code): "rw": rw, "project": project, "is_owner": is_owner, + "is_manager": is_manager, "show_api_keys": "show_api_keys" in request.GET, "enable_prometheus": settings.PROMETHEUS_ENABLED is True, } @@ -319,7 +322,7 @@ def project(request, code): elif "show_api_keys" in request.POST: ctx["show_api_keys"] = True elif "invite_team_member" in request.POST: - if not is_owner: + if not is_manager: return HttpResponseForbidden() form = forms.InviteTeamMemberForm(request.POST) @@ -341,7 +344,7 @@ def project(request, code): except User.DoesNotExist: user = _make_user(email, with_project=False) - if project.invite(user, rw=form.cleaned_data["rw"]): + if project.invite(user, role=form.cleaned_data["role"]): ctx["team_member_invited"] = email ctx["team_status"] = "success" else: @@ -349,7 +352,7 @@ def project(request, code): ctx["team_status"] = "info" elif "remove_team_member" in request.POST: - if not is_owner: + if not is_manager: return HttpResponseForbidden() form = forms.RemoveTeamMemberForm(request.POST) @@ -361,6 +364,9 @@ def project(request, code): if farewell_user is None: return HttpResponseBadRequest() + if farewell_user == request.user: + return HttpResponseBadRequest() + Member.objects.filter(project=project, user=farewell_user).delete() ctx["team_member_removed"] = form.cleaned_data["email"] @@ -428,6 +434,7 @@ def project(request, code): project.save() ctx["is_owner"] = True + ctx["is_manager"] = True messages.success(request, "You are now the owner of this project!") elif "reject_transfer" in request.POST: diff --git a/hc/test.py b/hc/test.py index c5531755..1e2239b4 100644 --- a/hc/test.py +++ b/hc/test.py @@ -36,7 +36,7 @@ class BaseTestCase(TestCase): self.bobs_profile.save() self.bobs_membership = Member.objects.create( - user=self.bob, project=self.project + user=self.bob, project=self.project, role=Member.Role.REGULAR ) # Charlie should have no access to Alice's stuff diff --git a/templates/accounts/project.html b/templates/accounts/project.html index df14e0ed..2f36d35a 100644 --- a/templates/accounts/project.html +++ b/templates/accounts/project.html @@ -169,7 +169,7 @@ {{ m.user.email }} {{ m.get_role_display}} - {% if is_owner %} + {% if is_manager and m.user != request.user %} {% endfor %} - {% if is_owner and invite_suggestions %} + {% if is_manager and invite_suggestions %} Add Users from Other Teams @@ -210,7 +210,7 @@
- {% if is_owner %} + {% if is_manager %} {% if project.can_invite_new_users %} + + Manager + + Can invite/remove other members. + + +