From ffa020e4c0f4b74abc471b88bb8659c75a6c16e5 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Tue, 6 Jun 2023 18:14:03 +0200 Subject: [PATCH 01/11] Add a persistence configuration page It is accessible from the add-on settings page and has both a design and a code tab. The design tab allows to set persistence strategies for Items, define cron strategies and set the default strategies. It does not duplicate names for (cron) persistence strategies and duplicate configs for the same set of Items. The code tab also allows to also specify threshold and time filters and needs minor adjustment once https://github.com/openhab/openhab-core/pull/3642 is merged code completion is not provided. Signed-off-by: Florian Hotze --- bundles/org.openhab.ui/web/src/js/routes.js | 13 + .../pages/settings/addons/addon-config.vue | 17 + .../persistence/configuration-popup.vue | 78 ++++ .../persistence/cron-strategy-popup.vue | 68 +++ .../settings/persistence/persistence-edit.vue | 426 ++++++++++++++++++ .../settings/persistence/strategy-picker.vue | 47 ++ 6 files changed, 649 insertions(+) create mode 100644 bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue create mode 100644 bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue create mode 100644 bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue create mode 100644 bundles/org.openhab.ui/web/src/pages/settings/persistence/strategy-picker.vue diff --git a/bundles/org.openhab.ui/web/src/js/routes.js b/bundles/org.openhab.ui/web/src/js/routes.js index 37c436d24e..6380a56c2e 100644 --- a/bundles/org.openhab.ui/web/src/js/routes.js +++ b/bundles/org.openhab.ui/web/src/js/routes.js @@ -34,6 +34,8 @@ const InboxListPage = () => import(/* webpackChunkName: "admin-config" */ '../pa const TransformationsListPage = () => import(/* webpackChunkName: "admin-config" */ '../pages/settings/transformations/transformations-list.vue') const TransformationsEditPage = () => import(/* webpackChunkName: "admin-rules" */ '../pages/settings/transformations/transformation-edit.vue') +const PersistenceEditPage = () => import(/* webpackChunkName: "admin-config" */ '../pages/settings/persistence/persistence-edit.vue') + const SemanticModelPage = () => import(/* webpackChunkName: "admin-config" */ '../pages/settings/model/model.vue') const PagesListPage = () => import(/* webpackChunkName: "admin-pages" */ '../pages/settings/pages/pages-list.vue') @@ -237,6 +239,17 @@ export default [ beforeEnter: [enforceAdminForRoute], async: loadAsync(SemanticModelPage) }, + { + path: 'persistence/', + routes: [ + { + path: ':serviceId', + beforeEnter: [enforceAdminForRoute], + beforeLeave: [checkDirtyBeforeLeave], + async: loadAsync(PersistenceEditPage) + } + ] + }, { path: 'rules/', beforeEnter: [enforceAdminForRoute], diff --git a/bundles/org.openhab.ui/web/src/pages/settings/addons/addon-config.vue b/bundles/org.openhab.ui/web/src/pages/settings/addons/addon-config.vue index b3c063af37..4bcc660c92 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/addons/addon-config.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/addons/addon-config.vue @@ -8,6 +8,15 @@ + + + + + Persistence configuration + + + + @@ -74,6 +83,14 @@ export default { strippedAddonId: '' } }, + computed: { + type () { + return this.addonId.split('-')[0] + }, + name () { + return this.addonId.split('-')[1] + } + }, methods: { save () { let promises = [] diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue new file mode 100644 index 0000000000..2ea2e0d4a5 --- /dev/null +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue @@ -0,0 +1,78 @@ + + + diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue new file mode 100644 index 0000000000..3095e2c5bd --- /dev/null +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue @@ -0,0 +1,68 @@ + + + diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue new file mode 100644 index 0000000000..6c17b69f8e --- /dev/null +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -0,0 +1,426 @@ + + + + + diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/strategy-picker.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/strategy-picker.vue new file mode 100644 index 0000000000..5d2fc64d4a --- /dev/null +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/strategy-picker.vue @@ -0,0 +1,47 @@ + + + + + From 8d3e893d562c281cf8d857193fa9eff5c69bd85a Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Tue, 6 Jun 2023 20:21:06 +0200 Subject: [PATCH 02/11] Adjust message for smaller screens Signed-off-by: Florian Hotze --- .../pages/settings/persistence/configuration-popup.vue | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue index 2ea2e0d4a5..7a7e20f4c1 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue @@ -17,15 +17,19 @@ Items - - ... whose members are to be persisted. + + + + ... to be persisted. Strategies - From 36cac8fa151de7d00507a7ba407a51975cf8a9f4 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Tue, 6 Jun 2023 20:32:41 +0200 Subject: [PATCH 03/11] Fix newly created persistence not editable Signed-off-by: Florian Hotze --- .../src/pages/settings/persistence/persistence-edit.vue | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue index 6c17b69f8e..75cd0dd05a 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -255,6 +255,8 @@ export default { }).catch((e) => { if (e === 404 || e === 'Not Found') { this.initializeNewPersistence() + this.loading = false + this.ready = true } else { Promise.reject(e) } @@ -264,8 +266,12 @@ export default { if (!this.isEditable) return if (this.currentTab === 'code') this.fromYaml() return this.$oh.api.put('/rest/persistence/' + this.persistence.serviceId, this.persistence).then((data) => { - this.newPersistence = false this.dirty = false + if (this.newPersistence) { + this.newPersistence = false + this.ready = false + this.load() + } if (!noToast) { this.$f7.toast.create({ text: 'Persistence configuration saved', From aa5b9e6baffdf95a945f5bc9a7854f8e3ec46d9f Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Wed, 7 Jun 2023 14:11:42 +0200 Subject: [PATCH 04/11] Minor refactoring to reduce code duplication & Add debug logging Signed-off-by: Florian Hotze --- .../settings/persistence/persistence-edit.vue | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue index 75cd0dd05a..4b2bf07f41 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -65,7 +65,7 @@ icon-ios="f7:minus_circle_filled" icon-md="material:remove_circle_outline" @click="showSwipeout" /> - Delete @@ -93,7 +93,7 @@ icon-ios="f7:minus_circle_filled" icon-md="material:remove_circle_outline" @click="showSwipeout" /> - Delete @@ -321,28 +321,11 @@ export default { }, saveConfiguration (index, configuration) { const idx = this.persistence.configs.findIndex((cfg) => cfg.items.join() === configuration.items.join()) - console.log(index, idx) if (idx !== -1 && idx !== index) { this.$f7.dialog.alert('A configuration for this/these Item(s) already exists!') return } - if (index === null) { - this.persistence.configs.push(configuration) - } else { - this.persistence.configs[index] = configuration - this.$forceUpdate() - } - }, - deleteConfiguration (ev, index) { - let swipeoutElement = ev.target - if (!this.isEditable) return - ev.cancelBubble = true - while (!swipeoutElement.classList.contains('swipeout')) { - swipeoutElement = swipeoutElement.parentElement - } - this.$f7.swipeout.delete(swipeoutElement, () => { - this.persistence.configs.splice(index, 1) - }) + this.saveModule('configs', index, configuration) }, editCronStrategy (ev, index, cronStrategy) { this.currentCronStrategy = cronStrategy @@ -370,14 +353,21 @@ export default { this.$f7.dialog.alert('A (cron) strategy with the same name already exists!') return } + this.saveModule('cronStrategies', index, cronStrategy) + }, + saveModule (module, index, updatedModule) { if (index === null) { - this.persistence.cronStrategies.push(cronStrategy) + console.debug(`Adding ${module}:`) + console.debug(updatedModule) + this.persistence[module].push(updatedModule) } else { - this.persistence.cronStrategies[index] = cronStrategy + console.debug(`Updating ${module} at index ${index}:`) + console.debug(updatedModule) + this.persistence[module][index] = updatedModule this.$forceUpdate() } }, - deleteCronStrategy (ev, index) { + deleteModule (ev, module, index) { let swipeoutElement = ev.target if (!this.isEditable) return ev.cancelBubble = true @@ -385,7 +375,9 @@ export default { swipeoutElement = swipeoutElement.parentElement } this.$f7.swipeout.delete(swipeoutElement, () => { - this.persistence.cronStrategies.splice(index, 1) + console.debug(`Removing ${module}:`) + console.debug(this.persistence[module][index]) + this.persistence[module].splice(index, 1) }) }, onEditorInput (value) { From 17ab42f7e2888efe056f22937b3f3cf1374b3b19 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Wed, 7 Jun 2023 22:13:11 +0200 Subject: [PATCH 05/11] Add delete button Signed-off-by: Florian Hotze --- .../settings/persistence/persistence-edit.vue | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue index 4b2bf07f41..d91c33da96 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -104,14 +104,21 @@ + md="material:control_point"/> + :value="persistence.defaults" @strategiesSelected="persistence.defaults = $event"/> + + + + Remove persistence configuration + + + @@ -287,6 +294,17 @@ export default { }).open() }) }, + deletePersistence () { + this.$f7.dialog.confirm( + `Are you sure you want to delete persistence configuration for ${this.serviceId}?`, + 'Delete persistence configuration', + () => { + this.$oh.api.delete('/rest/persistence/' + this.serviceId).then(() => { + this.$f7router.back(`/settings/addons/persistence-${this.serviceId}/config`, { force: true }) + }) + } + ) + }, showSwipeout (ev) { let swipeoutElement = ev.target ev.cancelBubble = true From cd08d62f4ee63c088dc3a16ce2d721c33a4ed454 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Wed, 7 Jun 2023 22:33:05 +0200 Subject: [PATCH 06/11] Improve handling of not editable persistence configurtion Signed-off-by: Florian Hotze --- .../pages/settings/persistence/persistence-edit.vue | 11 ++++++----- .../pages/settings/persistence/strategy-picker.vue | 9 ++++++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue index d91c33da96..708b5d04db 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -56,7 +56,7 @@ Configuration - + - + @@ -104,12 +104,11 @@ + md="material:control_point" /> - + @@ -317,6 +316,7 @@ export default { } }, editConfiguration (ev, index, configuration) { + if (!this.isEditable) return this.currentConfiguration = configuration const popup = { @@ -346,6 +346,7 @@ export default { this.saveModule('configs', index, configuration) }, editCronStrategy (ev, index, cronStrategy) { + if (!this.isEditable) return this.currentCronStrategy = cronStrategy const popup = { diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/strategy-picker.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/strategy-picker.vue index 5d2fc64d4a..e0a8f4a855 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/strategy-picker.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/strategy-picker.vue @@ -1,13 +1,16 @@ @@ -24,7 +27,7 @@ diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-popup.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-popup.vue new file mode 100644 index 0000000000..a970f28a09 --- /dev/null +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-popup.vue @@ -0,0 +1,65 @@ + + + diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue index 708b5d04db..976dddc28b 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -42,6 +42,19 @@ +
+ + Filters + +
+ + {{ ft.label }} + + + + +
+
@@ -52,6 +65,7 @@ +
Configuration @@ -80,6 +94,7 @@
+
Strategies @@ -108,7 +123,43 @@ - + +
+ +
+ + Filters + +
+ + {{ ft.label }} + + + + + + + Delete + + + + + + + + + +
@@ -165,9 +216,19 @@ import YAML from 'yaml' import CronStrategyPopup from '@/pages/settings/persistence/cron-strategy-popup.vue' import StrategyPicker from '@/pages/settings/persistence/strategy-picker.vue' import ConfigurationPopup from '@/pages/settings/persistence/configuration-popup.vue' +import FilterPopup from '@/pages/settings/persistence/filter-popup.vue' import cloneDeep from 'lodash/cloneDeep' import fastDeepEqual from 'fast-deep-equal/es6' +const filterInvertedParameter = { + advanced: false, + description: 'Whether to invert the above filter, i.e. persist values that do not equal the above values or are outside of the specified range', + label: 'Inverted', + name: 'inverted', + required: false, + type: 'BOOLEAN' +} + export default { mixins: [DirtyMixin], components: { @@ -186,8 +247,109 @@ export default { currentTab: 'design', currentConfiguration: null, currentCronStrategy: null, + currentFilter: null, predefinedStrategies: ['everyChange', 'everyUpdate', 'restoreOnStartup'], + filterTypes: [ + { + name: 'thresholdFilters', + label: 'Threshold', + configDescriptionParameters: [ + { + advanced: false, + description: 'Difference to last stored value that must be exceeded to persist a new value', + label: 'Value', + name: 'value', + required: true, + type: 'DECIMAL' + }, + { + advanced: false, + description: 'Whether the difference is relative (i.e. in percent)', + label: 'Relative', + name: 'relative', + required: false, + type: 'BOOLEAN' + }, + { + advanced: false, + description: 'Unit of the given value, only used for UoM Items and if relative is disabled', + label: 'Unit', + name: 'unit', + required: false, + type: 'STRING' + } + ] + }, + { + name: 'timeFilters', + label: 'Time', + configDescriptionParameters: [ + { + advanced: false, + description: 'Amount of time that must have passed since the last persist', + label: 'Value', + name: 'value', + required: true, + type: 'DECIMAL' + }, + { + advanced: false, + description: 'Time unit (defaults to seconds s)', + label: 'Unit', + name: 'unit', + required: false, + type: 'STRING' + } + ] + }, + { + name: 'equalsFilters', + label: 'Equals/Not Equals', + configDescriptionParameters: [ + { + advanced: false, + description: 'Enter values seperated by comma (use point . as decimal point), e.g. one, two, three, to be persisted', + label: 'Values', + name: 'values', + required: true, + type: '' + }, + filterInvertedParameter + ] + }, + { + name: 'includeFilters', + label: 'Include/Exclude', + configDescriptionParameters: [ + { + advanced: false, + description: 'Lower bound of the range of value to be persisted', + label: 'Lower Bound', + name: 'lower', + required: true, + type: 'DECIMAL' + }, + { + advanced: false, + description: 'Lower bound of the range of value to be persisted', + label: 'Upper Bound', + name: 'upper', + required: true, + type: 'DECIMAL' + }, + { + advanced: false, + description: 'Unit of the given bounds, only used for UoM Items', + label: 'Unit', + name: 'unit', + required: false, + type: 'STRING' + }, + filterInvertedParameter + ] + } + ], notEditableMgs: 'This persistence configuration is not editable because it has been provisioned from a file.' } }, @@ -197,6 +359,14 @@ export default { }, strategies () { return this.predefinedStrategies.concat(this.persistence.cronStrategies.map(cs => cs.name)) + }, + filters () { + let names = [] + for (let i = 0; i < this.filterTypes.length; i++) { + const filterTypeName = this.filterTypes[i].name + if (this.persistence[filterTypeName]) names = names.concat(this.persistence[filterTypeName].map((f) => f.name)) + } + return names } }, watch: { @@ -271,6 +441,21 @@ export default { save (noToast) { if (!this.isEditable) return if (this.currentTab === 'code') this.fromYaml() + + // Ensure relative is set on threshold filter, otherwise the save request fails with a 500 + this.persistence.thresholdFilters.forEach((f) => { + if (f.relative === undefined) f.relative = false + }) + // Ensure inverted is set for equals and include filter, otherwise the save request fails with a 500 + this.persistence.equalsFilters.forEach((f) => { + if (f.inverted === undefined) f.inverted = false + }) + this.persistence.includeFilters.forEach((f) => { + if (f.inverted === undefined) f.inverted = false + }) + // Update the code tab + if (this.persistenceYaml) this.toYaml() + return this.$oh.api.put('/rest/persistence/' + this.persistence.serviceId, this.persistence).then((data) => { this.dirty = false if (this.newPersistence) { @@ -331,7 +516,8 @@ export default { }, { props: { configuration: this.currentConfiguration, - strategies: this.strategies + strategies: this.strategies, + filters: this.filters } }) @@ -339,7 +525,7 @@ export default { }, saveConfiguration (index, configuration) { const idx = this.persistence.configs.findIndex((cfg) => cfg.items.join() === configuration.items.join()) - if (idx !== -1 && idx !== index) { + if (idx !== -1 && idx === index) { this.$f7.dialog.alert('A configuration for this/these Item(s) already exists!') return } @@ -368,12 +554,49 @@ export default { }, saveCronStrategy (index, cronStrategy) { const idx = this.persistence.cronStrategies.findIndex((cs) => cs.name === cronStrategy.name) - if ((idx !== -1 && idx !== index) || this.predefinedStrategies.includes(cronStrategy.name)) { + if ((idx !== -1 && idx === index) || this.predefinedStrategies.includes(cronStrategy.name)) { this.$f7.dialog.alert('A (cron) strategy with the same name already exists!') return } this.saveModule('cronStrategies', index, cronStrategy) }, + editFilter (ev, filterType, index, filter) { + if (!this.isEditable) return + this.currentFilter = filter + + // Stringify values array from equals filter + if (filterType.name === 'equalsFilters' && filter) filter.values = filter.values.join(', ') + + const popup = { + component: FilterPopup + } + this.$f7router.navigate({ + url: 'filter-config', + route: { + path: 'filter-config', + popup + } + }, { + props: { + filter: this.currentFilter, + filterType: filterType, + filterConfigDescriptionParameters: filterType.configDescriptionParameters + } + }) + + this.$f7.once('filterUpdate', (ev, ftn) => this.saveFilter(ftn, index, ev)) + }, + saveFilter (filterTypeName, index, filter) { + const idx = this.filters.findIndex((f) => f === filter.name) + if (index === null && idx !== -1) { + this.$f7.dialog.alert('A filter with the same name already exists!') + return + } + // Convert comma separated string to array for equals filter + if (filterTypeName === 'equalsFilters') filter.values = filter.values.split(',').map((v) => v.trim()) + + this.saveModule(filterTypeName, index, filter) + }, saveModule (module, index, updatedModule) { if (index === null) { console.debug(`Adding ${module}:`) @@ -409,7 +632,9 @@ export default { cronStrategies: this.persistence.cronStrategies, defaultStrategies: this.persistence.defaults, thresholdFilters: this.persistence.thresholdFilters, - timeFilters: this.persistence.timeFilters + timeFilters: this.persistence.timeFilters, + equalsFilters: this.persistence.equalsFilters, + includeFilters: this.persistence.includeFilters }) }, fromYaml () { @@ -421,6 +646,8 @@ export default { this.$set(this.persistence, 'defaults', updatedPersistence.defaultStrategies) this.$set(this.persistence, 'thresholdFilters', updatedPersistence.thresholdFilters) this.$set(this.persistence, 'timeFilters', updatedPersistence.timeFilters) + this.$set(this.persistence, 'equalsFilters', updatedPersistence.equalsFilters) + this.$set(this.persistence, 'includeFilters', updatedPersistence.includeFilters) return true } catch (e) { this.$f7.dialog.alert(e).open() From fe4fa13d119260bcd84cf58c9309908bd12cdb0e Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Sat, 24 Jun 2023 00:34:03 +0200 Subject: [PATCH 09/11] Minor fixes - Fix config/cron strategy already existing checks. - Remove cron strategies and filters from configs when they are removed to avoid 400 Bad Request. Signed-off-by: Florian Hotze --- .../settings/persistence/persistence-edit.vue | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue index 976dddc28b..cb4f4d97d3 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -108,7 +108,7 @@ icon-ios="f7:minus_circle_filled" icon-md="material:remove_circle_outline" @click="showSwipeout" /> - Delete @@ -144,7 +144,7 @@ icon-ios="f7:minus_circle_filled" icon-md="material:remove_circle_outline" @click="showSwipeout" /> - Delete @@ -525,7 +525,7 @@ export default { }, saveConfiguration (index, configuration) { const idx = this.persistence.configs.findIndex((cfg) => cfg.items.join() === configuration.items.join()) - if (idx !== -1 && idx === index) { + if (idx !== -1 && idx !== index) { this.$f7.dialog.alert('A configuration for this/these Item(s) already exists!') return } @@ -554,12 +554,21 @@ export default { }, saveCronStrategy (index, cronStrategy) { const idx = this.persistence.cronStrategies.findIndex((cs) => cs.name === cronStrategy.name) - if ((idx !== -1 && idx === index) || this.predefinedStrategies.includes(cronStrategy.name)) { + if ((idx !== -1 && idx !== index) || this.predefinedStrategies.includes(cronStrategy.name)) { this.$f7.dialog.alert('A (cron) strategy with the same name already exists!') return } this.saveModule('cronStrategies', index, cronStrategy) }, + deleteCronStrategy (ev, index) { + // Remove cron strategy from configs, otherwise we get a 400 + const csName = this.persistence.cronStrategies[index].name + this.persistence.configs.forEach((cfg) => { + const i = cfg.strategies.findIndex((cs) => cs === csName) + cfg.strategies.splice(i, 1) + }) + this.deleteModule(ev, 'cronStrategies', index) + }, editFilter (ev, filterType, index, filter) { if (!this.isEditable) return this.currentFilter = filter @@ -597,6 +606,15 @@ export default { this.saveModule(filterTypeName, index, filter) }, + deleteFilter (ev, module, index) { + // Remove filter from configs, otherwise we get a 400 + const filterName = this.persistence[module][index].name + this.persistence.configs.forEach((cfg) => { + const i = cfg.filters.findIndex((f) => f === filterName) + cfg.filters.splice(i, 1) + }) + this.deleteModule(ev, module, index) + }, saveModule (module, index, updatedModule) { if (index === null) { console.debug(`Adding ${module}:`) From 0c0d6650b674590033cca6b54c582f8204d1a0f1 Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Sat, 24 Jun 2023 12:37:46 +0200 Subject: [PATCH 10/11] Apply suggestions from code review Co-authored-by: J-N-K Signed-off-by: Florian Hotze --- .../web/src/pages/settings/persistence/persistence-edit.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue index cb4f4d97d3..818f2dee79 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -287,7 +287,7 @@ export default { configDescriptionParameters: [ { advanced: false, - description: 'Amount of time that must have passed since the last persist', + description: 'Amount of time that must have passed since the last value has been persisted', label: 'Value', name: 'value', required: true, @@ -309,7 +309,7 @@ export default { configDescriptionParameters: [ { advanced: false, - description: 'Enter values seperated by comma (use point . as decimal point), e.g. one, two, three, to be persisted', + description: 'Enter values separated by comma (use point . as decimal point), e.g. one, two, three, to be persisted', label: 'Values', name: 'values', required: true, From dccea80f913f91d28d266a5ba8132b91ccefd0cc Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Wed, 28 Jun 2023 11:14:00 +0200 Subject: [PATCH 11/11] Minor fixes and enhancements - Display a message when no filters are available. - Hide the delete button in create mode. - Pre-select everyChange as persistence strategy for new configs. - Make all code only use the filterTypes definitions and don't rely on the existence of any filter type's array. - Allow numbers and underslash for cron strategy & filter names. - Validate that lower bound is less than upper bound for includeFilters. Signed-off-by: Florian Hotze --- .../persistence/configuration-popup.vue | 4 +- .../persistence/cron-strategy-popup.vue | 2 +- .../settings/persistence/filter-picker.vue | 10 +++-- .../settings/persistence/filter-popup.vue | 8 +++- .../settings/persistence/persistence-edit.vue | 42 +++++++++++-------- 5 files changed, 43 insertions(+), 23 deletions(-) diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue index 0f37f97c17..e15bbf98cb 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/configuration-popup.vue @@ -64,7 +64,9 @@ export default { return { currentConfiguration: this.configuration || { items: [], - strategies: [], + strategies: [ + 'everyChange' + ], filters: [] } } diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue index 620c3fa372..80d1d53d28 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue @@ -21,7 +21,7 @@ @input="currentCronStrategy.name = $event.target.value" :disabled="!createMode" :info="(createMode) ? 'Note: cannot be changed after the creation' : ''" - required validate pattern="[A-Za-z]+" error-message="Required. A-Z,a-z only" /> + required validate pattern="[A-Za-z0-9_]+" error-message="Required. A-Z,a-z only" /> diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-picker.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-picker.vue index f51f632771..148741bdb2 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-picker.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-picker.vue @@ -1,16 +1,20 @@ diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-popup.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-popup.vue index a970f28a09..3384e4bdf1 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-popup.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-popup.vue @@ -21,7 +21,7 @@ @input="currentFilter.name = $event.target.value" :disabled="!createMode" :info="(createMode) ? 'Note: cannot be changed after the creation' : ''" - required validate pattern="[A-Za-z]+" error-message="Required. A-Z,a-z only" /> + required validate pattern="[A-Za-z0-9_]+" error-message="Required. A-Z,a-z only" /> @@ -57,6 +57,12 @@ export default { this.$f7.dialog.alert('Please review the configuration and correct validation errors') return } + if (this.filterType.name === 'includeFilters') { + if (this.currentFilter.upper <= this.currentFilter.lower) { + this.$f7.dialog.alert('The lower bound value must be less than the upper bound value') + return + } + } this.$f7.emit('filterUpdate', this.currentFilter, this.filterType.name) this.$refs.modulePopup.close() } diff --git a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue index 818f2dee79..6b287fd73f 100644 --- a/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue +++ b/bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue @@ -162,9 +162,9 @@ - + - + Remove persistence configuration @@ -250,6 +250,8 @@ export default { currentFilter: null, predefinedStrategies: ['everyChange', 'everyUpdate', 'restoreOnStartup'], + // Filter configuration is completely based on these definitions, when adding new filters, no code needs to be updated. + // However, please note that some validation and checks are in place for some filter types in save(), editFilter(), saveFilter() and filter-popup.vue filterTypes: [ { name: 'thresholdFilters', @@ -324,7 +326,7 @@ export default { configDescriptionParameters: [ { advanced: false, - description: 'Lower bound of the range of value to be persisted', + description: 'Lower bound of the range of values to be persisted', label: 'Lower Bound', name: 'lower', required: true, @@ -332,7 +334,7 @@ export default { }, { advanced: false, - description: 'Lower bound of the range of value to be persisted', + description: 'Upper bound of the range of values to be persisted', label: 'Upper Bound', name: 'upper', required: true, @@ -413,10 +415,10 @@ export default { name: 'everyDay', cronExpression: '0 0 0 * * ?' } - ], - thresholdFilters: [], - timeFilters: [] + ] } + // Dynamically add empty arrays for all filter types defined in the filterTypes object + this.filterTypes.forEach((ft) => { this.persistence[ft.name] = [] }) this.ready = true }, load () { @@ -442,6 +444,10 @@ export default { if (!this.isEditable) return if (this.currentTab === 'code') this.fromYaml() + // Ensure arrays for all filter types defined in the filterTypes object are existent + this.filterTypes.forEach((ft) => { + if (!this.persistence[ft.name]) this.persistence[ft.name] = [] + }) // Ensure relative is set on threshold filter, otherwise the save request fails with a 500 this.persistence.thresholdFilters.forEach((f) => { if (f.relative === undefined) f.relative = false @@ -604,6 +610,9 @@ export default { // Convert comma separated string to array for equals filter if (filterTypeName === 'equalsFilters') filter.values = filter.values.split(',').map((v) => v.trim()) + // Ensure that the filter type array exists. + // Even though the arrays are created when a new persistence config is initialized, we need this for existing, old configs. + if (!this.persistence[filterTypeName]) this.persistence[filterTypeName] = [] this.saveModule(filterTypeName, index, filter) }, deleteFilter (ev, module, index) { @@ -645,15 +654,15 @@ export default { this.dirty = true }, toYaml () { - this.persistenceYaml = YAML.stringify({ + const toCode = { configurations: this.persistence.configs, cronStrategies: this.persistence.cronStrategies, - defaultStrategies: this.persistence.defaults, - thresholdFilters: this.persistence.thresholdFilters, - timeFilters: this.persistence.timeFilters, - equalsFilters: this.persistence.equalsFilters, - includeFilters: this.persistence.includeFilters + defaultStrategies: this.persistence.defaults + } + this.filterTypes.forEach((ft) => { + toCode[ft.name] = this.persistence[ft.name] }) + this.persistenceYaml = YAML.stringify(toCode) }, fromYaml () { if (!this.isEditable) return false @@ -662,10 +671,9 @@ export default { this.$set(this.persistence, 'configs', updatedPersistence.configurations) this.$set(this.persistence, 'cronStrategies', updatedPersistence.cronStrategies) this.$set(this.persistence, 'defaults', updatedPersistence.defaultStrategies) - this.$set(this.persistence, 'thresholdFilters', updatedPersistence.thresholdFilters) - this.$set(this.persistence, 'timeFilters', updatedPersistence.timeFilters) - this.$set(this.persistence, 'equalsFilters', updatedPersistence.equalsFilters) - this.$set(this.persistence, 'includeFilters', updatedPersistence.includeFilters) + this.filterTypes.forEach((ft) => { + this.$set(this.persistence, ft.name, updatedPersistence[ft.name]) + }) return true } catch (e) { this.$f7.dialog.alert(e).open()