From 6b6be017bb6f6e890e41efc106d79e5f86750b32 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 21 Aug 2024 11:34:59 +0200 Subject: [PATCH] wip: use autocomplete to add users in groups --- umap/forms.py | 20 +++++++-- umap/static/umap/content.css | 4 +- umap/static/umap/js/modules/autocomplete.js | 6 +-- umap/templates/auth/group_form.html | 30 ++++++++++++++ umap/tests/integration/test_group.py | 46 +++++++++++++++++++++ umap/tests/test_group_views.py | 34 ++++++++++++++- umap/urls.py | 5 ++- umap/views.py | 13 ++++-- 8 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 umap/tests/integration/test_group.py diff --git a/umap/forms.py b/umap/forms.py index d90d7068..15f39514 100644 --- a/umap/forms.py +++ b/umap/forms.py @@ -113,11 +113,25 @@ class UserProfileForm(forms.ModelForm): fields = ("username", "first_name", "last_name") +class GroupMembersField(forms.ModelMultipleChoiceField): + def set_choices(self, choices): + iterator = self.iterator(self) + # Override queryset so to expose only selected choices: + # - we don't want a select with 100000 options + # - the select values will be used by the autocomplete widget to display + # already existing members of the group + iterator.queryset = choices + self.choices = iterator + + class GroupForm(forms.ModelForm): class Meta: model = Group fields = ["name", "members"] - members = forms.ModelMultipleChoiceField( - queryset=User.objects.all(), widget=forms.CheckboxSelectMultiple - ) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields["members"].set_choices(self.initial["members"]) + self.fields["members"].widget.attrs["hidden"] = "hidden" + + members = GroupMembersField(queryset=User.objects.all()) diff --git a/umap/static/umap/content.css b/umap/static/umap/content.css index 04cc8946..6a9fe92e 100644 --- a/umap/static/umap/content.css +++ b/umap/static/umap/content.css @@ -273,9 +273,7 @@ ul.umap-autocomplete { border: 1px solid #202425; padding: 7px; color: #eeeeec; -} -.umap-multiresult li + li { - margin-top: 7px; + margin-bottom: 7px; } .umap-singleresult div .close, .umap-multiresult li .close { diff --git a/umap/static/umap/js/modules/autocomplete.js b/umap/static/umap/js/modules/autocomplete.js index e693d6be..1eadd791 100644 --- a/umap/static/umap/js/modules/autocomplete.js +++ b/umap/static/umap/js/modules/autocomplete.js @@ -9,8 +9,8 @@ import { Request, ServerRequest } from './request.js' import { escapeHTML, generateId } from './utils.js' export class BaseAutocomplete { - constructor(el, options) { - this.el = el + constructor(parent, options) { + this.parent = parent this.options = { placeholder: translate('Start typing...'), emptyMessage: translate('No result'), @@ -43,7 +43,7 @@ export class BaseAutocomplete { this.input = DomUtil.element({ tagName: 'input', type: 'text', - parent: this.el, + parent: this.parent, placeholder: this.options.placeholder, autocomplete: 'off', className: this.options.className, diff --git a/umap/templates/auth/group_form.html b/umap/templates/auth/group_form.html index 16ea81e6..45af6d13 100644 --- a/umap/templates/auth/group_form.html +++ b/umap/templates/auth/group_form.html @@ -25,4 +25,34 @@ {% endif %} + {% endblock maincontent %} diff --git a/umap/tests/integration/test_group.py b/umap/tests/integration/test_group.py new file mode 100644 index 00000000..bd9a4746 --- /dev/null +++ b/umap/tests/integration/test_group.py @@ -0,0 +1,46 @@ +import re + +import pytest +from django.contrib.auth.models import Group + +pytestmark = pytest.mark.django_db + + +def test_can_add_user_to_group(live_server, map, user, group, login): + map.owner.groups.add(group) + map.owner.save() + assert Group.objects.count() == 1 + page = login(map.owner) + with page.expect_navigation(): + page.get_by_role("link", name="My teams").click() + with page.expect_navigation(): + page.get_by_role("link", name="Edit").click() + page.get_by_placeholder("Add user").click() + with page.expect_response(re.compile(r".*/agnocomplete/.*")): + page.get_by_placeholder("Add user").press_sequentially("joe") + page.get_by_text("Joe").click() + page.get_by_role("button", name="Save").click() + assert Group.objects.count() == 1 + modified = Group.objects.first() + assert user in modified.user_set.all() + + +def test_can_remove_user_from_group(live_server, map, user, user2, group, login): + map.owner.groups.add(group) + map.owner.save() + user.groups.add(group) + user.save() + user2.groups.add(group) + user2.save() + assert Group.objects.count() == 1 + page = login(map.owner) + with page.expect_navigation(): + page.get_by_role("link", name="My teams").click() + with page.expect_navigation(): + page.get_by_role("link", name="Edit").click() + page.locator("li").filter(has_text="Averell").locator(".close").click() + page.get_by_role("button", name="Save").click() + assert Group.objects.count() == 1 + modified = Group.objects.first() + assert user in modified.user_set.all() + assert user2 not in modified.user_set.all() diff --git a/umap/tests/test_group_views.py b/umap/tests/test_group_views.py index d880f850..0135c1bd 100644 --- a/umap/tests/test_group_views.py +++ b/umap/tests/test_group_views.py @@ -37,7 +37,7 @@ def test_can_create_a_group(client, user): assert group in user.groups.all() -def test_can_edit_a_group(client, user, group): +def test_can_edit_a_group_name(client, user, group): user.groups.add(group) user.save() assert Group.objects.count() == 1 @@ -52,6 +52,38 @@ def test_can_edit_a_group(client, user, group): assert modified in user.groups.all() +def test_can_add_user_to_group(client, user, user2, group): + user.groups.add(group) + user.save() + assert Group.objects.count() == 1 + url = reverse("group_update", args=(group.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url, {"name": group.name, "members": [user.pk, user2.pk]}) + assert response.status_code == 302 + assert response["Location"] == "/en/me/groups" + assert Group.objects.count() == 1 + modified = Group.objects.first() + assert user in modified.user_set.all() + assert user2 in modified.user_set.all() + + +def test_can_remove_user_from_group(client, user, user2, group): + user.groups.add(group) + user.save() + user2.groups.add(group) + user2.save() + assert Group.objects.count() == 1 + url = reverse("group_update", args=(group.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url, {"name": group.name, "members": [user.pk]}) + assert response.status_code == 302 + assert response["Location"] == "/en/me/groups" + assert Group.objects.count() == 1 + modified = Group.objects.first() + assert user in modified.user_set.all() + assert user2 not in modified.user_set.all() + + def test_cannot_edit_a_group_if_not_member(client, user, user2, group): user.groups.add(group) user.save() diff --git a/umap/urls.py b/umap/urls.py index 0f466ae4..ca75a7ab 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -25,6 +25,10 @@ admin.autodiscover() urlpatterns = [ re_path(r"^admin/", admin.site.urls), re_path("", include("social_django.urls", namespace="social")), + re_path( + r"^agnocomplete/", + include(("agnocomplete.urls", "agnocomplete"), namespace="agnocomplete"), + ), re_path(r"^m/(?P\d+)/$", views.MapShortUrl.as_view(), name="map_short_url"), re_path(r"^ajax-proxy/$", cache_page(180)(views.ajax_proxy), name="ajax-proxy"), re_path( @@ -40,7 +44,6 @@ urlpatterns = [ name="password_change_done", ), re_path(r"^i18n/", include("django.conf.urls.i18n")), - re_path(r"^agnocomplete/", include("agnocomplete.urls")), re_path(r"^map/oembed/", views.MapOEmbed.as_view(), name="map_oembed"), re_path( r"^map/(?P\d+)/download/", diff --git a/umap/views.py b/umap/views.py index 12282317..6eebf52e 100644 --- a/umap/views.py +++ b/umap/views.py @@ -213,9 +213,16 @@ class GroupUpdate(UpdateView): return initial def form_valid(self, form): - for user in form.cleaned_data["members"]: - user.groups.add(self.object) - user.save() + actual = self.object.user_set.all() + wanted = form.cleaned_data["members"] + for user in wanted: + if user not in actual: + user.groups.add(self.object) + user.save() + for user in actual: + if user not in wanted: + user.groups.remove(self.object) + user.save() return super().form_valid(form)