Fix error when trying to change map owner

This bug has been introduced with this change:

8b4842ff21

That was not the correct fix, and this one should be the proper one.

We don't want to edit the permissions reference until we do save, otherwise user
cannot save as it is already no more the owner.

So:
- change permissions.options
- save
- commit those changes to map.options.permissions
- use only those values to check for isOwner and isAnonymousMap
This commit is contained in:
Yohan Boniface 2023-09-20 18:26:04 +02:00
parent 157146dc04
commit c36ea1e4b8
5 changed files with 91 additions and 21 deletions

View file

@ -1180,6 +1180,7 @@ L.U.Map.include({
alert.content = L._('Congratulations, your map has been created!') alert.content = L._('Congratulations, your map has been created!')
this.options.umap_id = data.id this.options.umap_id = data.id
this.permissions.setOptions(data.permissions) this.permissions.setOptions(data.permissions)
this.permissions.commit()
if ( if (
data.permissions && data.permissions &&
data.permissions.anonymous_edit_url && data.permissions.anonymous_edit_url &&
@ -1215,6 +1216,7 @@ L.U.Map.include({
// Do not override local changes to permissions, // Do not override local changes to permissions,
// but update in case some other editors changed them in the meantime. // but update in case some other editors changed them in the meantime.
this.permissions.setOptions(data.permissions) this.permissions.setOptions(data.permissions)
this.permissions.commit()
} }
// Update URL in case the name has changed. // Update URL in case the name has changed.
if (history && history.pushState) if (history && history.pushState)

View file

@ -37,13 +37,13 @@ L.U.MapPermissions = L.Class.extend({
isOwner: function () { isOwner: function () {
return ( return (
this.map.options.user && this.map.options.user &&
this.map.permissions.options.owner && this.map.options.permissions.owner &&
this.map.options.user.id == this.map.permissions.options.owner.id this.map.options.user.id == this.map.options.permissions.owner.id
) )
}, },
isAnonymousMap: function () { isAnonymousMap: function () {
return !this.map.permissions.options.owner return !this.map.options.permissions.owner
}, },
getMap: function () { getMap: function () {

View file

@ -1,4 +1,5 @@
import re import re
from time import sleep
import pytest import pytest
from django.core.signing import get_cookie_signer from django.core.signing import get_cookie_signer
@ -76,7 +77,6 @@ def test_owner_permissions_form(map, datalayer, live_server, owner_session):
edit_permissions.click() edit_permissions.click()
select = owner_session.locator(".umap-field-share_status select") select = owner_session.locator(".umap-field-share_status select")
expect(select).to_be_hidden() expect(select).to_be_hidden()
# expect(select).to_have_value(Map.PUBLIC) # Does not work
owner_field = owner_session.locator(".umap-field-owner") owner_field = owner_session.locator(".umap-field-owner")
expect(owner_field).to_be_hidden() expect(owner_field).to_be_hidden()
editors_field = owner_session.locator(".umap-field-editors input") editors_field = owner_session.locator(".umap-field-editors input")
@ -122,3 +122,28 @@ def test_anonymous_can_add_marker_on_editable_layer(
expect(options).to_have_count(1) # Only Editable layer should be listed expect(options).to_have_count(1) # Only Editable layer should be listed
option = page.locator("select[name='datalayer'] option:checked") option = page.locator("select[name='datalayer'] option:checked")
expect(option).to_have_text(other.name) expect(option).to_have_text(other.name)
def test_can_change_perms_after_create(tilelayer, live_server, page):
page.goto(f"{live_server.url}/en/map/new")
save = page.get_by_title("Save current edits")
expect(save).to_be_visible()
save.click()
sleep(1) # Let save ajax go back
edit_permissions = page.get_by_title("Update permissions and editors")
expect(edit_permissions).to_be_visible()
edit_permissions.click()
select = page.locator(".umap-field-share_status select")
expect(select).to_be_hidden()
owner_field = page.locator(".umap-field-owner")
expect(owner_field).to_be_hidden()
editors_field = page.locator(".umap-field-editors input")
expect(editors_field).to_be_hidden()
datalayer_label = page.get_by_text('Who can edit "Layer 1"')
expect(datalayer_label).to_be_visible()
options = page.locator(".datalayer-permissions select[name='edit_status'] option")
expect(options).to_have_count(3)
option = page.locator(
".datalayer-permissions select[name='edit_status'] option:checked"
)
expect(option).to_have_text("Inherit")

View file

@ -88,9 +88,7 @@ def test_owner_permissions_form(map, datalayer, live_server, login):
expect(editors_field).to_be_visible() expect(editors_field).to_be_visible()
datalayer_label = page.get_by_text('Who can edit "test datalayer"') datalayer_label = page.get_by_text('Who can edit "test datalayer"')
expect(datalayer_label).to_be_visible() expect(datalayer_label).to_be_visible()
options = page.locator( options = page.locator(".datalayer-permissions select[name='edit_status'] option")
".datalayer-permissions select[name='edit_status'] option"
)
expect(options).to_have_count(4) expect(options).to_have_count(4)
option = page.locator( option = page.locator(
".datalayer-permissions select[name='edit_status'] option:checked" ".datalayer-permissions select[name='edit_status'] option:checked"
@ -168,3 +166,49 @@ def test_editor_do_not_have_delete_map_button(map, live_server, login, user):
advanced.click() advanced.click()
delete = page.get_by_role("link", name="Delete") delete = page.get_by_role("link", name="Delete")
expect(delete).to_be_hidden() expect(delete).to_be_hidden()
def test_can_change_perms_after_create(tilelayer, live_server, login, user):
page = login(user)
page.goto(f"{live_server.url}/en/map/new")
save = page.get_by_title("Save current edits")
expect(save).to_be_visible()
save.click()
sleep(1) # Let save ajax go back
edit_permissions = page.get_by_title("Update permissions and editors")
expect(edit_permissions).to_be_visible()
edit_permissions.click()
select = page.locator(".umap-field-share_status select")
expect(select).to_be_visible()
option = page.locator("select[name='share_status'] option:checked")
expect(option).to_have_text("Everyone (public)")
owner_field = page.locator(".umap-field-owner")
expect(owner_field).to_be_visible()
editors_field = page.locator(".umap-field-editors input")
expect(editors_field).to_be_visible()
datalayer_label = page.get_by_text('Who can edit "Layer 1"')
expect(datalayer_label).to_be_visible()
options = page.locator(".datalayer-permissions select[name='edit_status'] option")
expect(options).to_have_count(4)
option = page.locator(
".datalayer-permissions select[name='edit_status'] option:checked"
)
expect(option).to_have_text("Inherit")
def test_can_change_owner(map, live_server, login, user):
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()
close = page.locator(".umap-field-owner .close")
close.click()
input = page.locator("input.edit-owner")
input.type(user.username)
input.press("Tab")
save = page.get_by_title("Save current edits")
expect(save).to_be_visible()
save.click()
sleep(1) # Let save ajax go
modified = Map.objects.get(pk=map.pk)
assert modified.owner == user

View file

@ -447,6 +447,7 @@ class MapDetailMixin:
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs) context = super().get_context_data(**kwargs)
user = self.request.user
properties = { properties = {
"urls": _urls_for_js(), "urls": _urls_for_js(),
"tilelayers": TileLayer.get_list(), "tilelayers": TileLayer.get_list(),
@ -460,19 +461,18 @@ class MapDetailMixin:
], ],
"umap_version": VERSION, "umap_version": VERSION,
} }
if getattr(self, "object", None) and self.object.owner: created = bool(getattr(self, "object", None))
properties["edit_statuses"] = [ if (created and self.object.owner) or (not created and not user.is_anonymous):
(i, str(label)) for i, label in Map.EDIT_STATUS map_statuses = Map.EDIT_STATUS
] datalayer_statuses = DataLayer.EDIT_STATUS
properties["datalayer_edit_statuses"] = [
(i, str(label)) for i, label in DataLayer.EDIT_STATUS
]
else: else:
map_statuses = AnonymousMapPermissionsForm.STATUS
datalayer_statuses = AnonymousDataLayerPermissionsForm.STATUS
properties["edit_statuses"] = [ properties["edit_statuses"] = [
(i, str(label)) for i, label in AnonymousMapPermissionsForm.STATUS (i, str(label)) for i, label in map_statuses
] ]
properties["datalayer_edit_statuses"] = [ properties["datalayer_edit_statuses"] = [
(i, str(label)) for i, label in AnonymousDataLayerPermissionsForm.STATUS (i, str(label)) for i, label in datalayer_statuses
] ]
if self.get_short_url(): if self.get_short_url():
properties["shortUrl"] = self.get_short_url() properties["shortUrl"] = self.get_short_url()
@ -486,7 +486,6 @@ class MapDetailMixin:
locale = to_locale(lang) locale = to_locale(lang)
properties["locale"] = locale properties["locale"] = locale
context["locale"] = locale context["locale"] = locale
user = self.request.user
if not user.is_anonymous: if not user.is_anonymous:
properties["user"] = { properties["user"] = {
"id": user.pk, "id": user.pk,