From 3bdd5bedf199777a96d877d82e4ad087c0d743a5 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Mon, 29 Jul 2024 14:49:07 +0200 Subject: [PATCH 1/2] fix: catch login_required from map page and add a way to login --- umap/settings/base.py | 2 +- umap/static/umap/js/umap.controls.js | 2 +- umap/static/umap/js/umap.js | 12 ++++++++-- umap/templates/registration/login.html | 5 ++++ umap/templates/umap/login_popup_end.html | 9 +++++--- umap/tests/integration/test_basics.py | 29 ++++++++++++++++++++++++ umap/views.py | 23 ++++++++++++------- 7 files changed, 67 insertions(+), 15 deletions(-) diff --git a/umap/settings/base.py b/umap/settings/base.py index 80043c8a..f6a9a2ac 100644 --- a/umap/settings/base.py +++ b/umap/settings/base.py @@ -152,7 +152,7 @@ WSGI_APPLICATION = "umap.wsgi.application" LOGIN_URL = "/login/" LOGOUT_URL = "/logout/" -LOGIN_REDIRECT_URL = "/" +LOGIN_REDIRECT_URL = "login_popup_end" STATIC_URL = "/static/" MEDIA_URL = "/uploads/" diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index 8b5ea4e9..39a027b9 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -640,7 +640,7 @@ const ControlsMixin = { L.DomEvent.on(shareStatusButton, 'click', this.permissions.edit, this.permissions) } this.on('postsync', L.bind(update, this)) - if (this.options.user) { + if (this.options.user?.id) { L.DomUtil.createLink( 'umap-user', rightContainer, diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 8c402fbd..de172678 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -393,7 +393,7 @@ U.Map = L.Map.extend({ this._controls.search = new U.SearchControl() this._controls.embed = new L.Control.Embed(this) this._controls.tilelayersChooser = new U.TileLayerChooser(this) - if (this.options.user) this._controls.star = new U.StarControl(this) + if (this.options.user?.id) this._controls.star = new U.StarControl(this) this._controls.editinosm = new L.Control.EditInOSM({ position: 'topleft', widgetOptions: { @@ -1048,7 +1048,15 @@ U.Map = L.Map.extend({ if (error) { return } - + if (data.login_required) { + window.onLogin = () => this.saveSelf() + window.open(data.login_required) + return + } + if (data.user?.id) { + this.options.user = data.user + this.renderEditToolbar() + } if (!this.options.umap_id) { this.options.umap_id = data.id this.permissions.setOptions(data.permissions) diff --git a/umap/templates/registration/login.html b/umap/templates/registration/login.html index b9bedfec..d408e0cf 100644 --- a/umap/templates/registration/login.html +++ b/umap/templates/registration/login.html @@ -1,5 +1,10 @@ {% extends "base.html" %} +{% load i18n %} + +{% block head_title %} + {% trans "Login" %} +{% endblock head_title %} {% load umap_tags i18n %} {% block extra_head %} diff --git a/umap/templates/umap/login_popup_end.html b/umap/templates/umap/login_popup_end.html index a689f3e5..f9461cb2 100644 --- a/umap/templates/umap/login_popup_end.html +++ b/umap/templates/umap/login_popup_end.html @@ -5,10 +5,13 @@ ')).to_be_visible() # Description should contain escaped HTML expect(page.get_by_text("before after")).to_be_visible() + + +def test_login_from_map_page(live_server, page, tilelayer, settings, user, context): + settings.ENABLE_ACCOUNT_LOGIN = True + assert Map.objects.count() == 0 + page.goto(f"{live_server.url}/en/map/new/") + with ( + page.expect_response(re.compile(r".*/map/create/")), + context.expect_page() as login_page_info, + ): + page.get_by_role("button", name="Save").click() + assert Map.objects.count() == 0 + login_page = login_page_info.value + expect(login_page).to_have_title("Login") + login_page.get_by_placeholder("Username").fill(user.username) + login_page.get_by_placeholder("Password").fill("123123") + with page.expect_response(re.compile(r".*/map/create/")): + login_page.locator('#login_form input[type="submit"]').click() + # Login page should be closed + page.wait_for_timeout(500) # Seems needed from time to time… + assert len(context.pages) == 1 + # Save should have proceed + assert Map.objects.count() == 1 + # Use name should now appear on the header toolbar + expect(page.get_by_text("My Dashboard (Joe)")).to_be_visible() diff --git a/umap/views.py b/umap/views.py index bc3e6655..ce58624b 100644 --- a/umap/views.py +++ b/umap/views.py @@ -456,6 +456,17 @@ def simple_json_response(**kwargs): # ############## # +class SessionMixin: + def get_user_data(self): + if self.request.user.is_anonymous: + return {} + return { + "id": self.request.user.pk, + "name": str(self.request.user), + "url": reverse("user_dashboard"), + } + + class FormLessEditMixin: http_method_names = [ "post", @@ -470,7 +481,7 @@ class FormLessEditMixin: return self.get_form_class()(**kwargs) -class MapDetailMixin: +class MapDetailMixin(SessionMixin): model = Map pk_url_kwarg = "map_id" @@ -522,12 +533,7 @@ class MapDetailMixin: if self.get_short_url(): properties["shortUrl"] = self.get_short_url() - if not user.is_anonymous: - properties["user"] = { - "id": user.pk, - "name": str(user), - "url": reverse("user_dashboard"), - } + properties["user"] = self.get_user_data() return properties def get_context_data(self, **kwargs): @@ -755,7 +761,7 @@ class MapPreview(MapDetailMixin, TemplateView): return properties -class MapCreate(FormLessEditMixin, PermissionsMixin, CreateView): +class MapCreate(FormLessEditMixin, PermissionsMixin, SessionMixin, CreateView): model = Map form_class = MapSettingsForm @@ -772,6 +778,7 @@ class MapCreate(FormLessEditMixin, PermissionsMixin, CreateView): id=self.object.pk, url=self.object.get_absolute_url(), permissions=permissions, + user=self.get_user_data(), ) if not self.request.user.is_authenticated: key, value = self.object.signed_cookie_elements From 5145404dc40404ac8ee24e19fe23b02766a1d948 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 31 Jul 2024 22:29:29 +0200 Subject: [PATCH 2/2] fix: show delete button for owner and anonymous owner --- umap/static/umap/js/modules/permissions.js | 2 +- umap/static/umap/js/umap.js | 12 ++++++---- .../integration/test_anonymous_owned_map.py | 23 +++++++++++++++++++ umap/tests/integration/test_owned_map.py | 12 ++-------- umap/views.py | 6 ++++- 5 files changed, 38 insertions(+), 17 deletions(-) diff --git a/umap/static/umap/js/modules/permissions.js b/umap/static/umap/js/modules/permissions.js index a3bcec6d..52ae216c 100644 --- a/umap/static/umap/js/modules/permissions.js +++ b/umap/static/umap/js/modules/permissions.js @@ -34,7 +34,7 @@ export class MapPermissions { } isOwner() { - return this.map.options.user?.id === this.map.options.permissions.owner?.id + return Boolean(this.map.options.user?.is_owner) } isAnonymousMap() { diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index de172678..1bc981b5 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -1638,11 +1638,13 @@ U.Map = L.Map.extend({ }, del: async function () { - if (confirm(L._('Are you sure you want to delete this map?'))) { - const url = this.urls.get('map_delete', { map_id: this.options.umap_id }) - const [data, response, error] = await this.server.post(url) - if (data.redirect) window.location = data.redirect - } + this.dialog + .confirm(L._('Are you sure you want to delete this map?')) + .then(async () => { + const url = this.urls.get('map_delete', { map_id: this.options.umap_id }) + const [data, response, error] = await this.server.post(url) + if (data.redirect) window.location = data.redirect + }) }, clone: async function () { diff --git a/umap/tests/integration/test_anonymous_owned_map.py b/umap/tests/integration/test_anonymous_owned_map.py index aea3fec3..c406e189 100644 --- a/umap/tests/integration/test_anonymous_owned_map.py +++ b/umap/tests/integration/test_anonymous_owned_map.py @@ -219,3 +219,26 @@ def test_alert_message_after_create_show_link_even_without_mail( ).to_be_visible() expect(alert.get_by_role("button", name="Copy")).to_be_visible() expect(alert.get_by_role("button", name="Send me the link")).to_be_hidden() + + +def test_anonymous_owner_can_delete_the_map(anonymap, live_server, owner_session): + assert Map.objects.count() == 1 + owner_session.goto(f"{live_server.url}{anonymap.get_absolute_url()}") + owner_session.get_by_role("button", name="Edit").click() + owner_session.get_by_role("link", name="Map advanced properties").click() + owner_session.get_by_text("Advanced actions").click() + expect(owner_session.get_by_role("button", name="Delete")).to_be_visible() + owner_session.get_by_role("button", name="Delete").click() + with owner_session.expect_response(re.compile(r".*/update/delete/.*")): + owner_session.get_by_role("button", name="OK").click() + assert not Map.objects.count() + + +def test_non_owner_cannot_see_delete_button(anonymap, live_server, page): + anonymap.edit_status = Map.ANONYMOUS + anonymap.save() + page.goto(f"{live_server.url}{anonymap.get_absolute_url()}") + page.get_by_role("button", name="Edit").click() + page.get_by_role("link", name="Map advanced properties").click() + page.get_by_text("Advanced actions").click() + expect(page.get_by_role("button", name="Delete")).to_be_hidden() diff --git a/umap/tests/integration/test_owned_map.py b/umap/tests/integration/test_owned_map.py index 430026fe..bf189641 100644 --- a/umap/tests/integration/test_owned_map.py +++ b/umap/tests/integration/test_owned_map.py @@ -134,17 +134,9 @@ def test_owner_has_delete_map_button(map, live_server, login): advanced.click() delete = page.get_by_role("button", name="Delete", exact=True) expect(delete).to_be_visible() - dialog_shown = False - - def handle_dialog(dialog): - dialog.accept() - nonlocal dialog_shown - dialog_shown = True - - page.on("dialog", handle_dialog) + delete.click() with page.expect_navigation(): - delete.click() - assert dialog_shown + page.get_by_role("button", name="OK").click() assert Map.objects.all().count() == 0 diff --git a/umap/views.py b/umap/views.py index ce58624b..5f976977 100644 --- a/umap/views.py +++ b/umap/views.py @@ -458,12 +458,16 @@ def simple_json_response(**kwargs): class SessionMixin: def get_user_data(self): + data = {} + if hasattr(self, "object"): + data["is_owner"] = self.object.is_owner(self.request.user, self.request) if self.request.user.is_anonymous: - return {} + return data return { "id": self.request.user.pk, "name": str(self.request.user), "url": reverse("user_dashboard"), + **data, }