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 <florianh_dev@icloud.com>

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: []
       }
     }

bundles/org.openhab.ui/web/src/pages/settings/persistence/cron-strategy-popup.vue

@@ -3,7 +3,7 @@
     <f7-page>
       <f7-navbar>
         <f7-nav-left>
-          <f7-link icon-ios="f7:arrow_left" icon-md="material:arrow_back" icon-aurora="f7:arrow_left" popup-close />
+          <f7-link icon-ios="f7:arrow_left" icon-md="material:arrow_back" icon-aurora="f7:arrow_left" popup-close/>
         </f7-nav-left>
         <f7-nav-title>
           Configure cron strategy
@@ -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" />
           </f7-list>
         </f7-col>
         <f7-col>
@@ -31,7 +31,7 @@
           <f7-list>
             <parameter-cronexpression ref="cronExpression" :configDescription="cronExpressionConfigDescription"
                                       :value="currentCronStrategy.cronExpression"
-                                      @input="currentCronStrategy.cronExpression = $event" />
+                                      @input="currentCronStrategy.cronExpression = $event"/>
           </f7-list>
         </f7-col>
       </f7-block>

bundles/org.openhab.ui/web/src/pages/settings/persistence/filter-picker.vue

@@ -1,16 +1,20 @@
 <template>
   <f7-list class="strategy-picker-container" v-if="filters">
-    <f7-list-item title="Select filters" :smart-select="disabled !== true" :smart-select-params="smartSelectParams"
+    <f7-list-item title="Select filters" :smart-select="disabled !== true && filters.length > 0"
+                  :smart-select-params="smartSelectParams"
                   ref="smartSelect" class="defaults-picker">
-      <select v-if="disabled !== true" name="filters" multiple @change="select">
+      <select v-if="disabled !== true && filters.length > 0" name="filters" multiple @change="select">
         <option v-for="s in filters" :key="s" :value="s"
                 :selected="value.includes(s)">
           {{ s }}
         </option>
       </select>
-      <div v-else>
+      <div v-else-if="disabled === true">
         {{ value.join(', ') }}
       </div>
+      <div v-else-if="disabled !== true && filters.length === 0">
+        No filters available. Please add them first.
+      </div>
     </f7-list-item>
   </f7-list>
 </template>

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" />
           </f7-list>
         </f7-col>
         <f7-col>
@@ -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()
     }

bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue

@@ -162,9 +162,9 @@
               </div>
             </div>
           </f7-col>
-          <f7-col>
+          <f7-col v-if="isEditable && !newPersistence">
             <f7-list>
-              <f7-list-button v-if="isEditable" color="red" @click="deletePersistence">
+              <f7-list-button color="red" @click="deletePersistence">
                 Remove persistence configuration
               </f7-list-button>
             </f7-list>
@@ -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,15 +326,15 @@ 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,
               type: 'DECIMAL'
             },
             {
               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()