From dce0ee5f737ba6c2e54c57bd60bbeef3a0d8e8fd Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 15 Aug 2024 14:54:43 +0200 Subject: [PATCH 01/14] wip: use auth.models.Group and manage permissions --- umap/decorators.py | 2 +- umap/forms.py | 2 +- ...up_alter_datalayer_edit_status_and_more.py | 67 +++++++++++++++++++ umap/models.py | 44 +++++++++--- umap/static/umap/js/modules/permissions.js | 12 ++++ umap/static/umap/js/umap.forms.js | 17 +++++ umap/tests/base.py | 8 +++ umap/tests/conftest.py | 6 ++ umap/tests/integration/test_owned_map.py | 22 +++++- umap/tests/test_datalayer.py | 39 +++++++++-- umap/tests/test_datalayer_views.py | 2 +- umap/tests/test_map.py | 34 ++++++++-- umap/tests/test_map_views.py | 4 +- umap/tests/test_views.py | 2 +- umap/views.py | 10 ++- 15 files changed, 237 insertions(+), 34 deletions(-) create mode 100644 umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py diff --git a/umap/decorators.py b/umap/decorators.py index 3ae667c5..fbe70429 100644 --- a/umap/decorators.py +++ b/umap/decorators.py @@ -35,7 +35,7 @@ def can_edit_map(view_func): map_inst = get_object_or_404(Map, pk=kwargs["map_id"]) user = request.user kwargs["map_inst"] = map_inst # Avoid rerequesting the map in the view - if map_inst.edit_status >= map_inst.EDITORS: + if map_inst.edit_status >= map_inst.COLLABORATORS: can_edit = map_inst.can_edit(user=user, request=request) if not can_edit: if map_inst.owner and not user.is_authenticated: diff --git a/umap/forms.py b/umap/forms.py index d1225b22..c7813a61 100644 --- a/umap/forms.py +++ b/umap/forms.py @@ -36,7 +36,7 @@ class SendLinkForm(forms.Form): class UpdateMapPermissionsForm(forms.ModelForm): class Meta: model = Map - fields = ("edit_status", "editors", "share_status", "owner") + fields = ("edit_status", "editors", "share_status", "owner", "group") class AnonymousMapPermissionsForm(forms.ModelForm): diff --git a/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py b/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py new file mode 100644 index 00000000..bea2ef72 --- /dev/null +++ b/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py @@ -0,0 +1,67 @@ +# Generated by Django 5.1 on 2024-08-15 11:33 + +import django.db.models.deletion +import umap.models +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("auth", "0012_alter_user_first_name_max_length"), + ("umap", "0021_remove_map_description"), + ] + + operations = [ + migrations.AddField( + model_name="map", + name="group", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="auth.group", + verbose_name="group", + ), + ), + migrations.AlterField( + model_name="datalayer", + name="edit_status", + field=models.SmallIntegerField( + choices=[ + (0, "Inherit"), + (1, "Everyone"), + (2, "Editors and team only"), + (3, "Owner only"), + ], + default=0, + verbose_name="edit status", + ), + ), + migrations.AlterField( + model_name="map", + name="edit_status", + field=models.SmallIntegerField( + choices=[ + (1, "Everyone"), + (2, "Editors and team only"), + (3, "Owner only"), + ], + default=umap.models.get_default_edit_status, + verbose_name="edit status", + ), + ), + migrations.AlterField( + model_name="map", + name="share_status", + field=models.SmallIntegerField( + choices=[ + (1, "Everyone (public)"), + (2, "Anyone with link"), + (3, "Editors and team only"), + (9, "Blocked"), + ], + default=umap.models.get_default_share_status, + verbose_name="share status", + ), + ), + ] diff --git a/umap/models.py b/umap/models.py index 5efe3aba..fe3e3753 100644 --- a/umap/models.py +++ b/umap/models.py @@ -5,7 +5,7 @@ import time import uuid from django.conf import settings -from django.contrib.auth.models import User +from django.contrib.auth.models import Group, User from django.contrib.gis.db import models from django.core.files.base import File from django.core.signing import Signer @@ -36,9 +36,19 @@ def get_user_stars_url(self): return reverse("user_stars", kwargs={"identifier": identifier}) +def get_group_url(self): + return "TODO" + + +def get_group_metadata(self): + return {"id": self.pk, "name": self.name, "url": self.get_url()} + + User.add_to_class("__str__", display_name) User.add_to_class("get_url", get_user_url) User.add_to_class("get_stars_url", get_user_stars_url) +Group.add_to_class("get_url", get_group_url) +Group.add_to_class("get_metadata", get_group_metadata) def get_default_share_status(): @@ -137,7 +147,7 @@ class Map(NamedModel): """ ANONYMOUS = 1 - EDITORS = 2 + COLLABORATORS = 2 OWNER = 3 PUBLIC = 1 OPEN = 2 @@ -145,13 +155,13 @@ class Map(NamedModel): BLOCKED = 9 EDIT_STATUS = ( (ANONYMOUS, _("Everyone")), - (EDITORS, _("Editors only")), + (COLLABORATORS, _("Editors and team only")), (OWNER, _("Owner only")), ) SHARE_STATUS = ( (PUBLIC, _("Everyone (public)")), (OPEN, _("Anyone with link")), - (PRIVATE, _("Editors only")), + (PRIVATE, _("Editors and team only")), (BLOCKED, _("Blocked")), ) slug = models.SlugField(db_index=True) @@ -180,6 +190,13 @@ class Map(NamedModel): editors = models.ManyToManyField( settings.AUTH_USER_MODEL, blank=True, verbose_name=_("editors") ) + group = models.ForeignKey( + "auth.Group", + blank=True, + null=True, + verbose_name=_("team"), + on_delete=models.SET_NULL, + ) edit_status = models.SmallIntegerField( choices=EDIT_STATUS, default=get_default_edit_status, @@ -281,7 +298,7 @@ class Map(NamedModel): In owner mode: - only owner by default (OWNER) - - any editor if mode is EDITORS + - any editor or group member if mode is COLLABORATORS - anyone otherwise (ANONYMOUS) In anonymous owner mode: - only owner (has ownership cookie) by default (OWNER) @@ -297,8 +314,9 @@ class Map(NamedModel): can = False elif user == self.owner: can = True - elif self.edit_status == self.EDITORS and user in self.editors.all(): - can = True + elif self.edit_status == self.COLLABORATORS: + if user in self.editors.all() or self.group in user.groups.all(): + can = True return can def can_view(self, request): @@ -308,12 +326,15 @@ class Map(NamedModel): can = True elif self.share_status in [self.PUBLIC, self.OPEN]: can = True + elif request.user is None: + can = False elif request.user == self.owner: can = True else: can = not ( self.share_status == self.PRIVATE and request.user not in self.editors.all() + and self.group not in request.user.groups.all() ) return can @@ -383,12 +404,12 @@ class DataLayer(NamedModel): INHERIT = 0 ANONYMOUS = 1 - EDITORS = 2 + COLLABORATORS = 2 OWNER = 3 EDIT_STATUS = ( (INHERIT, _("Inherit")), (ANONYMOUS, _("Everyone")), - (EDITORS, _("Editors only")), + (COLLABORATORS, _("Editors and team only")), (OWNER, _("Owner only")), ) uuid = models.UUIDField( @@ -538,8 +559,9 @@ class DataLayer(NamedModel): can = True elif user is not None and user == self.map.owner: can = True - elif self.edit_status == self.EDITORS and user in self.map.editors.all(): - can = True + elif user is not None and self.edit_status == self.COLLABORATORS: + if user in self.map.editors.all() or self.map.group in user.groups.all(): + can = True return can diff --git a/umap/static/umap/js/modules/permissions.js b/umap/static/umap/js/modules/permissions.js index 52ae216c..5c6c20c2 100644 --- a/umap/static/umap/js/modules/permissions.js +++ b/umap/static/umap/js/modules/permissions.js @@ -25,6 +25,7 @@ export class MapPermissions { this.options = Object.assign( { owner: null, + group: null, editors: [], share_status: null, edit_status: null, @@ -96,6 +97,16 @@ export class MapPermissions { 'options.owner', { handler: 'ManageOwner', label: translate("Map's owner") }, ]) + if (this.map.options.user?.groups?.length) { + fields.push([ + 'options.group', + { + handler: 'ManageGroup', + label: translate('Attach map to a team'), + groups: this.map.options.user.groups, + }, + ]) + } } fields.push([ 'options.editors', @@ -150,6 +161,7 @@ export class MapPermissions { formData.append('edit_status', this.options.edit_status) if (this.isOwner()) { formData.append('owner', this.options.owner?.id) + formData.append('group', this.options.group?.id || '') formData.append('share_status', this.options.share_status) } const [data, response, error] = await this.map.server.post( diff --git a/umap/static/umap/js/umap.forms.js b/umap/static/umap/js/umap.forms.js index cb6760d8..15dc3311 100644 --- a/umap/static/umap/js/umap.forms.js +++ b/umap/static/umap/js/umap.forms.js @@ -1086,6 +1086,23 @@ L.FormBuilder.ManageEditors = L.FormBuilder.Element.extend({ }, }) +L.FormBuilder.ManageGroup = L.FormBuilder.IntSelect.extend({ + getOptions: function () { + return [[null, L._('None')]].concat( + this.options.groups.map((group) => [group.id, group.name]) + ) + }, + toHTML: function () { + return this.get()?.id + }, + toJS: function () { + const value = this.value() + for (const group of this.options.groups) { + if (group.id === value) return group + } + }, +}) + U.FormBuilder = L.FormBuilder.extend({ options: { className: 'umap-form', diff --git a/umap/tests/base.py b/umap/tests/base.py index 9491dada..4d0ce144 100644 --- a/umap/tests/base.py +++ b/umap/tests/base.py @@ -3,6 +3,7 @@ import json import factory from django.contrib.auth import get_user_model +from django.contrib.auth.models import Group from django.core.files.base import ContentFile from django.urls import reverse @@ -58,6 +59,13 @@ class UserFactory(factory.django.DjangoModelFactory): model = User +class GroupFactory(factory.django.DjangoModelFactory): + name = "Awesome Group" + + class Meta: + model = Group + + class MapFactory(factory.django.DjangoModelFactory): name = "test map" slug = "test-map" diff --git a/umap/tests/conftest.py b/umap/tests/conftest.py index 4dafa3dd..3bee02e7 100644 --- a/umap/tests/conftest.py +++ b/umap/tests/conftest.py @@ -9,6 +9,7 @@ from umap.models import Map from .base import ( DataLayerFactory, + GroupFactory, LicenceFactory, MapFactory, TileLayerFactory, @@ -29,6 +30,11 @@ def pytest_runtest_teardown(): cache.clear() +@pytest.fixture +def group(): + return GroupFactory() + + @pytest.fixture def user(): return UserFactory(password="123123") diff --git a/umap/tests/integration/test_owned_map.py b/umap/tests/integration/test_owned_map.py index bf189641..19d02804 100644 --- a/umap/tests/integration/test_owned_map.py +++ b/umap/tests/integration/test_owned_map.py @@ -81,7 +81,7 @@ def test_owner_permissions_form(map, datalayer, live_server, login): def test_map_update_with_editor(map, live_server, login, user): - map.edit_status = Map.EDITORS + map.edit_status = Map.COLLABORATORS map.editors.add(user) map.save() page = login(user) @@ -104,7 +104,7 @@ def test_map_update_with_editor(map, live_server, login, user): def test_permissions_form_with_editor(map, datalayer, live_server, login, user): - map.edit_status = Map.EDITORS + map.edit_status = Map.COLLABORATORS map.editors.add(user) map.save() page = login(user) @@ -141,7 +141,7 @@ def test_owner_has_delete_map_button(map, live_server, login): def test_editor_do_not_have_delete_map_button(map, live_server, login, user): - map.edit_status = Map.EDITORS + map.edit_status = Map.COLLABORATORS map.editors.add(user) map.save() page = login(user) @@ -241,3 +241,19 @@ def test_can_delete_datalayer(live_server, map, login, datalayer): expect(markers).to_have_count(0) # FIXME does not work, resolve to 1 element, even if this command is empty: expect(layers).to_have_count(0) + + +def test_can_set_group(map, live_server, login, group): + map.owner.groups.add(group) + map.owner.save() + page = login(map.owner) + page.goto(f"{live_server.url}{map.get_absolute_url()}?edit") + edit_permissions = page.get_by_title("Update permissions and editors") + edit_permissions.click() + page.locator("select[name=group]").select_option(str(group.pk)) + save = page.get_by_role("button", name="Save") + expect(save).to_be_visible() + with page.expect_response(re.compile(r".*/update/permissions/.*")): + save.click() + modified = Map.objects.get(pk=map.pk) + assert modified.group == group diff --git a/umap/tests/test_datalayer.py b/umap/tests/test_datalayer.py index ee564bc8..c60ab3c0 100644 --- a/umap/tests/test_datalayer.py +++ b/umap/tests/test_datalayer.py @@ -101,22 +101,33 @@ def test_should_remove_old_versions_on_save(map, settings): def test_anonymous_cannot_edit_in_editors_mode(datalayer): - datalayer.edit_status = DataLayer.EDITORS + datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() assert not datalayer.can_edit() def test_owner_can_edit_in_editors_mode(datalayer, user): - datalayer.edit_status = DataLayer.EDITORS + datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() assert datalayer.can_edit(datalayer.map.owner) -def test_editor_can_edit_in_editors_mode(datalayer, user): +def test_editor_can_edit_in_collaborators_mode(datalayer, user): map = datalayer.map map.editors.add(user) map.save() - datalayer.edit_status = DataLayer.EDITORS + datalayer.edit_status = DataLayer.COLLABORATORS + datalayer.save() + assert datalayer.can_edit(user) + + +def test_group_members_can_edit_in_collaborators_mode(datalayer, user, group): + user.groups.add(group) + user.save() + map = datalayer.map + map.group = group + map.save() + datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() assert datalayer.can_edit(user) @@ -170,6 +181,20 @@ def test_editors_cannot_edit_in_inherit_mode_and_map_in_owner_mode(datalayer, us assert not datalayer.can_edit(user) +def test_group_members_cannot_edit_in_inherit_mode_and_map_in_owner_mode( + datalayer, user, group +): + datalayer.edit_status = DataLayer.INHERIT + datalayer.save() + user.groups.add(group) + group.save() + map = datalayer.map + map.group = group + map.edit_status = Map.OWNER + map.save() + assert not datalayer.can_edit(user) + + def test_anonymous_cannot_edit_in_inherit_mode_and_map_in_owner_mode(datalayer): datalayer.edit_status = DataLayer.INHERIT datalayer.save() @@ -183,7 +208,7 @@ def test_owner_can_edit_in_inherit_mode_and_map_in_editors_mode(datalayer): datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map - map.edit_status = Map.EDITORS + map.edit_status = Map.COLLABORATORS map.save() assert datalayer.can_edit(map.owner) @@ -193,7 +218,7 @@ def test_editors_can_edit_in_inherit_mode_and_map_in_editors_mode(datalayer, use datalayer.save() map = datalayer.map map.editors.add(user) - map.edit_status = Map.EDITORS + map.edit_status = Map.COLLABORATORS map.save() assert datalayer.can_edit(user) @@ -202,7 +227,7 @@ def test_anonymous_cannot_edit_in_inherit_mode_and_map_in_editors_mode(datalayer datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map - map.edit_status = Map.EDITORS + map.edit_status = Map.COLLABORATORS map.save() assert not datalayer.can_edit() diff --git a/umap/tests/test_datalayer_views.py b/umap/tests/test_datalayer_views.py index c59a12fb..f50510d1 100644 --- a/umap/tests/test_datalayer_views.py +++ b/umap/tests/test_datalayer_views.py @@ -379,7 +379,7 @@ def test_owner_can_edit_in_owner_mode(datalayer, client, map, post_data): def test_editor_can_edit_in_editors_mode(datalayer, client, map, post_data): client.login(username=map.owner.username, password="123123") - datalayer.edit_status = DataLayer.EDITORS + datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() url = reverse("datalayer_update", args=(map.pk, datalayer.pk)) name = "new name" diff --git a/umap/tests/test_map.py b/umap/tests/test_map.py index 41cdf740..a0eb83e0 100644 --- a/umap/tests/test_map.py +++ b/umap/tests/test_map.py @@ -43,13 +43,31 @@ def test_editors_cannot_edit_if_status_owner(map, user): assert not map.can_edit(user) -def test_editors_can_edit_if_status_editors(map, user): - map.edit_status = map.EDITORS +def test_editors_can_edit_if_status_collaborators(map, user): + map.edit_status = map.COLLABORATORS map.editors.add(user) map.save() assert map.can_edit(user) +def test_group_members_cannot_edit_if_status_owner(map, user, group): + user.groups.add(group) + user.save() + map.edit_status = map.OWNER + map.group = group + map.save() + assert not map.can_edit(user) + + +def test_group_members_can_edit_if_status_collaborators(map, user, group): + user.groups.add(group) + user.save() + map.edit_status = map.COLLABORATORS + map.group = group + map.save() + assert map.can_edit(user) + + def test_logged_in_user_should_be_allowed_for_anonymous_map_with_anonymous_edit_status( map, user, rf ): # noqa @@ -87,6 +105,14 @@ def test_clone_should_keep_editors(map, user): assert user in clone.editors.all() +def test_clone_should_keep_group(map, user, group): + map.group = group + map.save() + clone = map.clone() + assert map.pk != clone.pk + assert clone.group == group + + def test_clone_should_update_owner_if_passed(map, user): clone = map.clone(owner=user) assert map.pk != clone.pk @@ -119,9 +145,9 @@ def test_publicmanager_should_get_only_public_maps(map, user, licence): def test_can_change_default_edit_status(user, settings): map = MapFactory(owner=user) assert map.edit_status == Map.OWNER - settings.UMAP_DEFAULT_EDIT_STATUS = Map.EDITORS + settings.UMAP_DEFAULT_EDIT_STATUS = Map.COLLABORATORS map = MapFactory(owner=user) - assert map.edit_status == Map.EDITORS + assert map.edit_status == Map.COLLABORATORS def test_can_change_default_share_status(user, settings): diff --git a/umap/tests/test_map_views.py b/umap/tests/test_map_views.py index aeb65e5a..03c1adee 100644 --- a/umap/tests/test_map_views.py +++ b/umap/tests/test_map_views.py @@ -210,7 +210,7 @@ def test_user_not_allowed_should_not_clone_map(client, map, user, settings): def test_clone_should_set_cloner_as_owner(client, map, user): url = reverse("map_clone", kwargs={"map_id": map.pk}) - map.edit_status = map.EDITORS + map.edit_status = map.COLLABORATORS map.editors.add(user) map.save() client.login(username=user.username, password="123123") @@ -330,7 +330,7 @@ def test_only_owner_can_delete(client, map, user): def test_map_editors_do_not_see_owner_change_input(client, map, user): map.editors.add(user) - map.edit_status = map.EDITORS + map.edit_status = map.COLLABORATORS map.save() url = reverse("map_update_permissions", kwargs={"map_id": map.pk}) client.login(username=user.username, password="123123") diff --git a/umap/tests/test_views.py b/umap/tests/test_views.py index c693cdd2..4e307ddb 100644 --- a/umap/tests/test_views.py +++ b/umap/tests/test_views.py @@ -474,7 +474,7 @@ def test_websocket_token_returns_a_valid_token_when_authorized(client, user, map @pytest.mark.django_db def test_websocket_token_is_generated_for_editors(client, user, user2, map): - map.edit_status = Map.EDITORS + map.edit_status = Map.COLLABORATORS map.editors.add(user2) map.save() diff --git a/umap/views.py b/umap/views.py index 5f976977..50a0dd0c 100644 --- a/umap/views.py +++ b/umap/views.py @@ -459,14 +459,16 @@ def simple_json_response(**kwargs): class SessionMixin: def get_user_data(self): data = {} + user = self.request.user if hasattr(self, "object"): - data["is_owner"] = self.object.is_owner(self.request.user, self.request) - if self.request.user.is_anonymous: + data["is_owner"] = self.object.is_owner(user, self.request) + if user.is_anonymous: return data return { - "id": self.request.user.pk, + "id": user.pk, "name": str(self.request.user), "url": reverse("user_dashboard"), + "groups": [group.get_metadata() for group in user.groups.all()], **data, } @@ -605,6 +607,8 @@ class PermissionsMixin: {"id": editor.pk, "name": str(editor)} for editor in self.object.editors.all() ] + if self.object.group: + permissions["group"] = self.object.group.get_metadata() if not self.object.owner and self.object.is_anonymous_owner(self.request): permissions["anonymous_edit_url"] = self.object.get_anonymous_edit_url() return permissions From a3e972bf5db497c76dce9143034c203e61ba9d92 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 15 Aug 2024 16:20:29 +0200 Subject: [PATCH 02/14] wip: add group maps page and display group as author when defined --- ...up_alter_datalayer_edit_status_and_more.py | 3 ++- umap/models.py | 5 +++- umap/static/umap/js/modules/caption.js | 2 +- umap/static/umap/js/modules/permissions.js | 17 ------------- umap/static/umap/js/umap.js | 19 +++++++++++++- umap/static/umap/map.css | 2 +- umap/templates/umap/map_list.html | 6 ++--- umap/tests/integration/test_caption.py | 20 +++++++++++++++ umap/urls.py | 1 + umap/views.py | 25 +++++++++++++++++++ 10 files changed, 75 insertions(+), 25 deletions(-) diff --git a/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py b/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py index bea2ef72..511a8678 100644 --- a/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py +++ b/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py @@ -1,9 +1,10 @@ # Generated by Django 5.1 on 2024-08-15 11:33 import django.db.models.deletion -import umap.models from django.db import migrations, models +import umap.models + class Migration(migrations.Migration): dependencies = [ diff --git a/umap/models.py b/umap/models.py index fe3e3753..d4fbae30 100644 --- a/umap/models.py +++ b/umap/models.py @@ -37,7 +37,7 @@ def get_user_stars_url(self): def get_group_url(self): - return "TODO" + return reverse("group_maps", kwargs={"pk": self.pk}) def get_group_metadata(self): @@ -268,6 +268,9 @@ class Map(NamedModel): path = reverse("map_anonymous_edit_url", kwargs={"signature": signature}) return settings.SITE_URL + path + def get_author(self): + return self.group or self.owner + def is_owner(self, user=None, request=None): if user and self.owner == user: return True diff --git a/umap/static/umap/js/modules/caption.js b/umap/static/umap/js/modules/caption.js index 36c63c4c..5496688b 100644 --- a/umap/static/umap/js/modules/caption.js +++ b/umap/static/umap/js/modules/caption.js @@ -20,7 +20,7 @@ export default class Caption { const container = DomUtil.create('div', 'umap-caption') const hgroup = DomUtil.element({ tagName: 'hgroup', parent: container }) DomUtil.createTitle(hgroup, this.map.options.name, 'icon-caption icon-block') - this.map.permissions.addOwnerLink('h4', hgroup) + this.map.addAuthorLink('h4', hgroup) if (this.map.options.description) { const description = DomUtil.element({ tagName: 'div', diff --git a/umap/static/umap/js/modules/permissions.js b/umap/static/umap/js/modules/permissions.js index 5c6c20c2..58c230a7 100644 --- a/umap/static/umap/js/modules/permissions.js +++ b/umap/static/umap/js/modules/permissions.js @@ -189,23 +189,6 @@ export class MapPermissions { }) } - addOwnerLink(element, container) { - if (this.options.owner?.name && this.options.owner.url) { - const ownerContainer = DomUtil.add( - element, - 'umap-map-owner', - container, - ` ${translate('by')} ` - ) - DomUtil.createLink( - '', - ownerContainer, - this.options.owner.name, - this.options.owner.url - ) - } - } - commit() { this.map.options.permissions = Object.assign( this.map.options.permissions, diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 0e060182..3493e082 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -1582,7 +1582,7 @@ U.Map = L.Map.extend({ ) const name = L.DomUtil.create('h3', '', container) L.DomEvent.disableClickPropagation(container) - this.permissions.addOwnerLink('span', container) + this.addAuthorLink('span', container) if (this.getOption('captionMenus')) { L.DomUtil.createButton( 'umap-about-link flat', @@ -1887,4 +1887,21 @@ U.Map = L.Map.extend({ .filter((val, idx, arr) => arr.indexOf(val) === idx) .sort(U.Utils.naturalSort) }, + + addAuthorLink: function (element, container) { + if (this.options.author?.name) { + const authorContainer = L.DomUtil.add( + element, + 'umap-map-author', + container, + ` ${L._('by')} ` + ) + L.DomUtil.createLink( + '', + authorContainer, + this.options.author.name, + this.options.author.url + ) + } + }, }) diff --git a/umap/static/umap/map.css b/umap/static/umap/map.css index 6db07c63..8bbb455c 100644 --- a/umap/static/umap/map.css +++ b/umap/static/umap/map.css @@ -922,7 +922,7 @@ a.umap-control-caption, .datalayer-name { cursor: pointer; } -.umap-caption .umap-map-owner { +.umap-caption .umap-map-author { padding-inline-start: 31px; } diff --git a/umap/templates/umap/map_list.html b/umap/templates/umap/map_list.html index 76948266..9e7f7f3e 100644 --- a/umap/templates/umap/map_list.html +++ b/umap/templates/umap/map_list.html @@ -6,9 +6,9 @@ {% map_fragment map_inst prefix=prefix page=request.GET.p %}
{{ map_inst.name }} - {% if map_inst.owner %} - {% trans "by" %} {{ map_inst.owner }} - {% endif %} + {% with author=map_inst.get_author %} + {% trans "by" %} {{ author }} + {% endwith %}
{% endfor %} diff --git a/umap/tests/integration/test_caption.py b/umap/tests/integration/test_caption.py index 1458a8a3..e5f2b7eb 100644 --- a/umap/tests/integration/test_caption.py +++ b/umap/tests/integration/test_caption.py @@ -25,3 +25,23 @@ def test_caption(live_server, page, map): panel.locator(".datalayer-legend .off").get_by_text(non_loaded.name) ).to_be_visible() expect(panel.locator(".datalayer-legend").get_by_text(hidden.name)).to_be_hidden() + + +def test_caption_should_display_owner_as_author(live_server, page, map): + map.settings["properties"]["onLoadPanel"] = "caption" + map.save() + page.goto(f"{live_server.url}{map.get_absolute_url()}") + panel = page.locator(".panel.left.on") + expect(panel).to_be_visible() + expect(panel.get_by_text("By Gabriel")).to_be_visible() + + +def test_caption_should_display_group_as_author(live_server, page, map, group): + map.settings["properties"]["onLoadPanel"] = "caption" + map.group = group + map.save() + page.goto(f"{live_server.url}{map.get_absolute_url()}") + panel = page.locator(".panel.left.on") + expect(panel).to_be_visible() + expect(panel.get_by_text("By Gabriel")).to_be_hidden() + expect(panel.get_by_text("By Awesome Group")).to_be_visible() diff --git a/umap/urls.py b/umap/urls.py index ecc9353a..2a39fe6b 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -187,6 +187,7 @@ urlpatterns += i18n_patterns( re_path(r"^about/$", views.about, name="about"), re_path(r"^user/(?P.+)/stars/$", views.user_stars, name="user_stars"), re_path(r"^user/(?P.+)/$", views.user_maps, name="user_maps"), + path("group//", views.group_maps, name="group_maps"), re_path(r"", include(i18n_urls)), ) urlpatterns += ( diff --git a/umap/views.py b/umap/views.py index 50a0dd0c..b55c9020 100644 --- a/umap/views.py +++ b/umap/views.py @@ -18,6 +18,7 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth import get_user_model from django.contrib.auth import logout as do_logout +from django.contrib.auth.models import Group from django.contrib.gis.measure import D from django.contrib.postgres.search import SearchQuery, SearchVector from django.contrib.staticfiles.storage import staticfiles_storage @@ -247,6 +248,24 @@ class UserStars(UserMaps): user_stars = UserStars.as_view() +class GroupMaps(PaginatorMixin, DetailView): + model = Group + list_template_name = "umap/map_list.html" + context_object_name = "current_group" + + def get_maps(self): + return Map.public.filter(group=self.object).order_by("-modified_at") + + def get_context_data(self, **kwargs): + kwargs.update( + {"maps": self.paginate(self.get_maps(), settings.UMAP_MAPS_PER_PAGE)} + ) + return super().get_context_data(**kwargs) + + +group_maps = GroupMaps.as_view() + + class SearchMixin: def get_search_queryset(self, **kwargs): q = self.request.GET.get("q") @@ -673,6 +692,12 @@ class MapView(MapDetailMixin, PermissionsMixin, DetailView): map_settings["properties"] = {} map_settings["properties"]["name"] = self.object.name map_settings["properties"]["permissions"] = self.get_permissions() + author = self.object.get_author() + if author: + map_settings["properties"]["author"] = { + "name": str(author), + "url": author.get_url(), + } return map_settings def is_starred(self): From 9b2a99019ba442c2a775a13cdfedaccb204bd3b9 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Mon, 19 Aug 2024 15:21:06 +0200 Subject: [PATCH 03/14] wip: add very basic CRUD for groups --- umap/decorators.py | 12 ++++ umap/forms.py | 11 ++++ umap/templates/auth/group_confirm_delete.html | 19 ++++++ umap/templates/auth/group_detail.html | 22 +++++++ umap/templates/auth/group_form.html | 28 +++++++++ umap/templates/auth/user_form.html | 7 +-- umap/templates/umap/dashboard_menu.html | 15 +++++ umap/templates/umap/user_dashboard.html | 8 +-- umap/templates/umap/user_groups.html | 19 ++++++ umap/urls.py | 18 ++++-- umap/views.py | 58 +++++++++++++++++++ 11 files changed, 199 insertions(+), 18 deletions(-) create mode 100644 umap/templates/auth/group_confirm_delete.html create mode 100644 umap/templates/auth/group_detail.html create mode 100644 umap/templates/auth/group_form.html create mode 100644 umap/templates/umap/dashboard_menu.html create mode 100644 umap/templates/umap/user_groups.html diff --git a/umap/decorators.py b/umap/decorators.py index fbe70429..f0187cbc 100644 --- a/umap/decorators.py +++ b/umap/decorators.py @@ -1,6 +1,7 @@ from functools import wraps from django.conf import settings +from django.contrib.auth.models import Group from django.http import HttpResponseForbidden from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy @@ -60,3 +61,14 @@ def can_view_map(view_func): return view_func(request, *args, **kwargs) return wrapper + + +def group_members_only(view_func): + @wraps(view_func) + def wrapper(request, *args, **kwargs): + group = get_object_or_404(Group, pk=kwargs["pk"]) + if group not in request.user.groups.all(): + return HttpResponseForbidden() + return view_func(request, *args, **kwargs) + + return wrapper diff --git a/umap/forms.py b/umap/forms.py index c7813a61..d90d7068 100644 --- a/umap/forms.py +++ b/umap/forms.py @@ -1,6 +1,7 @@ from django import forms from django.conf import settings from django.contrib.auth import get_user_model +from django.contrib.auth.models import Group from django.contrib.gis.geos import Point from django.forms.utils import ErrorList from django.template.defaultfilters import slugify @@ -110,3 +111,13 @@ class UserProfileForm(forms.ModelForm): class Meta: model = User fields = ("username", "first_name", "last_name") + + +class GroupForm(forms.ModelForm): + class Meta: + model = Group + fields = ["name", "members"] + + members = forms.ModelMultipleChoiceField( + queryset=User.objects.all(), widget=forms.CheckboxSelectMultiple + ) diff --git a/umap/templates/auth/group_confirm_delete.html b/umap/templates/auth/group_confirm_delete.html new file mode 100644 index 00000000..31283229 --- /dev/null +++ b/umap/templates/auth/group_confirm_delete.html @@ -0,0 +1,19 @@ +{% extends "umap/content.html" %} + +{% load i18n %} + +{% block maincontent %} + {% include "umap/dashboard_menu.html" with selected="groups" %} +
+
+
+ {% csrf_token %} +

+ Are you sure you want to delete "{{ object }}"? +

+ {{ form }} + +
+
+
+{% endblock maincontent %} diff --git a/umap/templates/auth/group_detail.html b/umap/templates/auth/group_detail.html new file mode 100644 index 00000000..bd044b00 --- /dev/null +++ b/umap/templates/auth/group_detail.html @@ -0,0 +1,22 @@ +{% extends "umap/content.html" %} + +{% load i18n %} + +{% block maincontent %} +
+

+ {% blocktrans %}Browse {{ current_group }}'s maps{% endblocktrans %} +

+
+
+
+ {% if maps %} + {% include "umap/map_list.html" %} + {% else %} +
+ {% blocktrans %}{{ current_group }} has no public maps.{% endblocktrans %} +
+ {% endif %} +
+
+{% endblock maincontent %} diff --git a/umap/templates/auth/group_form.html b/umap/templates/auth/group_form.html new file mode 100644 index 00000000..3709a8c8 --- /dev/null +++ b/umap/templates/auth/group_form.html @@ -0,0 +1,28 @@ +{% extends "umap/content.html" %} + +{% load i18n %} + +{% block maincontent %} + {% include "umap/dashboard_menu.html" with selected="groups" %} +
+
+ {% if form.non_field_errors %} +
    + {% for error in form.non_field_errors %} +
  • + {{ error }} +
  • + {% endfor %} +
+ {% endif %} +
+ {% csrf_token %} + {{ form }} + +
+ {% if group.user_set.count == 1 %} + {% trans "Delete this group" %} + {% endif %} +
+
+{% endblock maincontent %} diff --git a/umap/templates/auth/user_form.html b/umap/templates/auth/user_form.html index bbcc5f7e..534d0f80 100644 --- a/umap/templates/auth/user_form.html +++ b/umap/templates/auth/user_form.html @@ -3,12 +3,7 @@ {% load i18n %} {% block maincontent %} - + {% include "umap/dashboard_menu.html" with selected="profile" %}
{% if form.non_field_errors %} diff --git a/umap/templates/umap/dashboard_menu.html b/umap/templates/umap/dashboard_menu.html new file mode 100644 index 00000000..519abe8f --- /dev/null +++ b/umap/templates/umap/dashboard_menu.html @@ -0,0 +1,15 @@ +{% load i18n %} + + diff --git a/umap/templates/umap/user_dashboard.html b/umap/templates/umap/user_dashboard.html index 9459ee66..48cbf408 100644 --- a/umap/templates/umap/user_dashboard.html +++ b/umap/templates/umap/user_dashboard.html @@ -7,13 +7,7 @@ {% endblock head_title %} {% block maincontent %} {% trans "Search my maps" as placeholder %} - + {% include "umap/dashboard_menu.html" with selected="maps" %}
diff --git a/umap/templates/umap/user_groups.html b/umap/templates/umap/user_groups.html new file mode 100644 index 00000000..6a0e5668 --- /dev/null +++ b/umap/templates/umap/user_groups.html @@ -0,0 +1,19 @@ +{% extends "umap/content.html" %} + +{% load i18n %} + +{% block maincontent %} + {% include "umap/dashboard_menu.html" with selected="groups" %} +
+
+ + {% trans "New team" %} +
+
+{% endblock %} diff --git a/umap/urls.py b/umap/urls.py index 2a39fe6b..02accb2d 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -15,6 +15,7 @@ from . import views from .decorators import ( can_edit_map, can_view_map, + group_members_only, login_required_if_not_anonymous_allowed, ) from .utils import decorated_patterns @@ -96,8 +97,8 @@ i18n_urls += decorated_patterns( ) i18n_urls += decorated_patterns( [ensure_csrf_cookie], - re_path(r"^map/$", views.MapPreview.as_view(), name="map_preview"), - re_path(r"^map/new/$", views.MapNew.as_view(), name="map_new"), + path("map/", views.MapPreview.as_view(), name="map_preview"), + path("map/new/", views.MapNew.as_view(), name="map_new"), ) i18n_urls += decorated_patterns( [login_required_if_not_anonymous_allowed, never_cache], @@ -110,9 +111,16 @@ i18n_urls += decorated_patterns( views.ToggleMapStarStatus.as_view(), name="map_star", ), - re_path(r"^me$", views.user_dashboard, name="user_dashboard"), - re_path(r"^me/profile$", views.user_profile, name="user_profile"), - re_path(r"^me/download$", views.user_download, name="user_download"), + path("me", views.user_dashboard, name="user_dashboard"), + path("me/profile", views.user_profile, name="user_profile"), + path("me/download", views.user_download, name="user_download"), + path("me/groups", views.UserGroups.as_view(), name="user_groups"), + path("group/create/", views.GroupNew.as_view(), name="group_new"), +) +i18n_urls += decorated_patterns( + [login_required, group_members_only], + path("group//edit/", views.GroupUpdate.as_view(), name="group_update"), + path("group//delete/", views.GroupDelete.as_view(), name="group_delete"), ) map_urls = [ re_path( diff --git a/umap/views.py b/umap/views.py index b55c9020..0d305b90 100644 --- a/umap/views.py +++ b/umap/views.py @@ -61,6 +61,7 @@ from .forms import ( DataLayerForm, DataLayerPermissionsForm, FlatErrorList, + GroupForm, MapSettingsForm, SendLinkForm, UpdateMapPermissionsForm, @@ -189,6 +190,63 @@ class About(Home): about = About.as_view() +class GroupNew(CreateView): + model = Group + fields = ["name"] + success_url = reverse_lazy("user_groups") + + def form_valid(self, form): + response = super().form_valid(form) + self.request.user.groups.add(self.object) + self.request.user.save() + return response + + +class GroupUpdate(UpdateView): + model = Group + form_class = GroupForm + success_url = reverse_lazy("user_groups") + + def get_initial(self): + initial = super().get_initial() + initial["members"] = self.object.user_set.all() + return initial + + def form_valid(self, form): + for user in form.cleaned_data["members"]: + user.groups.add(self.object) + user.save() + return super().form_valid(form) + + +class GroupDelete(DeleteView): + model = Group + success_url = reverse_lazy("user_groups") + + def form_valid(self, form): + if self.object.user_set.count() > 1: + return HttpResponseBadRequest( + _("Cannot delete a group with more than one member") + ) + messages.info( + self.request, + _("Group “%(name)s” has been deleted") % {"name": self.object.name}, + ) + return super().form_valid(form) + + +class UserGroups(DetailView): + model = User + template_name = "umap/user_groups.html" + + def get_object(self): + return self.get_queryset().get(pk=self.request.user.pk) + + def get_context_data(self, **kwargs): + kwargs.update({"groups": self.object.groups.all()}) + return super().get_context_data(**kwargs) + + class UserProfile(UpdateView): model = User form_class = UserProfileForm From eccbbda44d579893b911ad932808037aa7fc2ea1 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Tue, 20 Aug 2024 11:05:59 +0200 Subject: [PATCH 04/14] wip: add basic tests for group views --- umap/templates/auth/group_form.html | 2 +- umap/tests/test_group_views.py | 98 +++++++++++++++++++++++++++++ umap/urls.py | 12 ++-- 3 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 umap/tests/test_group_views.py diff --git a/umap/templates/auth/group_form.html b/umap/templates/auth/group_form.html index 3709a8c8..6a6261c0 100644 --- a/umap/templates/auth/group_form.html +++ b/umap/templates/auth/group_form.html @@ -21,7 +21,7 @@ {% if group.user_set.count == 1 %} - {% trans "Delete this group" %} + {% trans "Delete this team" %} {% endif %}
diff --git a/umap/tests/test_group_views.py b/umap/tests/test_group_views.py new file mode 100644 index 00000000..0138e38a --- /dev/null +++ b/umap/tests/test_group_views.py @@ -0,0 +1,98 @@ +import pytest +from django.urls import reverse +from django.contrib.auth.models import Group + +pytestmark = pytest.mark.django_db + + +def test_can_see_group_maps(client, map, group): + map.group = group + map.save() + url = reverse("group_maps", args=(group.pk,)) + response = client.get(url) + assert response.status_code == 200 + assert map.name in response.content.decode() + + +def test_user_can_see_their_groups(client, group, user): + user.groups.add(group) + user.save() + url = reverse("user_groups") + client.login(username=user.username, password="123123") + response = client.get(url) + assert response.status_code == 200 + assert group.name in response.content.decode() + + +def test_can_create_a_group(client, user): + assert not Group.objects.count() + url = reverse("group_new") + client.login(username=user.username, password="123123") + response = client.post(url, {"name": "my new group", "members": [user.pk]}) + assert response.status_code == 302 + assert response["Location"] == "/en/me/groups" + assert Group.objects.count() == 1 + group = Group.objects.first() + assert group.name == "my new group" + assert group in user.groups.all() + + +def test_can_edit_a_group(client, user, 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": "my new group", "members": [user.pk]}) + assert response.status_code == 302 + assert response["Location"] == "/en/me/groups" + assert Group.objects.count() == 1 + modified = Group.objects.first() + assert modified.name == "my new group" + assert modified in user.groups.all() + + +def test_cannot_edit_a_group_if_not_member(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=user2.username, password="123123") + response = client.post(url, {"name": "my new group", "members": [user.pk]}) + assert response.status_code == 403 + + +def test_can_delete_a_group(client, user, group): + user.groups.add(group) + user.save() + assert Group.objects.count() == 1 + url = reverse("group_delete", args=(group.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url) + assert response.status_code == 302 + assert response["Location"] == "/en/me/groups" + assert Group.objects.count() == 0 + + +def test_cannot_delete_a_group_if_not_member(client, user, user2, group): + user.groups.add(group) + user.save() + assert Group.objects.count() == 1 + url = reverse("group_delete", args=(group.pk,)) + client.login(username=user2.username, password="123123") + response = client.post(url) + assert response.status_code == 403 + assert Group.objects.count() == 1 + + +def test_cannot_delete_a_group_if_more_than_one_member(client, user, user2, group): + user.groups.add(group) + user.save() + user2.groups.add(group) + user2.save() + assert Group.objects.count() == 1 + url = reverse("group_delete", args=(group.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url) + assert response.status_code == 400 + assert Group.objects.count() == 1 diff --git a/umap/urls.py b/umap/urls.py index 02accb2d..0f466ae4 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -102,7 +102,7 @@ i18n_urls += decorated_patterns( ) i18n_urls += decorated_patterns( [login_required_if_not_anonymous_allowed, never_cache], - re_path(r"^map/create/$", views.MapCreate.as_view(), name="map_create"), + path("map/create/", views.MapCreate.as_view(), name="map_create"), ) i18n_urls += decorated_patterns( [login_required], @@ -187,12 +187,10 @@ datalayer_urls = [ i18n_urls += decorated_patterns([can_edit_map, never_cache], *map_urls) i18n_urls += decorated_patterns([never_cache], *datalayer_urls) urlpatterns += i18n_patterns( - re_path(r"^$", views.home, name="home"), - re_path( - r"^showcase/$", cache_page(24 * 60 * 60)(views.showcase), name="maps_showcase" - ), - re_path(r"^search/$", views.search, name="search"), - re_path(r"^about/$", views.about, name="about"), + path("", views.home, name="home"), + path("showcase/", cache_page(24 * 60 * 60)(views.showcase), name="maps_showcase"), + path("search/", views.search, name="search"), + path("about/", views.about, name="about"), re_path(r"^user/(?P.+)/stars/$", views.user_stars, name="user_stars"), re_path(r"^user/(?P.+)/$", views.user_maps, name="user_maps"), path("group//", views.group_maps, name="group_maps"), From a5a68cc92210290424cadc6c919751f94b5b9644 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Tue, 20 Aug 2024 11:30:56 +0200 Subject: [PATCH 05/14] wip: show user groups maps in dashboard --- umap/templates/umap/user_groups.html | 2 +- umap/tests/test_views.py | 14 ++++++++++++++ umap/views.py | 7 ++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/umap/templates/umap/user_groups.html b/umap/templates/umap/user_groups.html index 6a0e5668..3e14ccb3 100644 --- a/umap/templates/umap/user_groups.html +++ b/umap/templates/umap/user_groups.html @@ -9,7 +9,7 @@ diff --git a/umap/tests/test_views.py b/umap/tests/test_views.py index 4e307ddb..1a0564a4 100644 --- a/umap/tests/test_views.py +++ b/umap/tests/test_views.py @@ -288,6 +288,20 @@ def test_user_dashboard_display_user_maps(client, map): assert "Owner only" in body +@pytest.mark.django_db +def test_user_dashboard_display_user_group_maps(client, map, group, user): + user.groups.add(group) + user.save() + map.group = group + map.save() + client.login(username=user.username, password="123123") + response = client.get(reverse("user_dashboard")) + assert response.status_code == 200 + body = response.content.decode() + assert map.name in body + assert map.get_absolute_url() in body + + @pytest.mark.django_db def test_user_dashboard_display_user_maps_distinct(client, map): # cf https://github.com/umap-project/umap/issues/1325 diff --git a/umap/views.py b/umap/views.py index 0d305b90..12282317 100644 --- a/umap/views.py +++ b/umap/views.py @@ -370,7 +370,12 @@ class UserDashboard(PaginatorMixin, DetailView, SearchMixin): def get_maps(self): qs = self.get_search_queryset() or Map.objects.all() - qs = qs.filter(owner=self.object).union(qs.filter(editors=self.object)) + groups = self.object.groups.all() + qs = ( + qs.filter(owner=self.object) + .union(qs.filter(editors=self.object)) + .union(qs.filter(group__in=groups)) + ) return qs.order_by("-modified_at") def get_context_data(self, **kwargs): From 1058e6074f8c3c0c900b1d3fb219af382bf39e76 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Tue, 20 Aug 2024 11:33:34 +0200 Subject: [PATCH 06/14] chore: lint --- umap/templates/auth/group_form.html | 2 +- umap/templates/umap/user_groups.html | 2 +- umap/tests/test_group_views.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/umap/templates/auth/group_form.html b/umap/templates/auth/group_form.html index 6a6261c0..16ea81e6 100644 --- a/umap/templates/auth/group_form.html +++ b/umap/templates/auth/group_form.html @@ -18,7 +18,7 @@
{% csrf_token %} {{ form }} - +
{% if group.user_set.count == 1 %} {% trans "Delete this team" %} diff --git a/umap/templates/umap/user_groups.html b/umap/templates/umap/user_groups.html index 3e14ccb3..1273d9ec 100644 --- a/umap/templates/umap/user_groups.html +++ b/umap/templates/umap/user_groups.html @@ -16,4 +16,4 @@ {% trans "New team" %}
-{% endblock %} +{% endblock maincontent %} diff --git a/umap/tests/test_group_views.py b/umap/tests/test_group_views.py index 0138e38a..d880f850 100644 --- a/umap/tests/test_group_views.py +++ b/umap/tests/test_group_views.py @@ -1,6 +1,6 @@ import pytest -from django.urls import reverse from django.contrib.auth.models import Group +from django.urls import reverse pytestmark = pytest.mark.django_db From 6b6be017bb6f6e890e41efc106d79e5f86750b32 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 21 Aug 2024 11:34:59 +0200 Subject: [PATCH 07/14] 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) From 13735a57393545d2c71c9838b0c3e6d2aa20c48a Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 29 Aug 2024 21:59:45 +0200 Subject: [PATCH 08/14] chore: use our own Team model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We suppose we'll quickly want more than a name, like a description or a logo_url, and maybe a access_status or permissions… --- umap/decorators.py | 9 +- umap/forms.py | 17 ++- ...it_status_and_more.py => 0022_add_team.py} | 49 ++++++- umap/models.py | 52 ++++--- umap/static/umap/base.css | 2 +- umap/static/umap/content.css | 3 +- umap/static/umap/js/modules/permissions.js | 12 +- umap/static/umap/js/umap.forms.js | 8 +- umap/templates/auth/group_detail.html | 22 --- umap/templates/auth/group_form.html | 58 -------- umap/templates/umap/dashboard_menu.html | 4 +- .../team_confirm_delete.html} | 2 +- umap/templates/umap/team_detail.html | 30 ++++ umap/templates/umap/team_form.html | 60 ++++++++ umap/templates/umap/user_groups.html | 19 --- umap/templates/umap/user_teams.html | 19 +++ umap/tests/base.py | 9 +- umap/tests/conftest.py | 6 +- umap/tests/integration/test_caption.py | 6 +- umap/tests/integration/test_owned_map.py | 8 +- .../{test_group.py => test_team.py} | 33 ++--- umap/tests/test_datalayer.py | 16 +-- umap/tests/test_group_views.py | 130 ----------------- umap/tests/test_map.py | 18 +-- umap/tests/test_team_views.py | 131 ++++++++++++++++++ umap/tests/test_views.py | 6 +- umap/urls.py | 14 +- umap/views.py | 70 +++++----- 28 files changed, 435 insertions(+), 378 deletions(-) rename umap/migrations/{0022_map_group_alter_datalayer_edit_status_and_more.py => 0022_add_team.py} (56%) delete mode 100644 umap/templates/auth/group_detail.html delete mode 100644 umap/templates/auth/group_form.html rename umap/templates/{auth/group_confirm_delete.html => umap/team_confirm_delete.html} (85%) create mode 100644 umap/templates/umap/team_detail.html create mode 100644 umap/templates/umap/team_form.html delete mode 100644 umap/templates/umap/user_groups.html create mode 100644 umap/templates/umap/user_teams.html rename umap/tests/integration/{test_group.py => test_team.py} (58%) delete mode 100644 umap/tests/test_group_views.py create mode 100644 umap/tests/test_team_views.py diff --git a/umap/decorators.py b/umap/decorators.py index f0187cbc..42b08d5f 100644 --- a/umap/decorators.py +++ b/umap/decorators.py @@ -1,12 +1,11 @@ from functools import wraps from django.conf import settings -from django.contrib.auth.models import Group from django.http import HttpResponseForbidden from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy -from .models import Map +from .models import Map, Team from .views import simple_json_response LOGIN_URL = getattr(settings, "LOGIN_URL", "login") @@ -63,11 +62,11 @@ def can_view_map(view_func): return wrapper -def group_members_only(view_func): +def team_members_only(view_func): @wraps(view_func) def wrapper(request, *args, **kwargs): - group = get_object_or_404(Group, pk=kwargs["pk"]) - if group not in request.user.groups.all(): + team = get_object_or_404(Team, pk=kwargs["pk"]) + if not request.user.is_authenticated or team not in request.user.teams.all(): return HttpResponseForbidden() return view_func(request, *args, **kwargs) diff --git a/umap/forms.py b/umap/forms.py index 15f39514..0ec29ccb 100644 --- a/umap/forms.py +++ b/umap/forms.py @@ -1,13 +1,12 @@ from django import forms from django.conf import settings from django.contrib.auth import get_user_model -from django.contrib.auth.models import Group from django.contrib.gis.geos import Point from django.forms.utils import ErrorList from django.template.defaultfilters import slugify from django.utils.translation import gettext_lazy as _ -from .models import DataLayer, Map +from .models import DataLayer, Map, Team DEFAULT_LATITUDE = ( settings.LEAFLET_LATITUDE if hasattr(settings, "LEAFLET_LATITUDE") else 51 @@ -37,7 +36,7 @@ class SendLinkForm(forms.Form): class UpdateMapPermissionsForm(forms.ModelForm): class Meta: model = Map - fields = ("edit_status", "editors", "share_status", "owner", "group") + fields = ("edit_status", "editors", "share_status", "owner", "team") class AnonymousMapPermissionsForm(forms.ModelForm): @@ -113,25 +112,25 @@ class UserProfileForm(forms.ModelForm): fields = ("username", "first_name", "last_name") -class GroupMembersField(forms.ModelMultipleChoiceField): +class TeamMembersField(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 + # already existing members of the team iterator.queryset = choices self.choices = iterator -class GroupForm(forms.ModelForm): +class TeamForm(forms.ModelForm): class Meta: - model = Group - fields = ["name", "members"] + model = Team + fields = ["name", "description", "logo_url", "members"] 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()) + members = TeamMembersField(queryset=User.objects.all()) diff --git a/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py b/umap/migrations/0022_add_team.py similarity index 56% rename from umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py rename to umap/migrations/0022_add_team.py index 511a8678..8163c96b 100644 --- a/umap/migrations/0022_map_group_alter_datalayer_edit_status_and_more.py +++ b/umap/migrations/0022_add_team.py @@ -1,27 +1,62 @@ # Generated by Django 5.1 on 2024-08-15 11:33 import django.db.models.deletion +from django.conf import settings from django.db import migrations, models import umap.models class Migration(migrations.Migration): - dependencies = [ - ("auth", "0012_alter_user_first_name_max_length"), - ("umap", "0021_remove_map_description"), - ] + dependencies = [("umap", "0021_remove_map_description")] operations = [ + migrations.CreateModel( + name="Team", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "name", + models.CharField(max_length=200, unique=True, verbose_name="name"), + ), + ( + "description", + models.TextField(blank=True, null=True, verbose_name="description"), + ), + ( + "logo_url", + models.URLField( + help_text="URL to an image.", + verbose_name="Logo URL", + blank=True, + null=True, + ), + ), + ( + "users", + models.ManyToManyField( + related_name="teams", to=settings.AUTH_USER_MODEL + ), + ), + ], + ), migrations.AddField( model_name="map", - name="group", + name="team", field=models.ForeignKey( blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, - to="auth.group", - verbose_name="group", + to="umap.team", + verbose_name="team", ), ), migrations.AlterField( diff --git a/umap/models.py b/umap/models.py index d4fbae30..f0c786dc 100644 --- a/umap/models.py +++ b/umap/models.py @@ -5,7 +5,7 @@ import time import uuid from django.conf import settings -from django.contrib.auth.models import Group, User +from django.contrib.auth.models import User from django.contrib.gis.db import models from django.core.files.base import File from django.core.signing import Signer @@ -36,19 +36,9 @@ def get_user_stars_url(self): return reverse("user_stars", kwargs={"identifier": identifier}) -def get_group_url(self): - return reverse("group_maps", kwargs={"pk": self.pk}) - - -def get_group_metadata(self): - return {"id": self.pk, "name": self.name, "url": self.get_url()} - - User.add_to_class("__str__", display_name) User.add_to_class("get_url", get_user_url) User.add_to_class("get_stars_url", get_user_stars_url) -Group.add_to_class("get_url", get_group_url) -Group.add_to_class("get_metadata", get_group_metadata) def get_default_share_status(): @@ -59,6 +49,32 @@ def get_default_edit_status(): return settings.UMAP_DEFAULT_EDIT_STATUS or Map.OWNER +class Team(models.Model): + name = models.CharField( + max_length=200, verbose_name=_("name"), unique=True, blank=False, null=False + ) + description = models.TextField(blank=True, null=True, verbose_name=_("description")) + logo_url = models.URLField( + verbose_name=_("Logo URL"), + help_text=_("URL to an image."), + null=True, + blank=True, + ) + users = models.ManyToManyField(User, related_name="teams") + + def __unicode__(self): + return self.name + + def __str__(self): + return self.name + + def get_url(self): + return reverse("team_maps", kwargs={"pk": self.pk}) + + def get_metadata(self): + return {"id": self.pk, "name": self.name, "url": self.get_url()} + + class NamedModel(models.Model): name = models.CharField(max_length=200, verbose_name=_("name")) @@ -190,8 +206,8 @@ class Map(NamedModel): editors = models.ManyToManyField( settings.AUTH_USER_MODEL, blank=True, verbose_name=_("editors") ) - group = models.ForeignKey( - "auth.Group", + team = models.ForeignKey( + Team, blank=True, null=True, verbose_name=_("team"), @@ -269,7 +285,7 @@ class Map(NamedModel): return settings.SITE_URL + path def get_author(self): - return self.group or self.owner + return self.team or self.owner def is_owner(self, user=None, request=None): if user and self.owner == user: @@ -301,7 +317,7 @@ class Map(NamedModel): In owner mode: - only owner by default (OWNER) - - any editor or group member if mode is COLLABORATORS + - any editor or team member if mode is COLLABORATORS - anyone otherwise (ANONYMOUS) In anonymous owner mode: - only owner (has ownership cookie) by default (OWNER) @@ -318,7 +334,7 @@ class Map(NamedModel): elif user == self.owner: can = True elif self.edit_status == self.COLLABORATORS: - if user in self.editors.all() or self.group in user.groups.all(): + if user in self.editors.all() or self.team in user.teams.all(): can = True return can @@ -337,7 +353,7 @@ class Map(NamedModel): can = not ( self.share_status == self.PRIVATE and request.user not in self.editors.all() - and self.group not in request.user.groups.all() + and self.team not in request.user.teams.all() ) return can @@ -563,7 +579,7 @@ class DataLayer(NamedModel): elif user is not None and user == self.map.owner: can = True elif user is not None and self.edit_status == self.COLLABORATORS: - if user in self.map.editors.all() or self.map.group in user.groups.all(): + if user in self.map.editors.all() or self.map.team in user.teams.all(): can = True return can diff --git a/umap/static/umap/base.css b/umap/static/umap/base.css index 82cba270..f233ff83 100644 --- a/umap/static/umap/base.css +++ b/umap/static/umap/base.css @@ -345,7 +345,7 @@ input + .error { input[type="file"] + .error { margin-top: 0; } -input:invalid { +input[value]:invalid { border-color: red; background-color: darkred; } diff --git a/umap/static/umap/content.css b/umap/static/umap/content.css index 6a9fe92e..f723fd35 100644 --- a/umap/static/umap/content.css +++ b/umap/static/umap/content.css @@ -133,6 +133,8 @@ h2.section { text-align: center; } h2.tabs { + display: flex; + justify-content: space-around; text-transform: uppercase; color: var(--color-darkBlue); text-align: start; @@ -144,7 +146,6 @@ h2.tabs a { text-decoration-thickness: 3px; text-decoration-skip-ink: none; text-underline-offset: 7px; - margin-inline-end: 2rem; display: inline-block; } h2.tabs a:not(.selected) { diff --git a/umap/static/umap/js/modules/permissions.js b/umap/static/umap/js/modules/permissions.js index 58c230a7..2dde4002 100644 --- a/umap/static/umap/js/modules/permissions.js +++ b/umap/static/umap/js/modules/permissions.js @@ -25,7 +25,7 @@ export class MapPermissions { this.options = Object.assign( { owner: null, - group: null, + team: null, editors: [], share_status: null, edit_status: null, @@ -97,13 +97,13 @@ export class MapPermissions { 'options.owner', { handler: 'ManageOwner', label: translate("Map's owner") }, ]) - if (this.map.options.user?.groups?.length) { + if (this.map.options.user?.teams?.length) { fields.push([ - 'options.group', + 'options.team', { - handler: 'ManageGroup', + handler: 'ManageTeam', label: translate('Attach map to a team'), - groups: this.map.options.user.groups, + teams: this.map.options.user.teams, }, ]) } @@ -161,7 +161,7 @@ export class MapPermissions { formData.append('edit_status', this.options.edit_status) if (this.isOwner()) { formData.append('owner', this.options.owner?.id) - formData.append('group', this.options.group?.id || '') + formData.append('team', this.options.team?.id || '') formData.append('share_status', this.options.share_status) } const [data, response, error] = await this.map.server.post( diff --git a/umap/static/umap/js/umap.forms.js b/umap/static/umap/js/umap.forms.js index 15dc3311..8eb730eb 100644 --- a/umap/static/umap/js/umap.forms.js +++ b/umap/static/umap/js/umap.forms.js @@ -1086,10 +1086,10 @@ L.FormBuilder.ManageEditors = L.FormBuilder.Element.extend({ }, }) -L.FormBuilder.ManageGroup = L.FormBuilder.IntSelect.extend({ +L.FormBuilder.ManageTeam = L.FormBuilder.IntSelect.extend({ getOptions: function () { return [[null, L._('None')]].concat( - this.options.groups.map((group) => [group.id, group.name]) + this.options.teams.map((team) => [team.id, team.name]) ) }, toHTML: function () { @@ -1097,8 +1097,8 @@ L.FormBuilder.ManageGroup = L.FormBuilder.IntSelect.extend({ }, toJS: function () { const value = this.value() - for (const group of this.options.groups) { - if (group.id === value) return group + for (const team of this.options.teams) { + if (team.id === value) return team } }, }) diff --git a/umap/templates/auth/group_detail.html b/umap/templates/auth/group_detail.html deleted file mode 100644 index bd044b00..00000000 --- a/umap/templates/auth/group_detail.html +++ /dev/null @@ -1,22 +0,0 @@ -{% extends "umap/content.html" %} - -{% load i18n %} - -{% block maincontent %} -
-

- {% blocktrans %}Browse {{ current_group }}'s maps{% endblocktrans %} -

-
-
-
- {% if maps %} - {% include "umap/map_list.html" %} - {% else %} -
- {% blocktrans %}{{ current_group }} has no public maps.{% endblocktrans %} -
- {% endif %} -
-
-{% endblock maincontent %} diff --git a/umap/templates/auth/group_form.html b/umap/templates/auth/group_form.html deleted file mode 100644 index 45af6d13..00000000 --- a/umap/templates/auth/group_form.html +++ /dev/null @@ -1,58 +0,0 @@ -{% extends "umap/content.html" %} - -{% load i18n %} - -{% block maincontent %} - {% include "umap/dashboard_menu.html" with selected="groups" %} -
-
- {% if form.non_field_errors %} -
    - {% for error in form.non_field_errors %} -
  • - {{ error }} -
  • - {% endfor %} -
- {% endif %} -
- {% csrf_token %} - {{ form }} - -
- {% if group.user_set.count == 1 %} - {% trans "Delete this team" %} - {% endif %} -
-
- -{% endblock maincontent %} diff --git a/umap/templates/umap/dashboard_menu.html b/umap/templates/umap/dashboard_menu.html index 519abe8f..ac07420f 100644 --- a/umap/templates/umap/dashboard_menu.html +++ b/umap/templates/umap/dashboard_menu.html @@ -9,7 +9,7 @@ {% endif %} {% trans "My profile" %} - {% trans "My teams" %} + {% trans "My teams" %} diff --git a/umap/templates/auth/group_confirm_delete.html b/umap/templates/umap/team_confirm_delete.html similarity index 85% rename from umap/templates/auth/group_confirm_delete.html rename to umap/templates/umap/team_confirm_delete.html index 31283229..deceb4b1 100644 --- a/umap/templates/auth/group_confirm_delete.html +++ b/umap/templates/umap/team_confirm_delete.html @@ -3,7 +3,7 @@ {% load i18n %} {% block maincontent %} - {% include "umap/dashboard_menu.html" with selected="groups" %} + {% include "umap/dashboard_menu.html" with selected="teams" %}
diff --git a/umap/templates/umap/team_detail.html b/umap/templates/umap/team_detail.html new file mode 100644 index 00000000..e5112376 --- /dev/null +++ b/umap/templates/umap/team_detail.html @@ -0,0 +1,30 @@ +{% extends "umap/content.html" %} + +{% load i18n %} + +{% block maincontent %} +
+
+
+

+ {% blocktrans %}Browse {{ current_team }}'s maps{% endblocktrans %} +

+ {% if current_team.description %} +

{{ current_team.description }}

+ {% endif %} + {% if current_team.logo_url %} +

+ {% endif %} +
+
+
+ {% if maps %} + {% include "umap/map_list.html" %} + {% else %} +
+ {% blocktrans %}{{ current_team }} has no public maps.{% endblocktrans %} +
+ {% endif %} +
+
+{% endblock maincontent %} diff --git a/umap/templates/umap/team_form.html b/umap/templates/umap/team_form.html new file mode 100644 index 00000000..a8501ac4 --- /dev/null +++ b/umap/templates/umap/team_form.html @@ -0,0 +1,60 @@ +{% extends "umap/content.html" %} + +{% load i18n %} + +{% block maincontent %} + {% include "umap/dashboard_menu.html" with selected="teams" %} +
+
+ {% if form.non_field_errors %} +
    + {% for error in form.non_field_errors %} +
  • + {{ error }} +
  • + {% endfor %} +
+ {% endif %} + + {% csrf_token %} + {{ form }} + + + {% if team.users.count == 1 %} + {% trans "Delete this team" %} + {% endif %} +
+
+ +{% endblock maincontent %} diff --git a/umap/templates/umap/user_groups.html b/umap/templates/umap/user_groups.html deleted file mode 100644 index 1273d9ec..00000000 --- a/umap/templates/umap/user_groups.html +++ /dev/null @@ -1,19 +0,0 @@ -{% extends "umap/content.html" %} - -{% load i18n %} - -{% block maincontent %} - {% include "umap/dashboard_menu.html" with selected="groups" %} -
-
- - {% trans "New team" %} -
-
-{% endblock maincontent %} diff --git a/umap/templates/umap/user_teams.html b/umap/templates/umap/user_teams.html new file mode 100644 index 00000000..d84296c3 --- /dev/null +++ b/umap/templates/umap/user_teams.html @@ -0,0 +1,19 @@ +{% extends "umap/content.html" %} + +{% load i18n %} + +{% block maincontent %} + {% include "umap/dashboard_menu.html" with selected="teams" %} +
+
+ + {% trans "New team" %} +
+
+{% endblock maincontent %} diff --git a/umap/tests/base.py b/umap/tests/base.py index 4d0ce144..ea5107a8 100644 --- a/umap/tests/base.py +++ b/umap/tests/base.py @@ -3,12 +3,11 @@ import json import factory from django.contrib.auth import get_user_model -from django.contrib.auth.models import Group from django.core.files.base import ContentFile from django.urls import reverse from umap.forms import DEFAULT_CENTER -from umap.models import DataLayer, Licence, Map, TileLayer +from umap.models import DataLayer, Licence, Map, TileLayer, Team User = get_user_model() @@ -59,11 +58,11 @@ class UserFactory(factory.django.DjangoModelFactory): model = User -class GroupFactory(factory.django.DjangoModelFactory): - name = "Awesome Group" +class TeamFactory(factory.django.DjangoModelFactory): + name = "Awesome Team" class Meta: - model = Group + model = Team class MapFactory(factory.django.DjangoModelFactory): diff --git a/umap/tests/conftest.py b/umap/tests/conftest.py index 3bee02e7..2f12753e 100644 --- a/umap/tests/conftest.py +++ b/umap/tests/conftest.py @@ -9,7 +9,7 @@ from umap.models import Map from .base import ( DataLayerFactory, - GroupFactory, + TeamFactory, LicenceFactory, MapFactory, TileLayerFactory, @@ -31,8 +31,8 @@ def pytest_runtest_teardown(): @pytest.fixture -def group(): - return GroupFactory() +def team(): + return TeamFactory() @pytest.fixture diff --git a/umap/tests/integration/test_caption.py b/umap/tests/integration/test_caption.py index e5f2b7eb..f9228fe5 100644 --- a/umap/tests/integration/test_caption.py +++ b/umap/tests/integration/test_caption.py @@ -36,12 +36,12 @@ def test_caption_should_display_owner_as_author(live_server, page, map): expect(panel.get_by_text("By Gabriel")).to_be_visible() -def test_caption_should_display_group_as_author(live_server, page, map, group): +def test_caption_should_display_team_as_author(live_server, page, map, team): map.settings["properties"]["onLoadPanel"] = "caption" - map.group = group + map.team = team map.save() page.goto(f"{live_server.url}{map.get_absolute_url()}") panel = page.locator(".panel.left.on") expect(panel).to_be_visible() expect(panel.get_by_text("By Gabriel")).to_be_hidden() - expect(panel.get_by_text("By Awesome Group")).to_be_visible() + expect(panel.get_by_text("By Awesome Team")).to_be_visible() diff --git a/umap/tests/integration/test_owned_map.py b/umap/tests/integration/test_owned_map.py index 19d02804..77518d69 100644 --- a/umap/tests/integration/test_owned_map.py +++ b/umap/tests/integration/test_owned_map.py @@ -243,17 +243,17 @@ def test_can_delete_datalayer(live_server, map, login, datalayer): expect(layers).to_have_count(0) -def test_can_set_group(map, live_server, login, group): - map.owner.groups.add(group) +def test_can_set_team(map, live_server, login, team): + map.owner.teams.add(team) map.owner.save() page = login(map.owner) page.goto(f"{live_server.url}{map.get_absolute_url()}?edit") edit_permissions = page.get_by_title("Update permissions and editors") edit_permissions.click() - page.locator("select[name=group]").select_option(str(group.pk)) + page.locator("select[name=team]").select_option(str(team.pk)) save = page.get_by_role("button", name="Save") expect(save).to_be_visible() with page.expect_response(re.compile(r".*/update/permissions/.*")): save.click() modified = Map.objects.get(pk=map.pk) - assert modified.group == group + assert modified.team == team diff --git a/umap/tests/integration/test_group.py b/umap/tests/integration/test_team.py similarity index 58% rename from umap/tests/integration/test_group.py rename to umap/tests/integration/test_team.py index bd9a4746..0a3beea4 100644 --- a/umap/tests/integration/test_group.py +++ b/umap/tests/integration/test_team.py @@ -1,15 +1,16 @@ import re import pytest -from django.contrib.auth.models import Group + +from umap.models import Team pytestmark = pytest.mark.django_db -def test_can_add_user_to_group(live_server, map, user, group, login): - map.owner.groups.add(group) +def test_can_add_user_to_team(live_server, map, user, team, login): + map.owner.teams.add(team) map.owner.save() - assert Group.objects.count() == 1 + assert Team.objects.count() == 1 page = login(map.owner) with page.expect_navigation(): page.get_by_role("link", name="My teams").click() @@ -20,19 +21,19 @@ def test_can_add_user_to_group(live_server, map, user, group, login): 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() + assert Team.objects.count() == 1 + modified = Team.objects.first() + assert user in modified.users.all() -def test_can_remove_user_from_group(live_server, map, user, user2, group, login): - map.owner.groups.add(group) +def test_can_remove_user_from_team(live_server, map, user, user2, team, login): + map.owner.teams.add(team) map.owner.save() - user.groups.add(group) + user.teams.add(team) user.save() - user2.groups.add(group) + user2.teams.add(team) user2.save() - assert Group.objects.count() == 1 + assert Team.objects.count() == 1 page = login(map.owner) with page.expect_navigation(): page.get_by_role("link", name="My teams").click() @@ -40,7 +41,7 @@ def test_can_remove_user_from_group(live_server, map, user, user2, group, login) 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() + assert Team.objects.count() == 1 + modified = Team.objects.first() + assert user in modified.users.all() + assert user2 not in modified.users.all() diff --git a/umap/tests/test_datalayer.py b/umap/tests/test_datalayer.py index c60ab3c0..99b0f5bb 100644 --- a/umap/tests/test_datalayer.py +++ b/umap/tests/test_datalayer.py @@ -121,11 +121,11 @@ def test_editor_can_edit_in_collaborators_mode(datalayer, user): assert datalayer.can_edit(user) -def test_group_members_can_edit_in_collaborators_mode(datalayer, user, group): - user.groups.add(group) +def test_team_members_can_edit_in_collaborators_mode(datalayer, user, team): + user.teams.add(team) user.save() map = datalayer.map - map.group = group + map.team = team map.save() datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() @@ -181,15 +181,15 @@ def test_editors_cannot_edit_in_inherit_mode_and_map_in_owner_mode(datalayer, us assert not datalayer.can_edit(user) -def test_group_members_cannot_edit_in_inherit_mode_and_map_in_owner_mode( - datalayer, user, group +def test_team_members_cannot_edit_in_inherit_mode_and_map_in_owner_mode( + datalayer, user, team ): datalayer.edit_status = DataLayer.INHERIT datalayer.save() - user.groups.add(group) - group.save() + user.teams.add(team) + team.save() map = datalayer.map - map.group = group + map.team = team map.edit_status = Map.OWNER map.save() assert not datalayer.can_edit(user) diff --git a/umap/tests/test_group_views.py b/umap/tests/test_group_views.py deleted file mode 100644 index 0135c1bd..00000000 --- a/umap/tests/test_group_views.py +++ /dev/null @@ -1,130 +0,0 @@ -import pytest -from django.contrib.auth.models import Group -from django.urls import reverse - -pytestmark = pytest.mark.django_db - - -def test_can_see_group_maps(client, map, group): - map.group = group - map.save() - url = reverse("group_maps", args=(group.pk,)) - response = client.get(url) - assert response.status_code == 200 - assert map.name in response.content.decode() - - -def test_user_can_see_their_groups(client, group, user): - user.groups.add(group) - user.save() - url = reverse("user_groups") - client.login(username=user.username, password="123123") - response = client.get(url) - assert response.status_code == 200 - assert group.name in response.content.decode() - - -def test_can_create_a_group(client, user): - assert not Group.objects.count() - url = reverse("group_new") - client.login(username=user.username, password="123123") - response = client.post(url, {"name": "my new group", "members": [user.pk]}) - assert response.status_code == 302 - assert response["Location"] == "/en/me/groups" - assert Group.objects.count() == 1 - group = Group.objects.first() - assert group.name == "my new group" - assert group in user.groups.all() - - -def test_can_edit_a_group_name(client, user, 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": "my new group", "members": [user.pk]}) - assert response.status_code == 302 - assert response["Location"] == "/en/me/groups" - assert Group.objects.count() == 1 - modified = Group.objects.first() - assert modified.name == "my new 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() - assert Group.objects.count() == 1 - url = reverse("group_update", args=(group.pk,)) - client.login(username=user2.username, password="123123") - response = client.post(url, {"name": "my new group", "members": [user.pk]}) - assert response.status_code == 403 - - -def test_can_delete_a_group(client, user, group): - user.groups.add(group) - user.save() - assert Group.objects.count() == 1 - url = reverse("group_delete", args=(group.pk,)) - client.login(username=user.username, password="123123") - response = client.post(url) - assert response.status_code == 302 - assert response["Location"] == "/en/me/groups" - assert Group.objects.count() == 0 - - -def test_cannot_delete_a_group_if_not_member(client, user, user2, group): - user.groups.add(group) - user.save() - assert Group.objects.count() == 1 - url = reverse("group_delete", args=(group.pk,)) - client.login(username=user2.username, password="123123") - response = client.post(url) - assert response.status_code == 403 - assert Group.objects.count() == 1 - - -def test_cannot_delete_a_group_if_more_than_one_member(client, user, user2, group): - user.groups.add(group) - user.save() - user2.groups.add(group) - user2.save() - assert Group.objects.count() == 1 - url = reverse("group_delete", args=(group.pk,)) - client.login(username=user.username, password="123123") - response = client.post(url) - assert response.status_code == 400 - assert Group.objects.count() == 1 diff --git a/umap/tests/test_map.py b/umap/tests/test_map.py index a0eb83e0..ff134e4c 100644 --- a/umap/tests/test_map.py +++ b/umap/tests/test_map.py @@ -50,20 +50,20 @@ def test_editors_can_edit_if_status_collaborators(map, user): assert map.can_edit(user) -def test_group_members_cannot_edit_if_status_owner(map, user, group): - user.groups.add(group) +def test_team_members_cannot_edit_if_status_owner(map, user, team): + user.teams.add(team) user.save() map.edit_status = map.OWNER - map.group = group + map.team = team map.save() assert not map.can_edit(user) -def test_group_members_can_edit_if_status_collaborators(map, user, group): - user.groups.add(group) +def test_team_members_can_edit_if_status_collaborators(map, user, team): + user.teams.add(team) user.save() map.edit_status = map.COLLABORATORS - map.group = group + map.team = team map.save() assert map.can_edit(user) @@ -105,12 +105,12 @@ def test_clone_should_keep_editors(map, user): assert user in clone.editors.all() -def test_clone_should_keep_group(map, user, group): - map.group = group +def test_clone_should_keep_team(map, user, team): + map.team = team map.save() clone = map.clone() assert map.pk != clone.pk - assert clone.group == group + assert clone.team == team def test_clone_should_update_owner_if_passed(map, user): diff --git a/umap/tests/test_team_views.py b/umap/tests/test_team_views.py new file mode 100644 index 00000000..7bce42b0 --- /dev/null +++ b/umap/tests/test_team_views.py @@ -0,0 +1,131 @@ +import pytest +from django.urls import reverse + +from umap.models import Team + +pytestmark = pytest.mark.django_db + + +def test_can_see_team_maps(client, map, team): + map.team = team + map.save() + url = reverse("team_maps", args=(team.pk,)) + response = client.get(url) + assert response.status_code == 200 + assert map.name in response.content.decode() + + +def test_user_can_see_their_teams(client, team, user): + user.teams.add(team) + user.save() + url = reverse("user_teams") + client.login(username=user.username, password="123123") + response = client.get(url) + assert response.status_code == 200 + assert team.name in response.content.decode() + + +def test_can_create_a_team(client, user): + assert not Team.objects.count() + url = reverse("team_new") + client.login(username=user.username, password="123123") + response = client.post(url, {"name": "my new team", "members": [user.pk]}) + assert response.status_code == 302 + assert response["Location"] == "/en/me/teams" + assert Team.objects.count() == 1 + team = Team.objects.first() + assert team.name == "my new team" + assert team in user.teams.all() + + +def test_can_edit_a_team_name(client, user, team): + user.teams.add(team) + user.save() + assert Team.objects.count() == 1 + url = reverse("team_update", args=(team.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url, {"name": "my new team", "members": [user.pk]}) + assert response.status_code == 302 + assert response["Location"] == "/en/me/teams" + assert Team.objects.count() == 1 + modified = Team.objects.first() + assert modified.name == "my new team" + assert modified in user.teams.all() + + +def test_can_add_user_to_team(client, user, user2, team): + user.teams.add(team) + user.save() + assert Team.objects.count() == 1 + url = reverse("team_update", args=(team.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url, {"name": team.name, "members": [user.pk, user2.pk]}) + assert response.status_code == 302 + assert response["Location"] == "/en/me/teams" + assert Team.objects.count() == 1 + modified = Team.objects.first() + assert user in modified.users.all() + assert user2 in modified.users.all() + + +def test_can_remove_user_from_team(client, user, user2, team): + user.teams.add(team) + user.save() + user2.teams.add(team) + user2.save() + assert Team.objects.count() == 1 + url = reverse("team_update", args=(team.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url, {"name": team.name, "members": [user.pk]}) + assert response.status_code == 302 + assert response["Location"] == "/en/me/teams" + assert Team.objects.count() == 1 + modified = Team.objects.first() + assert user in modified.users.all() + assert user2 not in modified.users.all() + + +def test_cannot_edit_a_team_if_not_member(client, user, user2, team): + user.teams.add(team) + user.save() + assert Team.objects.count() == 1 + url = reverse("team_update", args=(team.pk,)) + client.login(username=user2.username, password="456456") + response = client.post(url, {"name": "my new team", "members": [user.pk]}) + assert response.status_code == 403 + + +def test_can_delete_a_team(client, user, team): + user.teams.add(team) + user.save() + assert Team.objects.count() == 1 + url = reverse("team_delete", args=(team.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url) + assert response.status_code == 302 + assert response["Location"] == "/en/me/teams" + assert Team.objects.count() == 0 + + +def test_cannot_delete_a_team_if_not_member(client, user, user2, team): + user.teams.add(team) + user.save() + assert Team.objects.count() == 1 + url = reverse("team_delete", args=(team.pk,)) + client.login(username=user2.username, password="456456") + response = client.post(url) + assert response.status_code == 403 + assert Team.objects.count() == 1 + + +def test_cannot_delete_a_team_if_more_than_one_member(client, user, user2, team): + user.teams.add(team) + user.save() + user2.teams.add(team) + user2.save() + assert Team.objects.count() == 1 + url = reverse("team_delete", args=(team.pk,)) + client.login(username=user.username, password="123123") + response = client.post(url) + assert response.status_code == 400 + assert Team.objects.count() == 1 diff --git a/umap/tests/test_views.py b/umap/tests/test_views.py index 1a0564a4..c6a6dbc3 100644 --- a/umap/tests/test_views.py +++ b/umap/tests/test_views.py @@ -289,10 +289,10 @@ def test_user_dashboard_display_user_maps(client, map): @pytest.mark.django_db -def test_user_dashboard_display_user_group_maps(client, map, group, user): - user.groups.add(group) +def test_user_dashboard_display_user_team_maps(client, map, team, user): + user.teams.add(team) user.save() - map.group = group + map.team = team map.save() client.login(username=user.username, password="123123") response = client.get(reverse("user_dashboard")) diff --git a/umap/urls.py b/umap/urls.py index ca75a7ab..f2d3821f 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -15,7 +15,7 @@ from . import views from .decorators import ( can_edit_map, can_view_map, - group_members_only, + team_members_only, login_required_if_not_anonymous_allowed, ) from .utils import decorated_patterns @@ -117,13 +117,13 @@ i18n_urls += decorated_patterns( path("me", views.user_dashboard, name="user_dashboard"), path("me/profile", views.user_profile, name="user_profile"), path("me/download", views.user_download, name="user_download"), - path("me/groups", views.UserGroups.as_view(), name="user_groups"), - path("group/create/", views.GroupNew.as_view(), name="group_new"), + path("me/teams", views.UserTeams.as_view(), name="user_teams"), + path("team/create/", views.TeamNew.as_view(), name="team_new"), ) i18n_urls += decorated_patterns( - [login_required, group_members_only], - path("group//edit/", views.GroupUpdate.as_view(), name="group_update"), - path("group//delete/", views.GroupDelete.as_view(), name="group_delete"), + [login_required, team_members_only], + path("team//edit/", views.TeamUpdate.as_view(), name="team_update"), + path("team//delete/", views.TeamDelete.as_view(), name="team_delete"), ) map_urls = [ re_path( @@ -196,7 +196,7 @@ urlpatterns += i18n_patterns( path("about/", views.about, name="about"), re_path(r"^user/(?P.+)/stars/$", views.user_stars, name="user_stars"), re_path(r"^user/(?P.+)/$", views.user_maps, name="user_maps"), - path("group//", views.group_maps, name="group_maps"), + path("team//", views.TeamMaps.as_view(), name="team_maps"), re_path(r"", include(i18n_urls)), ) urlpatterns += ( diff --git a/umap/views.py b/umap/views.py index 6eebf52e..21dc174a 100644 --- a/umap/views.py +++ b/umap/views.py @@ -18,7 +18,6 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth import get_user_model from django.contrib.auth import logout as do_logout -from django.contrib.auth.models import Group from django.contrib.gis.measure import D from django.contrib.postgres.search import SearchQuery, SearchVector from django.contrib.staticfiles.storage import staticfiles_storage @@ -61,13 +60,13 @@ from .forms import ( DataLayerForm, DataLayerPermissionsForm, FlatErrorList, - GroupForm, + TeamForm, MapSettingsForm, SendLinkForm, UpdateMapPermissionsForm, UserProfileForm, ) -from .models import DataLayer, Licence, Map, Pictogram, Star, TileLayer +from .models import DataLayer, Licence, Map, Pictogram, Star, Team, TileLayer from .utils import ( ConflictError, _urls_for_js, @@ -190,67 +189,67 @@ class About(Home): about = About.as_view() -class GroupNew(CreateView): - model = Group - fields = ["name"] - success_url = reverse_lazy("user_groups") +class TeamNew(CreateView): + model = Team + fields = ["name", "description", "logo_url"] + success_url = reverse_lazy("user_teams") def form_valid(self, form): response = super().form_valid(form) - self.request.user.groups.add(self.object) + self.request.user.teams.add(self.object) self.request.user.save() return response -class GroupUpdate(UpdateView): - model = Group - form_class = GroupForm - success_url = reverse_lazy("user_groups") +class TeamUpdate(UpdateView): + model = Team + form_class = TeamForm + success_url = reverse_lazy("user_teams") def get_initial(self): initial = super().get_initial() - initial["members"] = self.object.user_set.all() + initial["members"] = self.object.users.all() return initial def form_valid(self, form): - actual = self.object.user_set.all() + actual = self.object.users.all() wanted = form.cleaned_data["members"] for user in wanted: if user not in actual: - user.groups.add(self.object) + user.teams.add(self.object) user.save() for user in actual: if user not in wanted: - user.groups.remove(self.object) + user.teams.remove(self.object) user.save() return super().form_valid(form) -class GroupDelete(DeleteView): - model = Group - success_url = reverse_lazy("user_groups") +class TeamDelete(DeleteView): + model = Team + success_url = reverse_lazy("user_teams") def form_valid(self, form): - if self.object.user_set.count() > 1: + if self.object.users.count() > 1: return HttpResponseBadRequest( - _("Cannot delete a group with more than one member") + _("Cannot delete a team with more than one member") ) messages.info( self.request, - _("Group “%(name)s” has been deleted") % {"name": self.object.name}, + _("Team “%(name)s” has been deleted") % {"name": self.object.name}, ) return super().form_valid(form) -class UserGroups(DetailView): +class UserTeams(DetailView): model = User - template_name = "umap/user_groups.html" + template_name = "umap/user_teams.html" def get_object(self): return self.get_queryset().get(pk=self.request.user.pk) def get_context_data(self, **kwargs): - kwargs.update({"groups": self.object.groups.all()}) + kwargs.update({"teams": self.object.teams.all()}) return super().get_context_data(**kwargs) @@ -313,13 +312,13 @@ class UserStars(UserMaps): user_stars = UserStars.as_view() -class GroupMaps(PaginatorMixin, DetailView): - model = Group +class TeamMaps(PaginatorMixin, DetailView): + model = Team list_template_name = "umap/map_list.html" - context_object_name = "current_group" + context_object_name = "current_team" def get_maps(self): - return Map.public.filter(group=self.object).order_by("-modified_at") + return Map.public.filter(team=self.object).order_by("-modified_at") def get_context_data(self, **kwargs): kwargs.update( @@ -328,9 +327,6 @@ class GroupMaps(PaginatorMixin, DetailView): return super().get_context_data(**kwargs) -group_maps = GroupMaps.as_view() - - class SearchMixin: def get_search_queryset(self, **kwargs): q = self.request.GET.get("q") @@ -377,11 +373,11 @@ class UserDashboard(PaginatorMixin, DetailView, SearchMixin): def get_maps(self): qs = self.get_search_queryset() or Map.objects.all() - groups = self.object.groups.all() + teams = self.object.teams.all() qs = ( qs.filter(owner=self.object) .union(qs.filter(editors=self.object)) - .union(qs.filter(group__in=groups)) + .union(qs.filter(team__in=teams)) ) return qs.order_by("-modified_at") @@ -557,7 +553,7 @@ class SessionMixin: "id": user.pk, "name": str(self.request.user), "url": reverse("user_dashboard"), - "groups": [group.get_metadata() for group in user.groups.all()], + "teams": [team.get_metadata() for team in user.teams.all()], **data, } @@ -696,8 +692,8 @@ class PermissionsMixin: {"id": editor.pk, "name": str(editor)} for editor in self.object.editors.all() ] - if self.object.group: - permissions["group"] = self.object.group.get_metadata() + if self.object.team: + permissions["team"] = self.object.team.get_metadata() if not self.object.owner and self.object.is_anonymous_owner(self.request): permissions["anonymous_edit_url"] = self.object.get_anonymous_edit_url() return permissions From a877dd1714f756bb4a5faaee5753b6cd7c67fff9 Mon Sep 17 00:00:00 2001 From: David Larlet Date: Fri, 30 Aug 2024 10:55:26 -0400 Subject: [PATCH 09/14] chore: remove logo_url from Team model --- umap/forms.py | 2 +- umap/migrations/0022_add_team.py | 9 --------- umap/models.py | 6 ------ umap/templates/umap/team_detail.html | 3 --- umap/views.py | 4 ++-- 5 files changed, 3 insertions(+), 21 deletions(-) diff --git a/umap/forms.py b/umap/forms.py index 0ec29ccb..a6afade7 100644 --- a/umap/forms.py +++ b/umap/forms.py @@ -126,7 +126,7 @@ class TeamMembersField(forms.ModelMultipleChoiceField): class TeamForm(forms.ModelForm): class Meta: model = Team - fields = ["name", "description", "logo_url", "members"] + fields = ["name", "description", "members"] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/umap/migrations/0022_add_team.py b/umap/migrations/0022_add_team.py index 8163c96b..8037bca3 100644 --- a/umap/migrations/0022_add_team.py +++ b/umap/migrations/0022_add_team.py @@ -31,15 +31,6 @@ class Migration(migrations.Migration): "description", models.TextField(blank=True, null=True, verbose_name="description"), ), - ( - "logo_url", - models.URLField( - help_text="URL to an image.", - verbose_name="Logo URL", - blank=True, - null=True, - ), - ), ( "users", models.ManyToManyField( diff --git a/umap/models.py b/umap/models.py index f0c786dc..e4c5e719 100644 --- a/umap/models.py +++ b/umap/models.py @@ -54,12 +54,6 @@ class Team(models.Model): max_length=200, verbose_name=_("name"), unique=True, blank=False, null=False ) description = models.TextField(blank=True, null=True, verbose_name=_("description")) - logo_url = models.URLField( - verbose_name=_("Logo URL"), - help_text=_("URL to an image."), - null=True, - blank=True, - ) users = models.ManyToManyField(User, related_name="teams") def __unicode__(self): diff --git a/umap/templates/umap/team_detail.html b/umap/templates/umap/team_detail.html index e5112376..14f86ba7 100644 --- a/umap/templates/umap/team_detail.html +++ b/umap/templates/umap/team_detail.html @@ -12,9 +12,6 @@ {% if current_team.description %}

{{ current_team.description }}

{% endif %} - {% if current_team.logo_url %} -

- {% endif %}
diff --git a/umap/views.py b/umap/views.py index 21dc174a..b7efd747 100644 --- a/umap/views.py +++ b/umap/views.py @@ -60,9 +60,9 @@ from .forms import ( DataLayerForm, DataLayerPermissionsForm, FlatErrorList, - TeamForm, MapSettingsForm, SendLinkForm, + TeamForm, UpdateMapPermissionsForm, UserProfileForm, ) @@ -191,7 +191,7 @@ about = About.as_view() class TeamNew(CreateView): model = Team - fields = ["name", "description", "logo_url"] + fields = ["name", "description"] success_url = reverse_lazy("user_teams") def form_valid(self, form): From a202ed47671ba9f22c06e172a85e3c2e69d8b529 Mon Sep 17 00:00:00 2001 From: David Larlet Date: Fri, 30 Aug 2024 10:55:46 -0400 Subject: [PATCH 10/14] chore: add Team to django admin --- umap/admin.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/umap/admin.py b/umap/admin.py index 34ae2321..b92fb8e7 100644 --- a/umap/admin.py +++ b/umap/admin.py @@ -1,6 +1,6 @@ from django.contrib.gis import admin -from .models import DataLayer, Licence, Map, Pictogram, TileLayer +from .models import DataLayer, Licence, Map, Pictogram, Team, TileLayer class TileLayerAdmin(admin.ModelAdmin): @@ -26,8 +26,13 @@ class PictogramAdmin(admin.ModelAdmin): list_filter = ("category",) +class TeamAdmin(admin.ModelAdmin): + filter_horizontal = ("users",) + + admin.site.register(Map, MapAdmin) admin.site.register(DataLayer) admin.site.register(Pictogram, PictogramAdmin) admin.site.register(TileLayer, TileLayerAdmin) admin.site.register(Licence) +admin.site.register(Team, TeamAdmin) From 426780af2c40dc847ebda4d4e10d0f76d15c9073 Mon Sep 17 00:00:00 2001 From: David Larlet Date: Fri, 30 Aug 2024 11:03:00 -0400 Subject: [PATCH 11/14] chore: clean up CSS for buttons in content pages --- umap/static/umap/content.css | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/umap/static/umap/content.css b/umap/static/umap/content.css index f723fd35..386c8581 100644 --- a/umap/static/umap/content.css +++ b/umap/static/umap/content.css @@ -194,26 +194,15 @@ input[type="submit"], .button-primary { font-weight: bold; } -.wrapper input[type="submit"]:hover { - background-color: #35537c; -} .wrapper .neutral, .wrapper input[type="submit"].neutral { background-color: var(--button-neutral-background); color: var(--button-neutral-color); } -.wrapper.somber { - background-color: #2E3641; - color: #efefef; - padding-top: 20px; - margin-top: 20px; -} -.wrapper.somber .row { - margin-top: 0; -} .wrapper .button, .wrapper input { height: 56px; line-height: 43px; + font-weight: bold; } /* **************************** */ From 11d3152682fbf78d4201e7ca8a85a925f072ff7d Mon Sep 17 00:00:00 2001 From: David Larlet Date: Fri, 30 Aug 2024 11:12:36 -0400 Subject: [PATCH 12/14] =?UTF-8?q?chore:=20display=20user=E2=80=99s=20teams?= =?UTF-8?q?=20with=20a=20table?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- umap/templates/umap/user_teams.html | 48 ++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/umap/templates/umap/user_teams.html b/umap/templates/umap/user_teams.html index d84296c3..159c2b60 100644 --- a/umap/templates/umap/user_teams.html +++ b/umap/templates/umap/user_teams.html @@ -6,14 +6,46 @@ {% include "umap/dashboard_menu.html" with selected="teams" %}
- - {% trans "New team" %} +
+ + + + + + + + + + {% for team in teams %} + + + + + + {% endfor %} + +
+ {% blocktrans %}Name{% endblocktrans %} + + {% blocktrans %}Users{% endblocktrans %} + + {% blocktrans %}Actions{% endblocktrans %} +
+ {{ team }} + + {% for user in team.users.all %} + {{ user }}{% if not forloop.last %}, {% endif %} + {% endfor %} + + + + {% translate "Edit" %} + +
+
+ {% trans "New team" %}
{% endblock maincontent %} From 2a2a4be357b681a35c9bbe7e26511391985d9fe5 Mon Sep 17 00:00:00 2001 From: David Larlet Date: Fri, 30 Aug 2024 12:34:04 -0400 Subject: [PATCH 13/14] lint: fix imports order --- umap/tests/base.py | 2 +- umap/tests/conftest.py | 2 +- umap/urls.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/umap/tests/base.py b/umap/tests/base.py index ea5107a8..12b672fc 100644 --- a/umap/tests/base.py +++ b/umap/tests/base.py @@ -7,7 +7,7 @@ from django.core.files.base import ContentFile from django.urls import reverse from umap.forms import DEFAULT_CENTER -from umap.models import DataLayer, Licence, Map, TileLayer, Team +from umap.models import DataLayer, Licence, Map, Team, TileLayer User = get_user_model() diff --git a/umap/tests/conftest.py b/umap/tests/conftest.py index 2f12753e..bbf36927 100644 --- a/umap/tests/conftest.py +++ b/umap/tests/conftest.py @@ -9,9 +9,9 @@ from umap.models import Map from .base import ( DataLayerFactory, - TeamFactory, LicenceFactory, MapFactory, + TeamFactory, TileLayerFactory, UserFactory, ) diff --git a/umap/urls.py b/umap/urls.py index f2d3821f..df2de682 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -15,8 +15,8 @@ from . import views from .decorators import ( can_edit_map, can_view_map, - team_members_only, login_required_if_not_anonymous_allowed, + team_members_only, ) from .utils import decorated_patterns From c6ebfd43631ae37dc71a49511f441f2d560e7d3a Mon Sep 17 00:00:00 2001 From: David Larlet Date: Fri, 30 Aug 2024 12:37:48 -0400 Subject: [PATCH 14/14] fix: proper way to test if we have a user --- umap/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/umap/models.py b/umap/models.py index e4c5e719..f3b03d56 100644 --- a/umap/models.py +++ b/umap/models.py @@ -339,7 +339,7 @@ class Map(NamedModel): can = True elif self.share_status in [self.PUBLIC, self.OPEN]: can = True - elif request.user is None: + elif not request.user.is_authenticated: can = False elif request.user == self.owner: can = True