fix: make sure anonymous is owner at create (#2189)

The tricky thing is that the Map.is_owner() method check for cookies on
the request, but at create this cookie is not set yet on the request, so
we have to deal with an exception here.

fix #2176
This commit is contained in:
Yohan Boniface 2024-10-04 17:55:29 +02:00 committed by GitHub
commit 8d59220d05
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 94 additions and 96 deletions

View file

@ -313,7 +313,7 @@ export const SingleMixin = (Base) =>
DomEvent.on(close, 'click', () => { DomEvent.on(close, 'click', () => {
this.selectedContainer.innerHTML = '' this.selectedContainer.innerHTML = ''
this.input.style.display = 'block' this.input.style.display = 'block'
this.options.on_unselect(result) this.options.on_unselect?.(result)
}) })
this.hide() this.hide()
} }
@ -342,7 +342,7 @@ export const MultipleMixin = (Base) =>
}) })
DomEvent.on(close, 'click', () => { DomEvent.on(close, 'click', () => {
this.selectedContainer.removeChild(result_el) this.selectedContainer.removeChild(result_el)
this.options.on_unselect(result) this.options.on_unselect?.(result)
}) })
this.hide() this.hide()
} }

View file

@ -19,7 +19,12 @@ export default class Caption {
open() { open() {
const container = DomUtil.create('div', 'umap-caption') const container = DomUtil.create('div', 'umap-caption')
const hgroup = DomUtil.element({ tagName: 'hgroup', parent: container }) const hgroup = DomUtil.element({ tagName: 'hgroup', parent: container })
DomUtil.createTitle(hgroup, this.map.options.name, 'icon-caption icon-block') DomUtil.createTitle(
hgroup,
this.map.getDisplayName(),
'icon-caption icon-block',
'map-name'
)
this.map.addAuthorLink('h4', hgroup) this.map.addAuthorLink('h4', hgroup)
if (this.map.options.description) { if (this.map.options.description) {
const description = DomUtil.element({ const description = DomUtil.element({

View file

@ -92,12 +92,12 @@ export class DataLayer {
set isDirty(status) { set isDirty(status) {
this._isDirty = status this._isDirty = status
if (status) { if (status) {
this.map.addDirtyDatalayer(this) this.map.isDirty = true
// A layer can be made dirty by indirect action (like dragging layers) // A layer can be made dirty by indirect action (like dragging layers)
// we need to have it loaded before saving it. // we need to have it loaded before saving it.
if (!this.isLoaded()) this.fetchData() if (!this.isLoaded()) this.fetchData()
} else { } else {
this.map.removeDirtyDatalayer(this) this.map.checkDirty()
this.isDeleted = false this.isDeleted = false
} }
} }
@ -339,12 +339,12 @@ export class DataLayer {
const id = stamp(this) const id = stamp(this)
if (!this.map.datalayers[id]) { if (!this.map.datalayers[id]) {
this.map.datalayers[id] = this this.map.datalayers[id] = this
}
if (!this.map.datalayers_index.includes(this)) { if (!this.map.datalayers_index.includes(this)) {
this.map.datalayers_index.push(this) this.map.datalayers_index.push(this)
} }
this.map.onDataLayersChanged() this.map.onDataLayersChanged()
} }
}
_dataUrl() { _dataUrl() {
const template = this.map.options.urls.datalayer_view const template = this.map.options.urls.datalayer_view
@ -559,7 +559,6 @@ export class DataLayer {
erase() { erase() {
this.hide() this.hide()
delete this.map.datalayers[stamp(this)]
this.map.datalayers_index.splice(this.getRank(), 1) this.map.datalayers_index.splice(this.getRank(), 1)
this.parentPane.removeChild(this.pane) this.parentPane.removeChild(this.pane)
this.map.onDataLayersChanged() this.map.onDataLayersChanged()
@ -1029,7 +1028,7 @@ export class DataLayer {
} }
async save() { async save() {
if (this.isDeleted) return this.saveDelete() if (this.isDeleted) return await this.saveDelete()
if (!this.isLoaded()) { if (!this.isLoaded()) {
return return
} }
@ -1093,8 +1092,8 @@ export class DataLayer {
if (this.umap_id) { if (this.umap_id) {
await this.map.server.post(this.getDeleteUrl()) await this.map.server.post(this.getDeleteUrl())
} }
delete this.map.datalayers[stamp(this)]
this.isDirty = false this.isDirty = false
this.map.continueSaving()
} }
getMap() { getMap() {

View file

@ -42,10 +42,6 @@ export class MapPermissions {
return !this.map.options.permissions.owner return !this.map.options.permissions.owner
} }
getMap() {
return this.map
}
_editAnonymous(container) { _editAnonymous(container) {
const fields = [] const fields = []
if (this.isOwner()) { if (this.isOwner()) {
@ -182,7 +178,7 @@ export class MapPermissions {
} }
async save() { async save() {
if (!this.isDirty) return this.map.continueSaving() if (!this.isDirty) return
const formData = new FormData() const formData = new FormData()
if (!this.isAnonymousMap() && this.options.editors) { if (!this.isAnonymousMap() && this.options.editors) {
const editors = this.options.editors.map((u) => u.id) const editors = this.options.editors.map((u) => u.id)
@ -205,7 +201,6 @@ export class MapPermissions {
if (!error) { if (!error) {
this.commit() this.commit()
this.isDirty = false this.isDirty = false
this.map.continueSaving()
this.map.fire('postsync') this.map.fire('postsync')
} }
} }
@ -230,10 +225,12 @@ export class MapPermissions {
} }
getShareStatusDisplay() { getShareStatusDisplay() {
if (this.map.options.share_statuses) {
return Object.fromEntries(this.map.options.share_statuses)[ return Object.fromEntries(this.map.options.share_statuses)[
this.options.share_status this.options.share_status
] ]
} }
}
} }
export class DataLayerPermissions { export class DataLayerPermissions {
@ -258,7 +255,7 @@ export class DataLayerPermissions {
return this._isDirty return this._isDirty
} }
getMap() { get map() {
return this.datalayer.map return this.datalayer.map
} }
@ -271,7 +268,7 @@ export class DataLayerPermissions {
label: translate('Who can edit "{layer}"', { label: translate('Who can edit "{layer}"', {
layer: this.datalayer.getName(), layer: this.datalayer.getName(),
}), }),
selectOptions: this.datalayer.map.options.datalayer_edit_statuses, selectOptions: this.map.options.datalayer_edit_statuses,
}, },
], ],
] ]
@ -283,16 +280,17 @@ export class DataLayerPermissions {
} }
getUrl() { getUrl() {
return Utils.template(this.datalayer.map.options.urls.datalayer_permissions, { return this.map.urls.get('datalayer_permissions', {
map_id: this.datalayer.map.options.umap_id, map_id: this.map.options.umap_id,
pk: this.datalayer.umap_id, pk: this.datalayer.umap_id,
}) })
} }
async save() { async save() {
if (!this.isDirty) return this.datalayer.map.continueSaving() if (!this.isDirty) return
const formData = new FormData() const formData = new FormData()
formData.append('edit_status', this.options.edit_status) formData.append('edit_status', this.options.edit_status)
const [data, response, error] = await this.datalayer.map.server.post( const [data, response, error] = await this.map.server.post(
this.getUrl(), this.getUrl(),
{}, {},
formData formData
@ -300,7 +298,6 @@ export class DataLayerPermissions {
if (!error) { if (!error) {
this.commit() this.commit()
this.isDirty = false this.isDirty = false
this.datalayer.map.continueSaving()
} }
} }

View file

@ -578,11 +578,15 @@ const ControlsMixin = {
], ],
renderEditToolbar: function () { renderEditToolbar: function () {
const container = L.DomUtil.create( const className = 'umap-main-edit-toolbox'
const container =
document.querySelector(`.${className}`) ||
L.DomUtil.create(
'div', 'div',
'umap-main-edit-toolbox with-transition dark', `${className} with-transition dark`,
this._controlContainer this._controlContainer
) )
container.innerHTML = ''
const leftContainer = L.DomUtil.create('div', 'umap-left-edit-toolbox', container) const leftContainer = L.DomUtil.create('div', 'umap-left-edit-toolbox', container)
const rightContainer = L.DomUtil.create('div', 'umap-right-edit-toolbox', container) const rightContainer = L.DomUtil.create('div', 'umap-right-edit-toolbox', container)
const logo = L.DomUtil.create('div', 'logo', leftContainer) const logo = L.DomUtil.create('div', 'logo', leftContainer)
@ -623,23 +627,10 @@ const ControlsMixin = {
}, },
this this
) )
const update = () => {
const status = this.permissions.getShareStatusDisplay()
nameButton.textContent = this.getDisplayName()
// status is not set until map is saved once
if (status) {
shareStatusButton.textContent = L._('Visibility: {status}', {
status: status,
})
}
}
update()
this.once('saved', L.bind(update, this))
if (this.options.editMode === 'advanced') { if (this.options.editMode === 'advanced') {
L.DomEvent.on(nameButton, 'click', this.editCaption, this) L.DomEvent.on(nameButton, 'click', this.editCaption, this)
L.DomEvent.on(shareStatusButton, 'click', this.permissions.edit, this.permissions) L.DomEvent.on(shareStatusButton, 'click', this.permissions.edit, this.permissions)
} }
this.on('postsync', L.bind(update, this))
if (this.options.user?.id) { if (this.options.user?.id) {
L.DomUtil.createLink( L.DomUtil.createLink(
'umap-user', 'umap-user',

View file

@ -141,10 +141,10 @@ L.DomUtil.createButtonIcon = (parent, className, title, callback, size = 16) =>
return el return el
} }
L.DomUtil.createTitle = (parent, text, className, tag = 'h3') => { L.DomUtil.createTitle = (parent, text, iconClassName, className = '', tag = 'h3') => {
const title = L.DomUtil.create(tag, '', parent) const title = L.DomUtil.create(tag, '', parent)
if (className) L.DomUtil.createIcon(title, className) if (className) L.DomUtil.createIcon(title, iconClassName)
L.DomUtil.add('span', '', title, text) L.DomUtil.add('span', className, title, text)
return title return title
} }

View file

@ -110,10 +110,9 @@ U.Map = L.Map.extend({
delete this.options.advancedFilterKey delete this.options.advancedFilterKey
} }
// Global storage for retrieving datalayers and features // Global storage for retrieving datalayers and features.
this.datalayers = {} this.datalayers = {} // All datalayers, including deleted.
this.datalayers_index = [] this.datalayers_index = [] // Datalayers actually on the map and ordered.
this.dirty_datalayers = []
this.features_index = {} this.features_index = {}
// Needed for actions labels // Needed for actions labels
@ -200,6 +199,7 @@ U.Map = L.Map.extend({
this.backup() this.backup()
this.on('click', this.closeInplaceToolbar) this.on('click', this.closeInplaceToolbar)
this.on('contextmenu', this.onContextMenu) this.on('contextmenu', this.onContextMenu)
this.propagate()
}, },
initSyncEngine: async function () { initSyncEngine: async function () {
@ -231,6 +231,7 @@ U.Map = L.Map.extend({
this.renderEditToolbar() this.renderEditToolbar()
this.renderControls() this.renderControls()
this.browser.redraw() this.browser.redraw()
this.propagate()
break break
case 'data': case 'data':
this.redrawVisibleDataLayers() this.redrawVisibleDataLayers()
@ -487,7 +488,7 @@ U.Map = L.Map.extend({
loadDataLayers: async function () { loadDataLayers: async function () {
this.datalayersLoaded = true this.datalayersLoaded = true
this.fire('datalayersloaded') this.fire('datalayersloaded')
for (const datalayer of Object.values(this.datalayers)) { for (const datalayer of this.datalayers_index) {
if (datalayer.showAtLoad()) await datalayer.show() if (datalayer.showAtLoad()) await datalayer.show()
} }
this.dataloaded = true this.dataloaded = true
@ -933,6 +934,7 @@ U.Map = L.Map.extend({
if (mustReindex) datalayer.reindex() if (mustReindex) datalayer.reindex()
datalayer.redraw() datalayer.redraw()
}) })
this.propagate()
this.fire('postsync') this.fire('postsync')
this.isDirty = true this.isDirty = true
}, },
@ -996,12 +998,12 @@ U.Map = L.Map.extend({
if (this.editTools) this.editTools.stopDrawing() if (this.editTools) this.editTools.stopDrawing()
this.resetOptions() this.resetOptions()
this.datalayers_index = [].concat(this._datalayers_index_bk) this.datalayers_index = [].concat(this._datalayers_index_bk)
this.dirty_datalayers.slice().forEach((datalayer) => { // Iter over all datalayers, including deleted if any.
for (const datalayer of Object.values(this.datalayers)) {
if (datalayer.isDeleted) datalayer.connectToMap() if (datalayer.isDeleted) datalayer.connectToMap()
datalayer.reset() if (datalayer.isDirty) datalayer.reset()
}) }
this.ensurePanesOrder() this.ensurePanesOrder()
this.dirty_datalayers = []
this.initTileLayers() this.initTileLayers()
this.isDirty = false this.isDirty = false
this.onDataLayersChanged() this.onDataLayersChanged()
@ -1011,25 +1013,6 @@ U.Map = L.Map.extend({
this._container.classList.toggle('umap-is-dirty', this.isDirty) this._container.classList.toggle('umap-is-dirty', this.isDirty)
}, },
addDirtyDatalayer: function (datalayer) {
if (this.dirty_datalayers.indexOf(datalayer) === -1) {
this.dirty_datalayers.push(datalayer)
this.isDirty = true
}
},
removeDirtyDatalayer: function (datalayer) {
if (this.dirty_datalayers.indexOf(datalayer) !== -1) {
this.dirty_datalayers.splice(this.dirty_datalayers.indexOf(datalayer), 1)
this.checkDirty()
}
},
continueSaving: function () {
if (this.dirty_datalayers.length) this.dirty_datalayers[0].save()
else this.fire('saved')
},
exportOptions: function () { exportOptions: function () {
const properties = {} const properties = {}
for (const option of Object.keys(U.SCHEMA)) { for (const option of Object.keys(U.SCHEMA)) {
@ -1059,19 +1042,17 @@ U.Map = L.Map.extend({
return return
} }
if (data.login_required) { if (data.login_required) {
window.onLogin = () => this.saveSelf() window.onLogin = () => this.save()
window.open(data.login_required) window.open(data.login_required)
return return
} }
if (data.user?.id) {
this.options.user = data.user this.options.user = data.user
this.renderEditToolbar() this.renderEditToolbar()
}
if (!this.options.umap_id) { if (!this.options.umap_id) {
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() this.permissions.commit()
if (data?.permissions?.anonymous_edit_url) { if (data.permissions?.anonymous_edit_url) {
this.once('saved', () => { this.once('saved', () => {
U.AlertCreation.info( U.AlertCreation.info(
L._('Your map has been created with an anonymous account!'), L._('Your map has been created with an anonymous account!'),
@ -1104,21 +1085,42 @@ U.Map = L.Map.extend({
} else { } else {
window.location = data.url window.location = data.url
} }
this.permissions.save() this.propagate()
return true
}, },
save: function () { save: async function () {
if (!this.isDirty) return if (!this.isDirty) return
if (this._default_extent) this._setCenterAndZoom() if (this._default_extent) this._setCenterAndZoom()
this.backup() this.backup()
this.once('saved', () => {
this.isDirty = false
})
if (this.options.editMode === 'advanced') { if (this.options.editMode === 'advanced') {
// Only save the map if the user has the rights to do so. // Only save the map if the user has the rights to do so.
this.saveSelf() const ok = await this.saveSelf()
} else { if (!ok) return
this.permissions.save() }
await this.permissions.save()
// Iter over all datalayers, including deleted.
for (const datalayer of Object.values(this.datalayers)) {
if (datalayer.isDirty) await datalayer.save()
}
this.isDirty = false
this.renderEditToolbar()
this.fire('saved')
},
propagate: function () {
let els = document.querySelectorAll('.map-name')
for (const el of els) {
el.textContent = this.getDisplayName()
}
const status = this.permissions.getShareStatusDisplay()
els = document.querySelectorAll('.share-status')
for (const el of els) {
if (status) {
el.textContent = L._('Visibility: {status}', {
status: status,
})
}
} }
}, },
@ -1831,7 +1833,7 @@ U.Map = L.Map.extend({
getFeatureById: function (id) { getFeatureById: function (id) {
let feature let feature
for (const datalayer of Object.values(this.datalayers)) { for (const datalayer of this.datalayers_index) {
feature = datalayer.getFeatureById(id) feature = datalayer.getFeatureById(id)
if (feature) return feature if (feature) return feature
} }

View file

@ -156,6 +156,7 @@ def test_can_change_perms_after_create(tilelayer, live_server, page):
".datalayer-permissions select[name='edit_status'] option:checked" ".datalayer-permissions select[name='edit_status'] option:checked"
) )
expect(option).to_have_text("Inherit") expect(option).to_have_text("Inherit")
expect(page.get_by_label("Secret edit link:")).to_be_visible()
def test_alert_message_after_create( def test_alert_message_after_create(

View file

@ -63,7 +63,6 @@ def test_cancel_deleting_datalayer_should_restore(
page.locator(".panel.right").get_by_title("Delete layer").click() page.locator(".panel.right").get_by_title("Delete layer").click()
page.get_by_role("button", name="OK").click() page.get_by_role("button", name="OK").click()
expect(markers).to_have_count(0) expect(markers).to_have_count(0)
page.get_by_role("button", name="Open browser").click()
expect(page.get_by_text("test datalayer")).to_be_hidden() expect(page.get_by_text("test datalayer")).to_be_hidden()
page.get_by_role("button", name="Cancel edits").click() page.get_by_role("button", name="Cancel edits").click()
page.locator("dialog").get_by_role("button", name="OK").click() page.locator("dialog").get_by_role("button", name="OK").click()

View file

@ -40,7 +40,7 @@ def test_map_name_impacts_ui(live_server, page, tilelayer):
name_input.fill("something else") name_input.fill("something else")
expect(page.get_by_role("button", name="something else").nth(1)).to_be_visible() expect(page.get_by_role("button", name="something else").first).to_be_visible()
def test_zoomcontrol_impacts_ui(live_server, page, tilelayer): def test_zoomcontrol_impacts_ui(live_server, page, tilelayer):

View file

@ -368,6 +368,7 @@ def test_anonymous_create(cookieclient, post_data):
assert ( assert (
created_map.get_anonymous_edit_url() in j["permissions"]["anonymous_edit_url"] created_map.get_anonymous_edit_url() in j["permissions"]["anonymous_edit_url"]
) )
assert j["user"]["is_owner"] is True
assert created_map.name == name assert created_map.name == name
key, value = created_map.signed_cookie_elements key, value = created_map.signed_cookie_elements
assert key in cookieclient.cookies assert key in cookieclient.cookies

View file

@ -863,15 +863,17 @@ class MapCreate(FormLessEditMixin, PermissionsMixin, SessionMixin, CreateView):
form.instance.owner = self.request.user form.instance.owner = self.request.user
self.object = form.save() self.object = form.save()
permissions = self.get_permissions() permissions = self.get_permissions()
user_data = self.get_user_data()
# User does not have the cookie yet. # User does not have the cookie yet.
if not self.object.owner: if not self.object.owner:
anonymous_url = self.object.get_anonymous_edit_url() anonymous_url = self.object.get_anonymous_edit_url()
permissions["anonymous_edit_url"] = anonymous_url permissions["anonymous_edit_url"] = anonymous_url
user_data["is_owner"] = True
response = simple_json_response( response = simple_json_response(
id=self.object.pk, id=self.object.pk,
url=self.object.get_absolute_url(), url=self.object.get_absolute_url(),
permissions=permissions, permissions=permissions,
user=self.get_user_data(), user=user_data,
) )
if not self.request.user.is_authenticated: if not self.request.user.is_authenticated:
key, value = self.object.signed_cookie_elements key, value = self.object.signed_cookie_elements
@ -908,7 +910,7 @@ def get_websocket_auth_token(request, map_id, map_inst):
return simple_json_response(token=signed_token) return simple_json_response(token=signed_token)
class MapUpdate(FormLessEditMixin, PermissionsMixin, UpdateView): class MapUpdate(FormLessEditMixin, PermissionsMixin, SessionMixin, UpdateView):
model = Map model = Map
form_class = MapSettingsForm form_class = MapSettingsForm
pk_url_kwarg = "map_id" pk_url_kwarg = "map_id"
@ -920,6 +922,7 @@ class MapUpdate(FormLessEditMixin, PermissionsMixin, UpdateView):
id=self.object.pk, id=self.object.pk,
url=self.object.get_absolute_url(), url=self.object.get_absolute_url(),
permissions=self.get_permissions(), permissions=self.get_permissions(),
user=self.get_user_data(),
) )