From 0389e9a18596335ee70f9fd0b04f22c3cbc52e74 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 26 Mar 2025 10:33:39 +0100 Subject: [PATCH] wip: allow to undo/sync rules When editing Rule(s), we are not editing the map data itself, but a sort of proxy objects. This was done mainly because map.properties.rules is an array of object, and at this time Leaflet.FormBuilder did not know how to edit an array (something like properties.rules.0.condition). Now that we integrated FormBuilder, it still does not know how to do this but we could teach it, or find another way (real Proxy or use reference to the original object in the Rule). --- umap/static/umap/js/modules/form/builder.js | 22 ++----- umap/static/umap/js/modules/rules.js | 23 ++++---- umap/static/umap/js/modules/sync/updaters.js | 41 ++++--------- umap/static/umap/js/modules/umap.js | 4 +- umap/static/umap/js/modules/utils.js | 21 +++++++ umap/static/umap/unittests/sync.js | 57 ------------------- umap/static/umap/unittests/utils.js | 47 +++++++++++++++ .../integration/test_conditional_rules.py | 3 + 8 files changed, 100 insertions(+), 118 deletions(-) diff --git a/umap/static/umap/js/modules/form/builder.js b/umap/static/umap/js/modules/form/builder.js index 55be4d63..88698d39 100644 --- a/umap/static/umap/js/modules/form/builder.js +++ b/umap/static/umap/js/modules/form/builder.js @@ -70,21 +70,7 @@ export class Form extends Utils.WithEvents { } setter(field, value) { - const path = field.split('.') - let obj = this.obj - let what - for (let i = 0, l = path.length; i < l; i++) { - what = path[i] - if (what === path[l - 1]) { - if (typeof value === 'undefined') { - delete obj[what] - } else { - obj[what] = value - } - } else { - obj = obj[what] - } - } + Utils.setObjectValue(this.obj, field, value) } restoreField(field) { @@ -191,7 +177,11 @@ export class MutatingForm extends Form { setter(field, value) { const oldValue = this.getter(field) - super.setter(field, value) + if ('setter' in this.obj) { + this.obj.setter(field, value) + } else { + super.setter(field, value) + } if ('render' in this.obj) { this.obj.render([field], this) } diff --git a/umap/static/umap/js/modules/rules.js b/umap/static/umap/js/modules/rules.js index a6c39ad1..4ecf3724 100644 --- a/umap/static/umap/js/modules/rules.js +++ b/umap/static/umap/js/modules/rules.js @@ -17,20 +17,10 @@ class Rule { this.parse() } - get isDirty() { - return this._isDirty - } - - set isDirty(status) { - this._isDirty = status - if (status) this._umap.isDirty = status - } - constructor(umap, condition = '', options = {}) { // TODO make this public properties when browser coverage is ok // cf https://caniuse.com/?search=public%20class%20field this._condition = null - this._isDirty = false this.OPERATORS = [ ['>', this.gt], ['<', this.lt], @@ -190,17 +180,25 @@ class Rule { _delete() { this._umap.rules.rules = this._umap.rules.rules.filter((rule) => rule !== this) + this._umap.rules.commit() + } + + setter(key, value) { + const oldRules = Utils.CopyJSON(this._umap.properties.rules || {}) + Utils.setObjectValue(this, key, value) + this._umap.rules.commit() + this._umap.sync.update('properties.rules', this._umap.properties.rules, oldRules) } } export default class Rules { constructor(umap) { this._umap = umap - this.rules = [] this.load() } load() { + this.rules = [] if (!this._umap.properties.rules?.length) return for (const { condition, options } of this._umap.properties.rules) { if (!condition) continue @@ -222,8 +220,8 @@ export default class Rules { else if (finalIndex > initialIndex) newIdx = referenceIdx else newIdx = referenceIdx + 1 this.rules.splice(newIdx, 0, moved) - moved.isDirty = true this._umap.render(['rules']) + this.commit() } edit(container) { @@ -242,7 +240,6 @@ export default class Rules { addRule() { const rule = new Rule(this._umap) - rule.isDirty = true this.rules.push(rule) rule.edit(map) } diff --git a/umap/static/umap/js/modules/sync/updaters.js b/umap/static/umap/js/modules/sync/updaters.js index 7388168d..e83ef2a7 100644 --- a/umap/static/umap/js/modules/sync/updaters.js +++ b/umap/static/umap/js/modules/sync/updaters.js @@ -1,4 +1,4 @@ -import { fieldInSchema } from '../utils.js' +import * as Utils from '../utils.js' /** * Updaters are classes able to convert messages @@ -10,27 +10,6 @@ class BaseUpdater { this._umap = umap } - updateObjectValue(obj, key, value) { - const parts = key.split('.') - const lastKey = parts.pop() - - // Reduce the current list of attributes, - // to find the object to set the property onto - const objectToSet = parts.reduce((currentObj, part) => { - if (currentObj !== undefined && part in currentObj) return currentObj[part] - }, obj) - - // In case the given path doesn't exist, stop here - if (objectToSet === undefined) return - - // Set the value (or delete it) - if (typeof value === 'undefined') { - delete objectToSet[lastKey] - } else { - objectToSet[lastKey] = value - } - } - getDataLayerFromID(layerId) { return this._umap.getDataLayerByUmapId(layerId) } @@ -43,8 +22,8 @@ class BaseUpdater { export class MapUpdater extends BaseUpdater { update({ key, value }) { - if (fieldInSchema(key)) { - this.updateObjectValue(this._umap, key, value) + if (Utils.fieldInSchema(key)) { + Utils.setObjectValue(this._umap, key, value) } this._umap.onPropertiesUpdated([key]) @@ -73,8 +52,8 @@ export class DataLayerUpdater extends BaseUpdater { update({ key, metadata, value }) { const datalayer = this.getDataLayerFromID(metadata.id) - if (fieldInSchema(key)) { - this.updateObjectValue(datalayer, key, value) + if (Utils.fieldInSchema(key)) { + Utils.setObjectValue(datalayer, key, value) } else { console.debug( 'Not applying update for datalayer because key is not in the schema', @@ -127,7 +106,7 @@ export class FeatureUpdater extends BaseUpdater { const feature = this.getFeatureFromMetadata(metadata) feature.geometry = value } else { - this.updateObjectValue(feature, key, value) + Utils.setObjectValue(feature, key, value) feature.datalayer.indexProperties(feature) } @@ -148,8 +127,8 @@ export class FeatureUpdater extends BaseUpdater { export class MapPermissionsUpdater extends BaseUpdater { update({ key, value }) { - if (fieldInSchema(key)) { - this.updateObjectValue(this._umap.permissions, key, value) + if (Utils.fieldInSchema(key)) { + Utils.setObjectValue(this._umap.permissions, key, value) } } @@ -160,8 +139,8 @@ export class MapPermissionsUpdater extends BaseUpdater { export class DataLayerPermissionsUpdater extends BaseUpdater { update({ key, value, metadata }) { - if (fieldInSchema(key)) { - this.updateObjectValue(this.getDataLayerFromID(metadata.id), key, value) + if (Utils.fieldInSchema(key)) { + Utils.setObjectValue(this.getDataLayerFromID(metadata.id), key, value) } } diff --git a/umap/static/umap/js/modules/umap.js b/umap/static/umap/js/modules/umap.js index 4e48e385..a0b38864 100644 --- a/umap/static/umap/js/modules/umap.js +++ b/umap/static/umap/js/modules/umap.js @@ -1162,7 +1162,6 @@ export default class Umap { } async save() { - this.rules.commit() const geojson = { type: 'Feature', geometry: this.geometry(), @@ -1323,6 +1322,9 @@ export default class Umap { this.bottomBar.redraw() break case 'data': + if (fields.includes('properties.rules')) { + this.rules.load() + } this.eachVisibleDataLayer((datalayer) => { datalayer.redraw() }) diff --git a/umap/static/umap/js/modules/utils.js b/umap/static/umap/js/modules/utils.js index 4c36abfe..58e3e1dd 100644 --- a/umap/static/umap/js/modules/utils.js +++ b/umap/static/umap/js/modules/utils.js @@ -484,6 +484,27 @@ export const debounce = (callback, wait) => { } } +export function setObjectValue(obj, key, value) { + const parts = key.split('.') + const lastKey = parts.pop() + + // Reduce the current list of attributes, + // to find the object to set the property onto + const objectToSet = parts.reduce((currentObj, part) => { + if (currentObj !== undefined && part in currentObj) return currentObj[part] + }, obj) + + // In case the given path doesn't exist, stop here + if (objectToSet === undefined) return + + // Set the value (or delete it) + if (typeof value === 'undefined') { + delete objectToSet[lastKey] + } else { + objectToSet[lastKey] = value + } +} + export const COLORS = [ 'Black', 'Navy', diff --git a/umap/static/umap/unittests/sync.js b/umap/static/umap/unittests/sync.js index c4e34b4a..f1888441 100644 --- a/umap/static/umap/unittests/sync.js +++ b/umap/static/umap/unittests/sync.js @@ -49,63 +49,6 @@ describe('#dispatch', () => { }) }) -describe('Updaters', () => { - describe('BaseUpdater', () => { - let updater - let map - let obj - - beforeEach(() => { - map = {} - updater = new MapUpdater(map) - obj = {} - }) - it('should be able to set object properties', () => { - let obj = {} - updater.updateObjectValue(obj, 'foo', 'foo') - expect(obj).deep.equal({ foo: 'foo' }) - }) - - it('should be able to set object properties recursively on existing objects', () => { - let obj = { foo: {} } - updater.updateObjectValue(obj, 'foo.bar', 'foo') - expect(obj).deep.equal({ foo: { bar: 'foo' } }) - }) - - it('should be able to set object properties recursively on deep objects', () => { - let obj = { foo: { bar: { baz: {} } } } - updater.updateObjectValue(obj, 'foo.bar.baz.test', 'value') - expect(obj).deep.equal({ foo: { bar: { baz: { test: 'value' } } } }) - }) - - it('should be able to replace object properties recursively on deep objects', () => { - let obj = { foo: { bar: { baz: { test: 'test' } } } } - updater.updateObjectValue(obj, 'foo.bar.baz.test', 'value') - expect(obj).deep.equal({ foo: { bar: { baz: { test: 'value' } } } }) - }) - - it('should not set object properties recursively on non-existing objects', () => { - let obj = { foo: {} } - updater.updateObjectValue(obj, 'bar.bar', 'value') - - expect(obj).deep.equal({ foo: {} }) - }) - - it('should delete keys for undefined values', () => { - let obj = { foo: 'foo' } - updater.updateObjectValue(obj, 'foo', undefined) - - expect(obj).deep.equal({}) - }) - - it('should delete keys for undefined values, recursively', () => { - let obj = { foo: { bar: 'bar' } } - updater.updateObjectValue(obj, 'foo.bar', undefined) - - expect(obj).deep.equal({ foo: {} }) - }) - }) -}) describe('Operations', () => { describe('haveSameContext', () => { diff --git a/umap/static/umap/unittests/utils.js b/umap/static/umap/unittests/utils.js index fb06900b..e3681f2f 100644 --- a/umap/static/umap/unittests/utils.js +++ b/umap/static/umap/unittests/utils.js @@ -862,4 +862,51 @@ describe('Utils', () => { assert.equal(Utils.isObject(''), false) }) }) + + describe('setObjectValue', () => { + it('should be able to set object properties', () => { + let obj = {} + Utils.setObjectValue(obj, 'foo', 'foo') + expect(obj).deep.equal({ foo: 'foo' }) + }) + + it('should be able to set object properties recursively on existing objects', () => { + let obj = { foo: {} } + Utils.setObjectValue(obj, 'foo.bar', 'foo') + expect(obj).deep.equal({ foo: { bar: 'foo' } }) + }) + + it('should be able to set object properties recursively on deep objects', () => { + let obj = { foo: { bar: { baz: {} } } } + Utils.setObjectValue(obj, 'foo.bar.baz.test', 'value') + expect(obj).deep.equal({ foo: { bar: { baz: { test: 'value' } } } }) + }) + + it('should be able to replace object properties recursively on deep objects', () => { + let obj = { foo: { bar: { baz: { test: 'test' } } } } + Utils.setObjectValue(obj, 'foo.bar.baz.test', 'value') + expect(obj).deep.equal({ foo: { bar: { baz: { test: 'value' } } } }) + }) + + it('should not set object properties recursively on non-existing objects', () => { + let obj = { foo: {} } + Utils.setObjectValue(obj, 'bar.bar', 'value') + + expect(obj).deep.equal({ foo: {} }) + }) + + it('should delete keys for undefined values', () => { + let obj = { foo: 'foo' } + Utils.setObjectValue(obj, 'foo', undefined) + + expect(obj).deep.equal({}) + }) + + it('should delete keys for undefined values, recursively', () => { + let obj = { foo: { bar: 'bar' } } + Utils.setObjectValue(obj, 'foo.bar', undefined) + + expect(obj).deep.equal({ foo: {} }) + }) + }) }) diff --git a/umap/tests/integration/test_conditional_rules.py b/umap/tests/integration/test_conditional_rules.py index e31a6480..37dabaff 100644 --- a/umap/tests/integration/test_conditional_rules.py +++ b/umap/tests/integration/test_conditional_rules.py @@ -261,6 +261,9 @@ def test_can_create_new_rule(live_server, page, openmap): page.get_by_title("AliceBlue").first.click() colors = getColors(markers) assert colors.count("rgb(240, 248, 255)") == 3 + page.get_by_role("button", name="Undo").click() + colors = getColors(markers) + assert colors.count("rgb(240, 248, 255)") == 0 def test_can_deactive_rule_from_list(live_server, page, openmap):