From 0ff4bd01e0a18c87c624396e05e4e9fd9c9d1b7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Fri, 14 Feb 2020 13:05:21 +0200 Subject: [PATCH] Improved UI to invite users from account's other projects. Fixes #258. The team size limit is applied to the number of distinct users across all projects. Fixes #332. --- CHANGELOG.md | 4 +++ hc/accounts/models.py | 14 +++++++-- hc/accounts/tests/test_project.py | 24 ++++++++++++++- hc/accounts/tests/test_project_model.py | 22 ++++++++++++- hc/accounts/views.py | 20 +++++++----- static/css/projects.css | 14 +++++++++ static/js/project.js | 12 ++++++-- templates/accounts/project.html | 41 ++++++++++++++++++++----- 8 files changed, 130 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed0506a8..c57187a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,12 @@ All notable changes to this project will be documented in this file. ## v1.14.0 - Unreleased +### Improvements +- Improved UI to invite users from account's other projects (#258) + ### Bug Fixes - The "render_docs" command checks if markdown and pygments is installed (#329) +- The team size limit is applied to the n. of distinct users across all projects (#332) ## v1.13.0 - 2020-02-13 diff --git a/hc/accounts/models.py b/hc/accounts/models.py index ab0793e3..6c9ddc41 100644 --- a/hc/accounts/models.py +++ b/hc/accounts/models.py @@ -245,8 +245,18 @@ class Project(models.Model): self.api_key_readonly = urlsafe_b64encode(os.urandom(24)).decode() self.save() - def can_invite(self): - return self.member_set.count() < self.owner_profile.team_limit + def team(self): + return User.objects.filter(memberships__project=self).order_by("email") + + def invite_suggestions(self): + q = User.objects.filter(memberships__project__owner_id=self.owner_id) + q = q.exclude(memberships__project=self) + return q.distinct().order_by("email") + + def can_invite_new_users(self): + q = User.objects.filter(memberships__project__owner_id=self.owner_id) + used = q.distinct().count() + return used < self.owner_profile.team_limit def invite(self, user): Member.objects.create(user=user, project=self) diff --git a/hc/accounts/tests/test_project.py b/hc/accounts/tests/test_project.py index 56cd045d..10d6bf0a 100644 --- a/hc/accounts/tests/test_project.py +++ b/hc/accounts/tests/test_project.py @@ -3,7 +3,7 @@ from django.core import mail from django.conf import settings from django.test.utils import override_settings from hc.test import BaseTestCase -from hc.accounts.models import Member +from hc.accounts.models import Member, Project from hc.api.models import TokenBucket @@ -88,6 +88,28 @@ class ProjectTestCase(BaseTestCase): ) self.assertHTMLEqual(mail.outbox[0].subject, subj) + 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 + self.profile.save() + + # But Charlie will have an existing membership in another Alice's project + # so Alice *should* be able to invite Charlie: + p2 = Project.objects.create(owner=self.alice) + 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"} + r = self.client.post(self.url, form) + self.assertEqual(r.status_code, 200) + + q = Member.objects.filter(project=self.project, user=self.charlie) + self.assertEqual(q.count(), 1) + + # And this should not have affected the rate limit: + q = TokenBucket.objects.filter(value="invite-%d" % self.alice.id) + self.assertFalse(q.exists()) + @override_settings(SECRET_KEY="test-secret") def test_it_rate_limits_invites(self): obj = TokenBucket(value="invite-%d" % self.alice.id) diff --git a/hc/accounts/tests/test_project_model.py b/hc/accounts/tests/test_project_model.py index c7623aa9..b4aff516 100644 --- a/hc/accounts/tests/test_project_model.py +++ b/hc/accounts/tests/test_project_model.py @@ -1,5 +1,5 @@ from hc.test import BaseTestCase -from hc.accounts.models import Project +from hc.accounts.models import Member, Project from hc.api.models import Check, Channel @@ -27,3 +27,23 @@ class ProjectModelTestCase(BaseTestCase): def test_it_handles_no_channels(self): # It's an issue if the project has no channels at all: self.assertTrue(self.project.have_channel_issues()) + + def test_it_allows_third_user(self): + # Alice is the owner, and Bob is invited -- there is space for the third user: + self.assertTrue(self.project.can_invite_new_users()) + + def test_it_allows_same_user_in_multiple_projects(self): + p2 = Project.objects.create(owner=self.alice) + Member.objects.create(user=self.bob, project=p2) + + # Bob's membership in two projects counts as one seat, + # one seat should be still free: + self.assertTrue(self.project.can_invite_new_users()) + + def test_it_checks_team_limit(self): + p2 = Project.objects.create(owner=self.alice) + Member.objects.create(user=self.charlie, project=p2) + + # Alice and Bob are in one project, Charlie is in another, + # so no seats left: + self.assertFalse(self.project.can_invite_new_users()) diff --git a/hc/accounts/views.py b/hc/accounts/views.py index f3889097..b1ceb588 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -265,6 +265,7 @@ def project(request, code): return HttpResponseNotFound() is_owner = project.owner_id == request.user.id + invite_suggestions = project.invite_suggestions() ctx = { "page": "project", "project": project, @@ -273,6 +274,7 @@ def project(request, code): "project_name_status": "default", "api_status": "default", "team_status": "default", + "invite_suggestions": invite_suggestions, } if request.method == "POST": @@ -293,15 +295,22 @@ 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 or not project.can_invite(): + if not is_owner: return HttpResponseForbidden() form = InviteTeamMemberForm(request.POST) if form.is_valid(): - if not TokenBucket.authorize_invite(request.user): - return render(request, "try_later.html") - email = form.cleaned_data["email"] + + if not invite_suggestions.filter(email=email).exists(): + # We're inviting a new user. Are we within team size limit? + if not project.can_invite_new_users(): + return HttpResponseForbidden() + + # And are we not hitting a rate limit? + if not TokenBucket.authorize_invite(request.user): + return render(request, "try_later.html") + try: user = User.objects.get(email=email) except User.DoesNotExist: @@ -343,9 +352,6 @@ def project(request, code): ctx["project_name_updated"] = True ctx["project_name_status"] = "success" - # Count members right before rendering the template, in case - # we just invited or removed someone - ctx["num_members"] = project.member_set.count() return render(request, "accounts/project.html", ctx) diff --git a/static/css/projects.css b/static/css/projects.css index 8a13166d..829e4712 100644 --- a/static/css/projects.css +++ b/static/css/projects.css @@ -44,4 +44,18 @@ #project-selector #add-project:hover .project { border-color: #0091EA; color: #333; +} + +.invite-suggestion { + color: #888; +} + +#suggestions-row td { + border-top: 0; + font-size: 85%; + padding-top: 20px; +} + +#team-table th { + border-top: 0; } \ No newline at end of file diff --git a/static/js/project.js b/static/js/project.js index 595ffb98..702f39e2 100644 --- a/static/js/project.js +++ b/static/js/project.js @@ -14,8 +14,14 @@ $(function() { $('#itm-email').focus(); }) - $('#set-team-name-modal').on('shown.bs.modal', function () { - $('#team-name').focus(); + $('#set-project-name-modal').on('shown.bs.modal', function () { + $('#project-name').focus(); }) -}); \ No newline at end of file + $(".add-to-team").click(function() { + $("#itm-email").val(this.dataset.email); + $("#invite-team-member-modal form").submit(); + return false; + }); + +}); diff --git a/templates/accounts/project.html b/templates/accounts/project.html index d379df5b..c34216a4 100644 --- a/templates/accounts/project.html +++ b/templates/accounts/project.html @@ -90,27 +90,53 @@

Team Access

- {% if num_members %} - + {% if project.team.exists or invite_suggestions %} +
+ + + + + - {% for member in project.member_set.all %} + {% for user in project.team %} - + {% endfor %} + + {% if is_owner and invite_suggestions %} + + + + + {% for user in project.invite_suggestions %} + + + + + + {% endfor %} + {% endif %}
EmailRole
{{ project.owner.email }} Owner
{{ member.user.email }} {{ user.email }} Member {% if is_owner %} Remove {% endif %}
+ Add Users from Other Teams +
{{ user.email }} + Add to Team +
{% else %}

@@ -123,7 +149,7 @@
{% if is_owner %} - {% if project.can_invite%} + {% if project.can_invite_new_users %} Team size limit reached. - To invite more members, please + To invite new members by email, please upgrade your account!

{% endif %} @@ -234,6 +260,7 @@
{% csrf_token %} +