From b947134fa2a5daa15b69c295bc06db6838851c71 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 24 Jan 2024 13:27:33 +0100 Subject: [PATCH] chore: ServerRequest returns an explicit error if any --- umap/static/umap/js/modules/request.js | 32 ++++-- .../umap/js/umap.datalayer.permissions.js | 14 ++- umap/static/umap/js/umap.forms.js | 8 +- umap/static/umap/js/umap.js | 107 +++++++++--------- umap/static/umap/js/umap.layer.js | 104 ++++++++++------- umap/static/umap/js/umap.permissions.js | 32 ++++-- 6 files changed, 172 insertions(+), 125 deletions(-) diff --git a/umap/static/umap/js/modules/request.js b/umap/static/umap/js/modules/request.js index a3aaf158..54347394 100644 --- a/umap/static/umap/js/modules/request.js +++ b/umap/static/umap/js/modules/request.js @@ -17,15 +17,15 @@ const BaseRequest = Evented.extend({ } catch (error) { this._onError(error) this.fire('dataload', { id: id }) - return + return null } if (!response.ok) { - this.onNok(response.status, await response.text()) + this.fire('dataload', { id: id }) + return this.onNok(response.status, response) } // TODO // - error handling // - UI connection / events - // - preflight mode in CORS ? this.fire('dataload', { id: id }) return response @@ -44,7 +44,9 @@ const BaseRequest = Evented.extend({ this.onError(error) }, onError: function (error) {}, - onNok: function (status) {}, + onNok: function (status, reponse) { + return response + }, }) export const Request = BaseRequest.extend({ @@ -55,8 +57,9 @@ export const Request = BaseRequest.extend({ console.error(error) this.ui.alert({ content: L._('Problem in the response'), level: 'error' }) }, - onNok: function (status, message) { + onNok: function (status, response) { this.onError(message) + return response }, }) @@ -81,26 +84,30 @@ export const ServerRequest = Request.extend({ headers['X-CSRFToken'] = token } const response = await Request.prototype.post.call(this, uri, headers, data) - return await this._handle_json_response(response) + return await this._as_json(response) }, get: async function (uri, headers) { const response = await Request.prototype.get.call(this, uri, headers) - return await this._handle_json_response(response) + return await this._as_json(response) }, - _handle_json_response: async function (response) { + _as_json: async function (response) { + if (!response) return [{}, null, new Error("Undefined error")] try { const data = await response.json() - this._handle_server_instructions(data) - return [data, response] + if (this._handle_server_instructions(data) !== false) { + return [{}, null] + } + return [data, response, null] } catch (error) { this._onError(error) + return [{}, null, error] } }, _handle_server_instructions: function (data) { - // In some case, the response contains instructions + // Generic cases, let's deal with them once if (data.redirect) { const newPath = data.redirect if (window.location.pathname == newPath) { @@ -122,6 +129,9 @@ export const ServerRequest = Request.extend({ this.fire('login') win.close() } + } else { + // Nothing to do, we can let the response proceed + return false } }, diff --git a/umap/static/umap/js/umap.datalayer.permissions.js b/umap/static/umap/js/umap.datalayer.permissions.js index f4a79e92..8ac2a2d7 100644 --- a/umap/static/umap/js/umap.datalayer.permissions.js +++ b/umap/static/umap/js/umap.datalayer.permissions.js @@ -55,10 +55,16 @@ L.U.DataLayerPermissions = L.Class.extend({ if (!this.isDirty) return this.datalayer.map.continueSaving() const formData = new FormData() formData.append('edit_status', this.options.edit_status) - await this.datalayer.map.server.post(this.getUrl(), {}, formData) - this.commit() - this.isDirty = false - this.datalayer.map.continueSaving() + const [data, response, error] = await this.datalayer.map.server.post( + this.getUrl(), + {}, + formData + ) + if (!error) { + this.commit() + this.isDirty = false + this.datalayer.map.continueSaving() + } }, commit: function () { diff --git a/umap/static/umap/js/umap.forms.js b/umap/static/umap/js/umap.forms.js index 3f36b184..f2776ae8 100644 --- a/umap/static/umap/js/umap.forms.js +++ b/umap/static/umap/js/umap.forms.js @@ -695,11 +695,13 @@ L.FormBuilder.IconUrl = L.FormBuilder.BlurInput.extend({ if (this.pictogram_list) { this.buildSymbolsList() } else { - const [{ pictogram_list }, response] = await this.builder.map.server.get( + const [{ pictogram_list }, response, error] = await this.builder.map.server.get( this.builder.map.options.urls.pictogram_list_json ) - this.pictogram_list = pictogram_list - this.buildSymbolsList() + if (!error) { + this.pictogram_list = pictogram_list + this.buildSymbolsList() + } } }, diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index f142f249..f5ce6c88 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -34,7 +34,7 @@ L.Map.mergeOptions({ // we cannot rely on this because of the y is overriden by Leaflet // See https://github.com/Leaflet/Leaflet/pull/9201 // And let's remove this -y when this PR is merged and released. - demoTileInfos: { s: 'a', z: 9, x: 265, y: 181, '-y': 181, r: '' }, + demoTileInfos: { 's': 'a', 'z': 9, 'x': 265, 'y': 181, '-y': 181, 'r': '' }, licences: [], licence: '', enableMarkerDraw: true, @@ -838,7 +838,10 @@ L.U.Map.include({ self.isDirty = true } if (this._controls.tilelayersChooser) - this._controls.tilelayersChooser.openSwitcher({ callback: callback, className: 'dark' }) + this._controls.tilelayersChooser.openSwitcher({ + callback: callback, + className: 'dark', + }) }, manageDatalayers: function () { @@ -1097,59 +1100,61 @@ L.U.Map.include({ formData.append('center', JSON.stringify(this.geometry())) formData.append('settings', JSON.stringify(geojson)) const uri = this.urls.get('map_save', { map_id: this.options.umap_id }) - const [data, response] = await this.server.post(uri, {}, formData) - let duration = 3000, - alert = { content: L._('Map has been saved!'), level: 'info' } - if (!this.options.umap_id) { - alert.content = L._('Congratulations, your map has been created!') - this.options.umap_id = data.id - this.permissions.setOptions(data.permissions) - this.permissions.commit() - if ( - data.permissions && - data.permissions.anonymous_edit_url && - this.options.urls.map_send_edit_link - ) { - alert.duration = Infinity - alert.content = - L._( - 'Your map has been created! As you are not logged in, here is your secret link to edit the map, please keep it safe:' - ) + `
${data.permissions.anonymous_edit_url}` + const [data, response, error] = await this.server.post(uri, {}, formData) + if (!error) { + let duration = 3000, + alert = { content: L._('Map has been saved!'), level: 'info' } + if (!this.options.umap_id) { + alert.content = L._('Congratulations, your map has been created!') + this.options.umap_id = data.id + this.permissions.setOptions(data.permissions) + this.permissions.commit() + if ( + data.permissions && + data.permissions.anonymous_edit_url && + this.options.urls.map_send_edit_link + ) { + alert.duration = Infinity + alert.content = + L._( + 'Your map has been created! As you are not logged in, here is your secret link to edit the map, please keep it safe:' + ) + `
${data.permissions.anonymous_edit_url}` - alert.actions = [ - { - label: L._('Send me the link'), - input: L._('Email'), - callback: this.sendEditLink, - callbackContext: this, - }, - { - label: L._('Copy link'), - callback: () => { - L.Util.copyToClipboard(data.permissions.anonymous_edit_url) - this.ui.alert({ - content: L._('Secret edit link copied to clipboard!'), - level: 'info', - }) + alert.actions = [ + { + label: L._('Send me the link'), + input: L._('Email'), + callback: this.sendEditLink, + callbackContext: this, }, - callbackContext: this, - }, - ] + { + label: L._('Copy link'), + callback: () => { + L.Util.copyToClipboard(data.permissions.anonymous_edit_url) + this.ui.alert({ + content: L._('Secret edit link copied to clipboard!'), + level: 'info', + }) + }, + callbackContext: this, + }, + ] + } + } else if (!this.permissions.isDirty) { + // Do not override local changes to permissions, + // but update in case some other editors changed them in the meantime. + this.permissions.setOptions(data.permissions) + this.permissions.commit() } - } else if (!this.permissions.isDirty) { - // Do not override local changes to permissions, - // but update in case some other editors changed them in the meantime. - this.permissions.setOptions(data.permissions) - this.permissions.commit() + // Update URL in case the name has changed. + if (history && history.pushState) + history.pushState({}, this.options.name, data.url) + else window.location = data.url + alert.content = data.info || alert.content + this.once('saved', () => this.ui.alert(alert)) + this.ui.closePanel() + this.permissions.save() } - // Update URL in case the name has changed. - if (history && history.pushState) - history.pushState({}, this.options.name, data.url) - else window.location = data.url - alert.content = data.info || alert.content - this.once('saved', () => this.ui.alert(alert)) - this.ui.closePanel() - this.permissions.save() }, save: function () { diff --git a/umap/static/umap/js/umap.layer.js b/umap/static/umap/js/umap.layer.js index bdae1f3a..cbb17f20 100644 --- a/umap/static/umap/js/umap.layer.js +++ b/umap/static/umap/js/umap.layer.js @@ -674,22 +674,24 @@ L.U.DataLayer = L.Evented.extend({ if (!this.umap_id) return if (this._loading) return this._loading = true - const [geojson, response] = await this.map.server.get(this._dataUrl()) - this._last_modified = response.headers['Last-Modified'] - // FIXME: for now this property is set dynamically from backend - // And thus it's not in the geojson file in the server - // So do not let all options to be reset - // Fix is a proper migration so all datalayers settings are - // in DB, and we remove it from geojson flat files. - if (geojson._umap_options) { - geojson._umap_options.editMode = this.options.editMode + const [geojson, response, error] = await this.map.server.get(this._dataUrl()) + if (!error) { + this._last_modified = response.headers['Last-Modified'] + // FIXME: for now this property is set dynamically from backend + // And thus it's not in the geojson file in the server + // So do not let all options to be reset + // Fix is a proper migration so all datalayers settings are + // in DB, and we remove it from geojson flat files. + if (geojson._umap_options) { + geojson._umap_options.editMode = this.options.editMode + } + // In case of maps pre 1.0 still around + if (geojson._storage) geojson._storage.editMode = this.options.editMode + this.fromUmapGeoJSON(geojson) + this.backupOptions() + this.fire('loaded') + this._loading = false } - // In case of maps pre 1.0 still around - if (geojson._storage) geojson._storage.editMode = this.options.editMode - this.fromUmapGeoJSON(geojson) - this.backupOptions() - this.fire('loaded') - this._loading = false }, fromGeoJSON: function (geojson) { @@ -749,8 +751,10 @@ L.U.DataLayer = L.Evented.extend({ url = this.map.proxyUrl(url, this.options.remoteData.ttl) const response = await this.map.request.get(url) this.clear() - this.rawToGeoJSON(await response.text(), this.options.remoteData.format, (geojson) => - this.fromGeoJSON(geojson) + this.rawToGeoJSON( + await response.text(), + this.options.remoteData.format, + (geojson) => this.fromGeoJSON(geojson) ) }, @@ -1391,8 +1395,10 @@ L.U.DataLayer = L.Evented.extend({ const versionsContainer = L.DomUtil.createFieldset(container, L._('Versions'), { callback: async function () { - const [{versions}, response] = await this.map.server.get(this.getVersionsUrl()) - versions.forEach(appendVersion) + const [{ versions }, response, error] = await this.map.server.get( + this.getVersionsUrl() + ) + if (!error) versions.forEach(appendVersion) }, context: this, }) @@ -1401,13 +1407,17 @@ L.U.DataLayer = L.Evented.extend({ restore: async function (version) { if (!this.map.editEnabled) return if (!confirm(L._('Are you sure you want to restore this version?'))) return - const [geojson, response] = await this.map.server.get(this.getVersionUrl(version)) - if (geojson._storage) geojson._umap_options = geojson._storage // Retrocompat. - if (geojson._umap_options) this.setOptions(geojson._umap_options) - this.empty() - if (this.isRemoteLayer()) this.fetchRemoteData() - else this.addData(geojson) - this.isDirty = true + const [geojson, response, error] = await this.map.server.get( + this.getVersionUrl(version) + ) + if (!error) { + if (geojson._storage) geojson._umap_options = geojson._storage // Retrocompat. + if (geojson._umap_options) this.setOptions(geojson._umap_options) + this.empty() + if (this.isRemoteLayer()) this.fetchRemoteData() + else this.addData(geojson) + this.isDirty = true + } }, featuresToGeoJSON: function () { @@ -1544,27 +1554,33 @@ L.U.DataLayer = L.Evented.extend({ map_id: this.map.options.umap_id, pk: this.umap_id, }) - const headers = this._last_modified + const headers = this._last_modified ? { 'If-Unmodified-Since': this._last_modified } : {} - const [data, response] = await this.map.server.post(saveUrl, headers, formData) - // Response contains geojson only if save has conflicted and conflicts have - // been resolved. So we need to reload to get extra data (saved from someone else) - if (data.geojson) { - this.clear() - this.fromGeoJSON(data.geojson) - delete data.geojson + const [data, response, error] = await this.map.server.post( + saveUrl, + headers, + formData + ) + if (!error) { + // Response contains geojson only if save has conflicted and conflicts have + // been resolved. So we need to reload to get extra data (saved from someone else) + if (data.geojson) { + this.clear() + this.fromGeoJSON(data.geojson) + delete data.geojson + } + this._geojson = geojson + this._last_modified = response.headers['Last-Modified'] + this.setUmapId(data.id) + this.updateOptions(data) + this.backupOptions() + this.connectToMap() + this._loaded = true + this.redraw() // Needed for reordering features + this.isDirty = false + this.permissions.save() } - this._geojson = geojson - this._last_modified = response.headers['Last-Modified'] - this.setUmapId(data.id) - this.updateOptions(data) - this.backupOptions() - this.connectToMap() - this._loaded = true - this.redraw() // Needed for reordering features - this.isDirty = false - this.permissions.save() }, saveDelete: async function () { diff --git a/umap/static/umap/js/umap.permissions.js b/umap/static/umap/js/umap.permissions.js index 087f0c0c..de5544fe 100644 --- a/umap/static/umap/js/umap.permissions.js +++ b/umap/static/umap/js/umap.permissions.js @@ -132,13 +132,15 @@ L.U.MapPermissions = L.Class.extend({ }, attach: async function () { - await this.map.server.post(this.getAttachUrl()) - this.options.owner = this.map.options.user - this.map.ui.alert({ - content: L._('Map has been attached to your account'), - level: 'info', - }) - this.map.ui.closePanel() + const [data, response, error] = await this.map.server.post(this.getAttachUrl()) + if (!error) { + this.options.owner = this.map.options.user + this.map.ui.alert({ + content: L._('Map has been attached to your account'), + level: 'info', + }) + this.map.ui.closePanel() + } }, save: async function () { @@ -155,11 +157,17 @@ L.U.MapPermissions = L.Class.extend({ formData.append('owner', this.options.owner && this.options.owner.id) formData.append('share_status', this.options.share_status) } - await this.map.server.post(this.getUrl(), {}, formData) - this.commit() - this.isDirty = false - this.map.continueSaving() - this.map.fire('postsync') + const [data, response, error] = await this.map.server.post( + this.getUrl(), + {}, + formData + ) + if (!error) { + this.commit() + this.isDirty = false + this.map.continueSaving() + this.map.fire('postsync') + } }, getUrl: function () {