From 93dce567dca7e0d3b0d814e8d6bcd52c3aa5073d Mon Sep 17 00:00:00 2001
From: Florian Hotze <florianh_dev@icloud.com>
Date: Mon, 5 Feb 2024 21:15:09 +0100
Subject: [PATCH] Fix UoM metadata not stored when Item created from link
 (#2302)

Fixes #2299.

Also enhances the Item name validation when creating a new Item from the
Items list.

---------

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
---
 .../web/src/components/item/group-form.vue    |  2 +-
 .../web/src/components/item/item-form.vue     | 17 ++-------
 .../web/src/components/item/item-mixin.js     | 38 ++++++++++++++-----
 .../web/src/components/model/item-details.vue |  4 +-
 .../src/pages/settings/items/item-edit.vue    |  4 +-
 .../pages/settings/things/link/link-add.vue   | 23 +++++------
 6 files changed, 48 insertions(+), 40 deletions(-)

diff --git a/bundles/org.openhab.ui/web/src/components/item/group-form.vue b/bundles/org.openhab.ui/web/src/components/item/group-form.vue
index 2cf78264d6..b3593a94d7 100644
--- a/bundles/org.openhab.ui/web/src/components/item/group-form.vue
+++ b/bundles/org.openhab.ui/web/src/components/item/group-form.vue
@@ -20,7 +20,7 @@
                      info="Used internally, for persistence and external systems. It is independent from the state visualization in the UI, which is defined through the state description."
                      @input="item.unit = $event.target.value" clear-button />
       <f7-list-input v-if="item.type && item.type.startsWith('Number:') && createMode" label="State Description Pattern" type="text" :value="item.stateDescriptionPattern"
-                     info="Pattern or transformation applied to the state for display purposes."
+                     info="Pattern or transformation applied to the state for display purposes. Only saved if you change the pre-filled default value."
                      @input="item.stateDescriptionPattern = $event.target.value" clear-button />
       <f7-list-item key="function-picker-arithmetic" v-if="item.type === 'Group' && item.groupType && (['Dimmer', 'Rollershutter'].indexOf(item.groupType) >= 0 || item.groupType.indexOf('Number') === 0)" title="Aggregation Function" smart-select :smart-select-params="{openIn: 'popover', closeOnSelect: true}">
         <select name="select-function" @change="setFunction($event.target.value)">
diff --git a/bundles/org.openhab.ui/web/src/components/item/item-form.vue b/bundles/org.openhab.ui/web/src/components/item/item-form.vue
index 078a1308db..9bfe4568be 100644
--- a/bundles/org.openhab.ui/web/src/components/item/item-form.vue
+++ b/bundles/org.openhab.ui/web/src/components/item/item-form.vue
@@ -27,7 +27,7 @@
                      info="Used internally, for persistence and external systems. It is independent from the state visualization in the UI, which is defined through the state description."
                      @input="item.unit = $event.target.value" clear-button />
       <f7-list-input v-show="!hideType && item.type && item.type.startsWith('Number:') && createMode" label="State Description Pattern" type="text" :value="item.stateDescriptionPattern"
-                     info="Pattern or transformation applied to the state for display purposes."
+                     info="Pattern or transformation applied to the state for display purposes. Only saved if you change the pre-filled default value."
                      @input="item.stateDescriptionPattern = $event.target.value" clear-button />
       <f7-list-input v-if="!hideCategory" ref="category" label="Category" autocomplete="off" type="text" placeholder="temperature, firstfloor..." :value="item.category"
                      @input="item.category = $event.target.value" clear-button>
@@ -118,18 +118,7 @@ export default {
     },
     onNameInput (event) {
       this.item.name = event.target.value
-      this.validateName(this.item.name)
-    },
-    validateName (name) {
-      let oldError = this.nameErrorMessage
-      if (!/^[A-Za-z0-9_]+$/.test(name)) {
-        this.nameErrorMessage = 'Required. A-Z,a-z,0-9,_ only'
-      } else if (this.items.some(item => item.name === name)) {
-        this.nameErrorMessage = 'An Item with this name already exists'
-      } else {
-        this.nameErrorMessage = ''
-      }
-      if (oldError !== this.nameErrorMessage) this.$emit('valid', !this.nameErrorMessage)
+      this.$set(this, 'nameErrorMessage', this.validateItemName(this.item.name))
     }
   },
   mounted () {
@@ -137,7 +126,7 @@ export default {
     if (!this.item.category) this.$set(this.item, 'category', '')
     if (this.createMode) {
       if (!this.items) this.items = []
-      this.validateName(this.item.name)
+      this.$set(this, 'nameErrorMessage', this.validateItemName(this.item.name))
     }
     const categoryControl = this.$refs.category
     if (!categoryControl || !categoryControl.$el) return
diff --git a/bundles/org.openhab.ui/web/src/components/item/item-mixin.js b/bundles/org.openhab.ui/web/src/components/item/item-mixin.js
index 09e710ca84..12283832ef 100644
--- a/bundles/org.openhab.ui/web/src/components/item/item-mixin.js
+++ b/bundles/org.openhab.ui/web/src/components/item/item-mixin.js
@@ -28,9 +28,29 @@ export default {
       if (!item.tags) return []
       return item.tags.filter((t) => !this.isSemanticTag(t))
     },
-    doSave (item) {
-      console.log(item)
-
+    /**
+     * Validate the Item name against valid characters and (if existing Items are available on `this.items`) names of existing Items.
+     *
+     * @param {string} name Item name to validate
+     * @returns {string} The error message if the name is invalid, or an empty string if the name is valid.
+     */
+    validateItemName (name) {
+      if (!/^[A-Za-z0-9_]+$/.test(name)) {
+        return 'Required. A-Z,a-z,0-9,_ only'
+      } else if (this.items && this.items.some(item => item.name === name)) {
+        return 'An Item with this name already exists'
+      }
+      return ''
+    },
+    /**
+     * Save an Item, i.e. add a new Item or update an existing Item.
+     *
+     * If a new Item is created (checks `this.createMode`), and it is an UoM Item, unit metadata and state description (if changed from the default) metadata are saved as well.
+     *
+     * @param item
+     * @returns {Promise}
+     */
+    saveItem (item) {
       if (item.groupType === 'None') delete item.groupType
       if (item.function === 'None') delete item.groupType
 
@@ -41,17 +61,17 @@ export default {
 
       // TODO: Add support for saving metadata
       return this.$oh.api.put('/rest/items/' + item.name, item).then(() => {
-        let unitPromise = Promise.resolve()
+        // Save unit metadata if Item is an UoM Item
         if (this.createMode && (item.type.startsWith('Number:') || item.groupType?.startsWith('Number:')) && unit) {
           const metadata = {
             value: unit,
             config: {}
           }
-          unitPromise = this.$oh.api.put('/rest/items/' + item.name + '/metadata/unit', metadata)
+          return this.$oh.api.put('/rest/items/' + item.name + '/metadata/unit', metadata)
         }
-        return unitPromise
+        return Promise.resolve()
       }).then(() => {
-        let stateDescriptionPromise = Promise.resolve()
+        // Save state description if Item is an UoM Item and if state description changed from the default value
         if (this.createMode && (item.type.startsWith('Number:') || item.groupType?.startsWith('Number:')) && stateDescriptionPattern) {
           if (stateDescriptionPattern !== `%.0f ${unit}`) {
             const metadata = {
@@ -60,11 +80,9 @@ export default {
                 pattern: stateDescriptionPattern
               }
             }
-            stateDescriptionPromise = this.$oh.api.put('/rest/items/' + item.name + '/metadata/stateDescription', metadata)
+            return this.$oh.api.put('/rest/items/' + item.name + '/metadata/stateDescription', metadata)
           }
         }
-        return stateDescriptionPromise
-      }).then(() => {
         return Promise.resolve()
       }).catch((err) => {
         return Promise.reject(err)
diff --git a/bundles/org.openhab.ui/web/src/components/model/item-details.vue b/bundles/org.openhab.ui/web/src/components/model/item-details.vue
index e45e3e5cd7..ac14072121 100644
--- a/bundles/org.openhab.ui/web/src/components/model/item-details.vue
+++ b/bundles/org.openhab.ui/web/src/components/model/item-details.vue
@@ -100,7 +100,7 @@ export default {
     },
     save () {
       this.editMode = false
-      this.doSave(this.editedItem).then(() => {
+      this.saveItem(this.editedItem).then(() => {
         this.$f7.toast.create({
           text: 'Item updated',
           destroyOnClose: true,
@@ -121,7 +121,7 @@ export default {
       // TODO properly validate item
       if (!this.editedItem.name) return
 
-      this.doSave(this.editedItem).then(() => {
+      this.saveItem(this.editedItem).then(() => {
         this.$f7.toast.create({
           text: 'Item created',
           destroyOnClose: true,
diff --git a/bundles/org.openhab.ui/web/src/pages/settings/items/item-edit.vue b/bundles/org.openhab.ui/web/src/pages/settings/items/item-edit.vue
index bd61aaceec..98382174b9 100644
--- a/bundles/org.openhab.ui/web/src/pages/settings/items/item-edit.vue
+++ b/bundles/org.openhab.ui/web/src/pages/settings/items/item-edit.vue
@@ -160,10 +160,10 @@ export default {
       if (this.currentTab === 'code') {
         if (!this.fromYaml()) return Promise.reject()
       }
-      if (!this.item.name) return this.$f7.dialog.alert('Please give the Item a valid name').open() // user cannot change name
+      if (this.validateItemName(this.item.name) !== '') return this.$f7.dialog.alert('Please give the Item a valid name: ' + this.validateItemName(this.item.name)).open()
       if (!this.item.type || !this.types.ItemTypes.includes(this.item.type.split(':')[0])) return this.$f7.dialog.alert('Please give Item a valid type').open()
 
-      this.doSave(this.item).then(() => {
+      this.saveItem(this.item).then(() => {
         if (this.createMode) {
           this.$f7.toast.create({
             text: 'Item created',
diff --git a/bundles/org.openhab.ui/web/src/pages/settings/things/link/link-add.vue b/bundles/org.openhab.ui/web/src/pages/settings/things/link/link-add.vue
index a98a435981..05c26f9a14 100644
--- a/bundles/org.openhab.ui/web/src/pages/settings/things/link/link-add.vue
+++ b/bundles/org.openhab.ui/web/src/pages/settings/things/link/link-add.vue
@@ -28,13 +28,13 @@
         <f7-col>
           <f7-block-title>Item</f7-block-title>
           <f7-list media-list>
-            <f7-list-item radio :checked="!createItem" value="false" @change="createItem = false" title="Use an existing Item" name="item-creation-choice" />
-            <f7-list-item radio :checked="createItem" value="true" @change="createItem = true" title="Create a new Item" name="item-creation-choice" />
+            <f7-list-item radio :checked="!createMode" value="false" @change="createMode = false" title="Use an existing Item" name="item-creation-choice" />
+            <f7-list-item radio :checked="createMode" value="true" @change="createMode = true" title="Create a new Item" name="item-creation-choice" />
           </f7-list>
         </f7-col>
 
         <!-- Choose item to link -->
-        <f7-col v-if="!createItem">
+        <f7-col v-if="!createMode">
           <f7-list>
             <item-picker key="itemLink" title="Item to Link" name="item" :value="selectedItemName" :multiple="false" :items="items" :filterType="getCompatibleItemTypes()"
                          @input="(value) => selectedItemName = value" />
@@ -43,7 +43,7 @@
 
         <!-- Create new item -->
         <f7-col v-else>
-          <item-form :item="newItem" :items="items" :createMode="true" @valid="itemValid = $event" />
+          <item-form :item="newItem" :items="items" :createMode="true" />
           <f7-list>
             <item-picker key="newItem-groups" title="Parent Group(s)" name="parent-groups" :value="newItem.groupNames" :items="items" @input="(value) => newItem.groupNames = value" :multiple="true" filterType="Group" />
           </f7-list>
@@ -133,8 +133,10 @@ import ItemForm from '@/components/item/item-form.vue'
 import Item from '@/components/item/item.vue'
 
 import * as Types from '@/assets/item-types.js'
+import ItemMixin from '@/components/item/item-mixin'
 
 export default {
+  mixins: [ItemMixin],
   components: {
     ConfigSheet,
     ItemPicker,
@@ -147,9 +149,8 @@ export default {
   data () {
     return {
       ready: true,
-      createItem: false,
+      createMode: false,
       items: null,
-      itemValid: true,
       link: {
         itemName: null,
         channelUID: null,
@@ -178,7 +179,7 @@ export default {
   },
   computed: {
     currentItem () {
-      return this.item ? this.item : this.createItem ? this.newItem : this.items ? this.items.find(item => item.name === this.selectedItemName) : null
+      return this.item ? this.item : this.createMode ? this.newItem : this.items ? this.items.find(item => item.name === this.selectedItemName) : null
     },
     compatibleProfileTypes () {
       let currentItemType = this.currentItem && this.currentItem.type ? this.currentItem.type : ''
@@ -259,8 +260,8 @@ export default {
       }
 
       // checks
-      if (this.createItem && !this.itemValid) {
-        this.$f7.dialog.alert('Please correct the item to link')
+      if (this.createMode && this.validateItemName(this.newItem.name) !== '') {
+        this.$f7.dialog.alert('Please correct the newly created item')
         return
       }
       if (!link.itemName) {
@@ -287,8 +288,8 @@ export default {
         }
       }
 
-      if (this.createItem) {
-        this.$oh.api.put('/rest/items/' + this.newItem.name, this.newItem).then((data) => {
+      if (this.createMode) {
+        this.saveItem(this.newItem).then((data) => {
           this.$oh.api.put('/rest/links/' + link.itemName + '/' + encodeURIComponent(link.channelUID), link).then((data) => {
             this.$f7.toast.create({
               text: 'Item and link created',