From d536ea2ed517fd70da93713f991b8ef8a97a52e0 Mon Sep 17 00:00:00 2001 From: "Ethan.Z" Date: Tue, 10 Mar 2026 16:17:02 -0400 Subject: [PATCH 1/7] disable toggling if cluster's featureMap attribute is external --- src-electron/rest/user-data.js | 4 +- .../validation/conformance-checker.js | 18 +++++-- src/util/feature-mixin.js | 2 + test/feature.test.js | 47 +++++++++++++++++++ 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src-electron/rest/user-data.js b/src-electron/rest/user-data.js index ea140ccbcb..7195bfda0f 100644 --- a/src-electron/rest/user-data.js +++ b/src-electron/rest/user-data.js @@ -115,6 +115,7 @@ function httpPostCheckConformOnFeatureUpdate(db) { clusterFeatures, endpointId, endpointTypeId, + featureMapStorageOption, changeConfirmed } = request.body @@ -135,7 +136,8 @@ function httpPostCheckConformOnFeatureUpdate(db) { featureMap, featureData, endpointId, - clusterFeatures + clusterFeatures, + featureMapStorageOption ) if (changeConfirmed || result.disableChange) { diff --git a/src-electron/validation/conformance-checker.js b/src-electron/validation/conformance-checker.js index d60f7b0ef9..1aa01699da 100644 --- a/src-electron/validation/conformance-checker.js +++ b/src-electron/validation/conformance-checker.js @@ -50,7 +50,8 @@ function generateWarningMessage( elementMap = {}, descElements = {}, featuresToUpdate = {}, - changedConformFeatures = [] + changedConformFeatures = [], + featureMapStorageOption = null ) { // feature change is disabled by default before the checks let result = { @@ -72,6 +73,15 @@ function generateWarningMessage( let updateDisabledString = `cannot be ${added ? 'enabled' : 'disabled'} as` + // Check 0: if the featureMap attribute storage is external, ZAP cannot modify it + if (featureMapStorageOption === dbEnum.storageOption.external) { + result.warningMessage.push( + warningPrefix + + ` ${updateDisabledString} the featureMap attribute in the cluster is external and ZAP does not have control over it.` + ) + return result + } + // Check 1: if any operands in the feature conformance are missing from elementMap let missingOperands = [] if (Object.keys(elementMap).length > 0 && featureData.conformance) { @@ -244,7 +254,8 @@ function checkElementConformance( featureMap, featureData = null, endpointId = null, - clusterFeatures = null + clusterFeatures = null, + featureMapStorageOption = null ) { let { attributes, commands, events } = elements let featureCode = featureData ? featureData.code : '' @@ -299,7 +310,8 @@ function checkElementConformance( elementMap, descElements, featuresToUpdate, - changedConformFeatures + changedConformFeatures, + featureMapStorageOption ) if (warningInfo.disableChange) { diff --git a/src/util/feature-mixin.js b/src/util/feature-mixin.js index c1f0b6f555..742657afe4 100644 --- a/src/util/feature-mixin.js +++ b/src/util/feature-mixin.js @@ -117,6 +117,7 @@ export default { clusterFeatures: this.clusterFeatures, endpointId: this.endpointId[this.selectedEndpointId], endpointTypeId: this.selectedEndpointTypeId, + featureMapStorageOption: this.featureMapAttribute?.storageOption, changeConfirmed: false }).then((res) => { // store backend response and frontend data for reuse if updates are confirmed @@ -221,6 +222,7 @@ export default { clusterFeatures: this.clusterFeatures, endpointId: this.endpointId[this.selectedEndpointId], endpointTypeId: this.selectedEndpointTypeId, + featureMapStorageOption: this.featureMapAttribute?.storageOption, changeConfirmed: true }) if (this.displayWarning) { diff --git a/test/feature.test.js b/test/feature.test.js index 7cd946998e..93b86bea9a 100644 --- a/test/feature.test.js +++ b/test/feature.test.js @@ -763,6 +763,53 @@ test( expect(result.attributesToUpdate.length).toBe(0) expect(result.commandsToUpdate.length).toBe(0) expect(result.eventsToUpdate.length).toBe(0) + featureMap['UNKNOWN'] = 0 + + // 6. test toggle a feature when featureMap attribute storage is External + // should disable the toggle and show a warning, regardless of conformance + featureMap['HS'] = 1 + result = conformChecker.checkElementConformance( + elements, + featureMap, + featureHS, + endpointId, + clusterFeatures, + dbEnum.storageOption.external + ) + expectedWarning = + warningPrefix + + `feature: ${featureHS.name} (${featureHS.code}) ${featureBitMessage} cannot be enabled as ` + + `the featureMap attribute in the cluster is external and ZAP does not have control over it.` + expect(result.displayWarning).toBeTruthy() + expect(result.disableChange).toBeTruthy() + expect(result.warningMessage[0]).toBe(expectedWarning) + expect(result.attributesToUpdate.length).toBe(0) + expect(result.commandsToUpdate.length).toBe(0) + expect(result.eventsToUpdate.length).toBe(0) + + // 7. test toggle the same feature when featureMap attribute storage is RAM + // should behave identically to no storageOption passed — toggle is allowed + result = conformChecker.checkElementConformance( + elements, + featureMap, + featureHS, + endpointId, + clusterFeatures, + dbEnum.storageOption.ram + ) + expect(result.displayWarning).toBeFalsy() + expect(result.disableChange).toBeFalsy() + expect(result.warningMessage).toBe('') + expect(result.attributesToUpdate.length).toBe(1) + expect(result.attributesToUpdate[0].name).toBe('CurrentHue') + expect(result.attributesToUpdate[0].value).toBeTruthy() + expect(result.commandsToUpdate.length).toBe(1) + expect(result.commandsToUpdate[0].name).toBe('MoveToHue') + expect(result.commandsToUpdate[0].value).toBeTruthy() + expect(result.eventsToUpdate.length).toBe(1) + expect(result.eventsToUpdate[0].name).toBe('event1') + expect(result.eventsToUpdate[0].value).toBeTruthy() + featureMap['HS'] = 0 }, testUtil.timeout.short() ) From 460c15358a91f2904710d8252938563d8d6e869b Mon Sep 17 00:00:00 2001 From: "Ethan.Z" Date: Thu, 12 Mar 2026 11:30:49 -0400 Subject: [PATCH 2/7] fix comments, remove html in Notify as all messages are plain text to remove security issue --- src/util/feature-mixin.js | 3 +-- test/feature.test.js | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/util/feature-mixin.js b/src/util/feature-mixin.js index 742657afe4..e27269aa99 100644 --- a/src/util/feature-mixin.js +++ b/src/util/feature-mixin.js @@ -295,8 +295,7 @@ export default { message: warning, type: 'warning', classes: 'custom-notification notification-warning', - position: 'top', - html: true + position: 'top' }) } }, diff --git a/test/feature.test.js b/test/feature.test.js index 93b86bea9a..05d4b24b6e 100644 --- a/test/feature.test.js +++ b/test/feature.test.js @@ -776,10 +776,7 @@ test( clusterFeatures, dbEnum.storageOption.external ) - expectedWarning = - warningPrefix + - `feature: ${featureHS.name} (${featureHS.code}) ${featureBitMessage} cannot be enabled as ` + - `the featureMap attribute in the cluster is external and ZAP does not have control over it.` + expectedWarning = `${warningPrefix}feature: ${featureHS.name} (${featureHS.code}) ${featureBitMessage} cannot be enabled as the featureMap attribute in the cluster is external and ZAP does not have control over it.` expect(result.displayWarning).toBeTruthy() expect(result.disableChange).toBeTruthy() expect(result.warningMessage[0]).toBe(expectedWarning) From 7be7e5c94b914b084a37cfaaa3878e80f6910efc Mon Sep 17 00:00:00 2001 From: "Ethan.Z" Date: Thu, 12 Mar 2026 16:04:46 -0400 Subject: [PATCH 3/7] show warning when external attributes depend on the toggled feature --- src-electron/db/query-attribute.js | 3 +- src-electron/db/query-session-notification.js | 10 ++-- .../validation/conformance-checker.js | 53 +++++++++++++------ src/util/feature-mixin.js | 7 +++ test/feature.test.js | 34 ++++++++++-- 5 files changed, 83 insertions(+), 24 deletions(-) diff --git a/src-electron/db/query-attribute.js b/src-electron/db/query-attribute.js index 2638e6289f..029179c032 100644 --- a/src-electron/db/query-attribute.js +++ b/src-electron/db/query-attribute.js @@ -1301,7 +1301,8 @@ async function selectAttributesByEndpointTypeClusterId( ATTRIBUTE.REPORT_MIN_INTERVAL, ATTRIBUTE.REPORT_MAX_INTERVAL, ATTRIBUTE.REPORTABLE_CHANGE, - ENDPOINT_TYPE_ATTRIBUTE.INCLUDED + ENDPOINT_TYPE_ATTRIBUTE.INCLUDED, + ENDPOINT_TYPE_ATTRIBUTE.STORAGE_OPTION FROM ATTRIBUTE JOIN diff --git a/src-electron/db/query-session-notification.js b/src-electron/db/query-session-notification.js index 0cceb85a9c..ddf511b4c0 100644 --- a/src-electron/db/query-session-notification.js +++ b/src-electron/db/query-session-notification.js @@ -212,10 +212,12 @@ async function setNotificationOnFeatureChange(db, sessionId, result) { } } else { await deleteNotificationWithPatterns(db, sessionId, outdatedWarningPatterns) - if (displayWarning) { - await setNotification(db, 'WARNING', warningMessage, sessionId, 2, 0) - } else { - await searchNotificationByMessageAndDelete(db, sessionId, warningMessage) + for (const message of warningMessage) { + if (displayWarning) { + await setNotification(db, 'WARNING', message, sessionId, 2, 0) + } else { + await searchNotificationByMessageAndDelete(db, sessionId, message) + } } } } diff --git a/src-electron/validation/conformance-checker.js b/src-electron/validation/conformance-checker.js index 1aa01699da..a65bdb560c 100644 --- a/src-electron/validation/conformance-checker.js +++ b/src-electron/validation/conformance-checker.js @@ -51,7 +51,8 @@ function generateWarningMessage( descElements = {}, featuresToUpdate = {}, changedConformFeatures = [], - featureMapStorageOption = null + featureMapStorageOption = null, + attributes = [] ) { // feature change is disabled by default before the checks let result = { @@ -177,27 +178,29 @@ function generateWarningMessage( let buildNonElementConformMessage = (state, conformance) => ` should be ${state}, as it is ${conformance} ${deviceTypeString}.` - // in this case only 1 warning message is needed - result.warningMessage = '' + result.warningMessage = [] if (conformance == 'notSupported') { - result.warningMessage = + result.warningMessage.push( warningPrefix + - (combinedOperands - ? buildElementConformMessage('disabled') - : buildNonElementConformMessage('disabled', 'not supported')) + (combinedOperands + ? buildElementConformMessage('disabled') + : buildNonElementConformMessage('disabled', 'not supported')) + ) result.displayWarning = added } if (conformance == 'provisional') { - result.warningMessage = + result.warningMessage.push( warningPrefix + ' is enabled, but it is still provisional.' + ) result.displayWarning = added } if (conformance == 'mandatory') { - result.warningMessage = + result.warningMessage.push( warningPrefix + - (combinedOperands - ? buildElementConformMessage('enabled') - : buildNonElementConformMessage('enabled', 'mandatory')) + (combinedOperands + ? buildElementConformMessage('enabled') + : buildNonElementConformMessage('enabled', 'mandatory')) + ) result.displayWarning = !added } // if the feature conformance contains the operand 'desc', do not disable toggling, but show warning message @@ -206,9 +209,10 @@ function generateWarningMessage( dbEnum.conformanceTag.described ) if (featureContainsDesc) { - result.warningMessage = + result.warningMessage.push( warningPrefix + - ` is being ${added ? 'enabled' : 'disabled'}, but it has descriptive conformance and requires manual validation from the feature specification to enable/disable the right dependencies in ZAP.` + ` is being ${added ? 'enabled' : 'disabled'}, but it has descriptive conformance and requires manual validation from the feature specification to enable/disable the right dependencies in ZAP.` + ) result.displayWarning = true } @@ -218,6 +222,24 @@ function generateWarningMessage( let prefix = buildWarningPrefix(feature) return getOutdatedWarningPatterns(prefix) }) + + // Check 4: if any attributes to be updated have external storage, + // ZAP cannot write their values — warn the user without disabling the toggle. + let externalAttributes = filterElementsToUpdate( + attributes, + elementMap, + featureData.code + ).filter((attr) => attr.storageOption === dbEnum.storageOption.external) + if (externalAttributes.length > 0) { + let externalAttrNames = externalAttributes.map((a) => a.name).join(', ') + let attrPlural = + externalAttributes.length === 1 ? 'attribute' : 'attributes' + result.warningMessage.push( + warningPrefix + + ` ${attrPlural} ${externalAttrNames} ${externalAttributes.length === 1 ? 'has' : 'have'} external storage. ZAP is unable to manage ${externalAttributes.length === 1 ? 'its' : 'their'} value automatically.` + ) + result.displayWarning = true + } } return result @@ -311,7 +333,8 @@ function checkElementConformance( descElements, featuresToUpdate, changedConformFeatures, - featureMapStorageOption + featureMapStorageOption, + attributes ) if (warningInfo.disableChange) { diff --git a/src/util/feature-mixin.js b/src/util/feature-mixin.js index e27269aa99..71bf217a11 100644 --- a/src/util/feature-mixin.js +++ b/src/util/feature-mixin.js @@ -159,6 +159,13 @@ export default { // update attributes, commands, and events for the toggle feature, and set notifications this.attributesToUpdate.forEach((attribute) => { + // External attributes are shown in the dialog but ZAP cannot control them + // so skip dispatching an update so their DB state is preserved. + // This ensures the backend still generates and saves the external-attribute + // warning when the confirm POST arrives. + if (attribute.storageOption === dbEnum.storageOption.external) { + return + } let editContext = { action: 'boolean', endpointTypeIdList: this.endpointTypeIdList, diff --git a/test/feature.test.js b/test/feature.test.js index 05d4b24b6e..79e50ce130 100644 --- a/test/feature.test.js +++ b/test/feature.test.js @@ -651,7 +651,7 @@ test( // no warnings should be generated expect(result.displayWarning).toBeFalsy() expect(result.disableChange).toBeFalsy() - expect(result.warningMessage).toBe('') + expect(result.warningMessage).toEqual([]) // elements with conformance 'HS' should be enabled // and their values should be set to true expect(result.attributesToUpdate.length).toBe(1) @@ -681,7 +681,7 @@ test( `as it is mandatory for device type: ${deviceType}.` expect(result.displayWarning).toBeTruthy() expect(result.disableChange).toBeFalsy() - expect(result.warningMessage).toBe(expectedWarning) + expect(result.warningMessage).toEqual([expectedWarning]) // attributes and commands with conformance 'XY' should be disabled // and their values should be set to false expect(result.attributesToUpdate.length).toBe(1) @@ -796,7 +796,7 @@ test( ) expect(result.displayWarning).toBeFalsy() expect(result.disableChange).toBeFalsy() - expect(result.warningMessage).toBe('') + expect(result.warningMessage).toEqual([]) expect(result.attributesToUpdate.length).toBe(1) expect(result.attributesToUpdate[0].name).toBe('CurrentHue') expect(result.attributesToUpdate[0].value).toBeTruthy() @@ -807,6 +807,32 @@ test( expect(result.eventsToUpdate[0].name).toBe('event1') expect(result.eventsToUpdate[0].value).toBeTruthy() featureMap['HS'] = 0 + + // 8. test enable a feature when a dependent attribute has external storage + // should display warning and change is still allowed + elements.attributes.find( + (attr) => attr.name === 'CurrentHue' + ).storageOption = dbEnum.storageOption.external + featureMap['HS'] = 1 + result = conformChecker.checkElementConformance( + elements, + featureMap, + featureHS, + endpointId, + clusterFeatures + ) + expectedWarning = + warningPrefix + + `feature: ${featureHS.name} (${featureHS.code}) ${featureBitMessage}` + + ` attribute CurrentHue has external storage. ZAP is unable to manage its value automatically.` + expect(result.displayWarning).toBeTruthy() + expect(result.disableChange).toBeFalsy() + expect(result.warningMessage).toContain(expectedWarning) + // clean up + elements.attributes.find( + (attr) => attr.name === 'CurrentHue' + ).storageOption = undefined + featureMap['HS'] = 0 }, testUtil.timeout.short() ) @@ -856,7 +882,7 @@ test( expect(result.displayWarning).toBeTruthy() expect(result.disableChange).toBeFalsy() - expect(result.warningMessage).toBe(expectedWarning) + expect(result.warningMessage).toEqual([expectedWarning]) }, testUtil.timeout.short() ) From f0307b0e0a92c4ca4009afe6154e26c02cf0f421 Mon Sep 17 00:00:00 2001 From: "Ethan.Z" Date: Thu, 12 Mar 2026 16:28:15 -0400 Subject: [PATCH 4/7] imporve warning message --- src-electron/validation/conformance-checker.js | 13 +++++++++---- test/feature.test.js | 9 ++++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src-electron/validation/conformance-checker.js b/src-electron/validation/conformance-checker.js index a65bdb560c..ad1e49aa40 100644 --- a/src-electron/validation/conformance-checker.js +++ b/src-electron/validation/conformance-checker.js @@ -232,11 +232,16 @@ function generateWarningMessage( ).filter((attr) => attr.storageOption === dbEnum.storageOption.external) if (externalAttributes.length > 0) { let externalAttrNames = externalAttributes.map((a) => a.name).join(', ') - let attrPlural = - externalAttributes.length === 1 ? 'attribute' : 'attributes' + let isSingular = externalAttributes.length === 1 + let clusterPrefix = env.formatEmojiMessage( + '⚠️', + `Check Feature Compliance on endpoint: ${endpointId}, cluster: ${featureData.cluster},` + ) result.warningMessage.push( - warningPrefix + - ` ${attrPlural} ${externalAttrNames} ${externalAttributes.length === 1 ? 'has' : 'have'} external storage. ZAP is unable to manage ${externalAttributes.length === 1 ? 'its' : 'their'} value automatically.` + clusterPrefix + + ` ${isSingular ? 'attribute' : 'attributes'} ${externalAttrNames},` + + ` required by feature: ${featureData.name} (${featureData.code}),` + + ` ${isSingular ? 'has' : 'have'} external storage and ZAP does not have control over it.` ) result.displayWarning = true } diff --git a/test/feature.test.js b/test/feature.test.js index 79e50ce130..50f8e9a04e 100644 --- a/test/feature.test.js +++ b/test/feature.test.js @@ -821,10 +821,13 @@ test( endpointId, clusterFeatures ) + let clusterPrefix = env.formatEmojiMessage( + '⚠️', + `Check Feature Compliance on endpoint: ${endpointId}, cluster: ${featureHS.cluster},` + ) expectedWarning = - warningPrefix + - `feature: ${featureHS.name} (${featureHS.code}) ${featureBitMessage}` + - ` attribute CurrentHue has external storage. ZAP is unable to manage its value automatically.` + clusterPrefix + + ` attribute CurrentHue, required by feature: ${featureHS.name} (${featureHS.code}), has external storage and ZAP does not have control over it.` expect(result.displayWarning).toBeTruthy() expect(result.disableChange).toBeFalsy() expect(result.warningMessage).toContain(expectedWarning) From dabaa80ab6cde31350473ea99d9573fc38fc44dc Mon Sep 17 00:00:00 2001 From: "Ethan.Z" Date: Thu, 12 Mar 2026 21:36:03 -0400 Subject: [PATCH 5/7] fix unit test --- test/notification.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/notification.test.js b/test/notification.test.js index 173a60f201..2ab5e033c2 100644 --- a/test/notification.test.js +++ b/test/notification.test.js @@ -336,12 +336,12 @@ test( let notisWithMultipleMessages let warningResult = { - warningMessage: warningMessage, + warningMessage: [warningMessage], disableChange: false, displayWarning: true } let deleteWarningResult = { - warningMessage: warningMessage, + warningMessage: [warningMessage], disableChange: false, displayWarning: false } @@ -426,7 +426,7 @@ test( // set other feature warnings let otherResult = { - warningMessage: disableMessage2, + warningMessage: [disableMessage2], disableChange: false, displayWarning: true } @@ -436,21 +436,21 @@ test( otherResult ) - otherResult.warningMessage = messageOfOtherPattern + otherResult.warningMessage = [messageOfOtherPattern] await sessionNotification.setNotificationOnFeatureChange( db, sessionId, otherResult ) - otherResult.warningMessage = messageOfOtherFeature + otherResult.warningMessage = [messageOfOtherFeature] await sessionNotification.setNotificationOnFeatureChange( db, sessionId, otherResult ) - otherResult.warningMessage = messageOfOtherType + otherResult.warningMessage = [messageOfOtherType] await sessionNotification.setNotificationOnFeatureChange( db, sessionId, From ed3e4da563cbffd950fbcf0bc5a387a18db8597e Mon Sep 17 00:00:00 2001 From: "Ethan.Z" Date: Thu, 19 Mar 2026 14:43:54 -0400 Subject: [PATCH 6/7] rename variable to storageOption --- src-electron/rest/user-data.js | 4 ++-- src-electron/validation/conformance-checker.js | 8 ++++---- src/util/feature-mixin.js | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src-electron/rest/user-data.js b/src-electron/rest/user-data.js index 7195bfda0f..468d1572d8 100644 --- a/src-electron/rest/user-data.js +++ b/src-electron/rest/user-data.js @@ -115,7 +115,7 @@ function httpPostCheckConformOnFeatureUpdate(db) { clusterFeatures, endpointId, endpointTypeId, - featureMapStorageOption, + storageOption, changeConfirmed } = request.body @@ -137,7 +137,7 @@ function httpPostCheckConformOnFeatureUpdate(db) { featureData, endpointId, clusterFeatures, - featureMapStorageOption + storageOption ) if (changeConfirmed || result.disableChange) { diff --git a/src-electron/validation/conformance-checker.js b/src-electron/validation/conformance-checker.js index ad1e49aa40..c757730a5a 100644 --- a/src-electron/validation/conformance-checker.js +++ b/src-electron/validation/conformance-checker.js @@ -51,7 +51,7 @@ function generateWarningMessage( descElements = {}, featuresToUpdate = {}, changedConformFeatures = [], - featureMapStorageOption = null, + storageOption = null, attributes = [] ) { // feature change is disabled by default before the checks @@ -75,7 +75,7 @@ function generateWarningMessage( let updateDisabledString = `cannot be ${added ? 'enabled' : 'disabled'} as` // Check 0: if the featureMap attribute storage is external, ZAP cannot modify it - if (featureMapStorageOption === dbEnum.storageOption.external) { + if (storageOption === dbEnum.storageOption.external) { result.warningMessage.push( warningPrefix + ` ${updateDisabledString} the featureMap attribute in the cluster is external and ZAP does not have control over it.` @@ -282,7 +282,7 @@ function checkElementConformance( featureData = null, endpointId = null, clusterFeatures = null, - featureMapStorageOption = null + storageOption = null ) { let { attributes, commands, events } = elements let featureCode = featureData ? featureData.code : '' @@ -338,7 +338,7 @@ function checkElementConformance( descElements, featuresToUpdate, changedConformFeatures, - featureMapStorageOption, + storageOption, attributes ) diff --git a/src/util/feature-mixin.js b/src/util/feature-mixin.js index 71bf217a11..8d9f67eed6 100644 --- a/src/util/feature-mixin.js +++ b/src/util/feature-mixin.js @@ -117,7 +117,7 @@ export default { clusterFeatures: this.clusterFeatures, endpointId: this.endpointId[this.selectedEndpointId], endpointTypeId: this.selectedEndpointTypeId, - featureMapStorageOption: this.featureMapAttribute?.storageOption, + storageOption: this.featureMapAttribute?.storageOption, changeConfirmed: false }).then((res) => { // store backend response and frontend data for reuse if updates are confirmed @@ -229,7 +229,7 @@ export default { clusterFeatures: this.clusterFeatures, endpointId: this.endpointId[this.selectedEndpointId], endpointTypeId: this.selectedEndpointTypeId, - featureMapStorageOption: this.featureMapAttribute?.storageOption, + storageOption: this.featureMapAttribute?.storageOption, changeConfirmed: true }) if (this.displayWarning) { From 870cfb322241eefea3c2e69ae76b667684fed4f3 Mon Sep 17 00:00:00 2001 From: "Ethan.Z" Date: Tue, 31 Mar 2026 17:32:15 -0400 Subject: [PATCH 7/7] show warning if attribute toggled to External, and delete warnings from both attribute and feature toggle if switch away from External --- docs/api.md | 40 +++++++++ src-electron/db/query-endpoint.js | 33 +++++++ src-electron/db/query-session-notification.js | 76 ++++++++++++++++ src-electron/rest/user-data.js | 69 +++++++++++++++ .../validation/conformance-checker.js | 2 +- src-shared/db-enum.js | 6 ++ test/feature.test.js | 2 +- test/notification.test.js | 88 +++++++++++++++++++ 8 files changed, 314 insertions(+), 2 deletions(-) diff --git a/docs/api.md b/docs/api.md index c43c50e0d2..8075e28ef3 100644 --- a/docs/api.md +++ b/docs/api.md @@ -365,6 +365,12 @@ This module provides common external URLs. ## DB API: DB types and enums. This module provides mappings between database columns and JS keys. + + +### DB API: DB types and enums..warnings +Text fragments shared by warning messages + +**Kind**: static property of [DB API: DB types and enums.](#module_DB API_ DB types and enums.) ## Renderer API: Renderer API. @@ -15247,6 +15253,7 @@ This module provides the API to access zcl specific information. * [~httpPostSaveSessionKeyValue(db)](#module_REST API_ user data..httpPostSaveSessionKeyValue) ⇒ * [~httpPostCluster(db)](#module_REST API_ user data..httpPostCluster) ⇒ * [~httpPostForcedExternal(db)](#module_REST API_ user data..httpPostForcedExternal) ⇒ function + * [~handleAttributeStorageNotification(db, sessionId, endpointTypeId, clusterRef, attributeId, storageValue)](#module_REST API_ user data..handleAttributeStorageNotification) * [~httpPostAttributeUpdate(db)](#module_REST API_ user data..httpPostAttributeUpdate) ⇒ * [~httpPostCommandUpdate(db)](#module_REST API_ user data..httpPostCommandUpdate) ⇒ * [~httpPostEventUpdate(db)](#module_REST API_ user data..httpPostEventUpdate) ⇒ @@ -15483,6 +15490,22 @@ options for the identified package. The results are sent back to the client as a | --- | --- | --- | | db | Object | The database connection object. | + + +### REST API: user data~handleAttributeStorageNotification(db, sessionId, endpointTypeId, clusterRef, attributeId, storageValue) +Set or delete external storage warnings when the user changes attribute storage. + +**Kind**: inner method of [REST API: user data](#module_REST API_ user data) + +| Param | Type | +| --- | --- | +| db | \* | +| sessionId | \* | +| endpointTypeId | \* | +| clusterRef | \* | +| attributeId | \* | +| storageValue | \* | + ### REST API: user data~httpPostAttributeUpdate(db) ⇒ @@ -16599,6 +16622,7 @@ This module provides the REST API to the user specific data. * [~httpPostSaveSessionKeyValue(db)](#module_REST API_ user data..httpPostSaveSessionKeyValue) ⇒ * [~httpPostCluster(db)](#module_REST API_ user data..httpPostCluster) ⇒ * [~httpPostForcedExternal(db)](#module_REST API_ user data..httpPostForcedExternal) ⇒ function + * [~handleAttributeStorageNotification(db, sessionId, endpointTypeId, clusterRef, attributeId, storageValue)](#module_REST API_ user data..handleAttributeStorageNotification) * [~httpPostAttributeUpdate(db)](#module_REST API_ user data..httpPostAttributeUpdate) ⇒ * [~httpPostCommandUpdate(db)](#module_REST API_ user data..httpPostCommandUpdate) ⇒ * [~httpPostEventUpdate(db)](#module_REST API_ user data..httpPostEventUpdate) ⇒ @@ -16835,6 +16859,22 @@ options for the identified package. The results are sent back to the client as a | --- | --- | --- | | db | Object | The database connection object. | + + +### REST API: user data~handleAttributeStorageNotification(db, sessionId, endpointTypeId, clusterRef, attributeId, storageValue) +Set or delete external storage warnings when the user changes attribute storage. + +**Kind**: inner method of [REST API: user data](#module_REST API_ user data) + +| Param | Type | +| --- | --- | +| db | \* | +| sessionId | \* | +| endpointTypeId | \* | +| clusterRef | \* | +| attributeId | \* | +| storageValue | \* | + ### REST API: user data~httpPostAttributeUpdate(db) ⇒ diff --git a/src-electron/db/query-endpoint.js b/src-electron/db/query-endpoint.js index 119638edf3..32c85b9200 100644 --- a/src-electron/db/query-endpoint.js +++ b/src-electron/db/query-endpoint.js @@ -491,6 +491,37 @@ async function deleteEndpoint(db, id) { return dbApi.dbRemove(db, 'DELETE FROM ENDPOINT WHERE ENDPOINT_ID = ?', [id]) } +/** + * Retrieves endpoint identifiers for a session and endpoint type. + * + * @param {*} db + * @param {*} sessionId + * @param {*} endpointTypeId + * @returns Promise that resolves into an array of endpoint identifiers. + */ +async function selectEndpointIdentifiersBySessionAndEndpointType( + db, + sessionId, + endpointTypeId +) { + let rows = await dbApi.dbAll( + db, + ` +SELECT + ENDPOINT_IDENTIFIER +FROM + ENDPOINT +WHERE + SESSION_REF = ? + AND ENDPOINT_TYPE_REF = ? +ORDER BY + ENDPOINT_IDENTIFIER +`, + [sessionId, endpointTypeId] + ) + return rows.map((r) => r.ENDPOINT_IDENTIFIER) +} + /** * Returns ENDPOINT_ID of the Endpoint's Parent Endpoint * @@ -667,3 +698,5 @@ exports.getParentEndpointIdentifier = getParentEndpointIdentifier exports.selectAllEndpointsBasedOnTemplateCategory = selectAllEndpointsBasedOnTemplateCategory exports.getRootNode = getRootNode +exports.selectEndpointIdentifiersBySessionAndEndpointType = + selectEndpointIdentifiersBySessionAndEndpointType diff --git a/src-electron/db/query-session-notification.js b/src-electron/db/query-session-notification.js index ddf511b4c0..0f87e01e32 100644 --- a/src-electron/db/query-session-notification.js +++ b/src-electron/db/query-session-notification.js @@ -26,6 +26,7 @@ const dbApi = require('./db-api.js') const dbMapping = require('./db-mapping.js') const wsServer = require('../server/ws-server.js') const dbEnum = require('../../src-shared/db-enum.js') + /** * Sets a notification in the SESSION_NOTICE table * @@ -308,6 +309,77 @@ async function getNotificationMessagesWithPattern(db, sessionId, pattern) { return rows.reverse().map((row) => row.NOTICE_MESSAGE) } +/** + * True if the notice mentions this endpoint, cluster, attribute, and the external-storage warning phrase. + * + * @param {string} message + * @param {string} endpointDisplay + * @param {string} clusterName + * @param {string} attributeName + * @returns {boolean} + */ +function messageIsExternalAttributeStorageNoticeForAttribute( + message, + endpointDisplay, + clusterName, + attributeName +) { + let phrase = dbEnum.warnings.externalStorageControl + let endpointStr = String(endpointDisplay) + let clusterStr = String(clusterName) + let attrStr = String(attributeName) + return ( + message.includes(phrase) && + message.includes(`endpoint: ${endpointStr},`) && + message.includes(`cluster: ${clusterStr},`) && + message.includes(attrStr) + ) +} + +/** + * Remove external-attribute-storage session notices for a given attribute. + * + * @param {*} db + * @param {*} sessionId + * @param {*} endpointDisplay + * @param {*} clusterName + * @param {*} attributeName + * @returns {Promise} + */ +async function deleteExternalAttributeStorageNotificationsForAttribute( + db, + sessionId, + endpointDisplay, + clusterName, + attributeName +) { + let likePattern = `%${dbEnum.warnings.externalStorageControl}%` + let rows = await dbApi.dbAll( + db, + `SELECT NOTICE_ID, NOTICE_MESSAGE FROM SESSION_NOTICE + WHERE SESSION_REF = ? AND NOTICE_MESSAGE LIKE ?`, + [sessionId, likePattern] + ) + let endpointStr = String(endpointDisplay) + let clusterStr = String(clusterName) + let attrStr = String(attributeName) + let ids = [] + for (let row of rows) { + if ( + messageIsExternalAttributeStorageNoticeForAttribute( + row.NOTICE_MESSAGE, + endpointStr, + clusterStr, + attrStr + ) + ) { + ids.push(row.NOTICE_ID) + } + } + if (ids.length === 0) return false + return Promise.all(ids.map((id) => deleteNotification(db, id))) +} + // exports exports.setNotification = setNotification exports.deleteNotification = deleteNotification @@ -321,3 +393,7 @@ exports.setNotificationOnFeatureChange = setNotificationOnFeatureChange exports.setRequiredElementWarning = setRequiredElementWarning exports.deleteNotificationWithPatterns = deleteNotificationWithPatterns exports.getNotificationMessagesWithPattern = getNotificationMessagesWithPattern +exports.deleteExternalAttributeStorageNotificationsForAttribute = + deleteExternalAttributeStorageNotificationsForAttribute +exports.messageIsExternalAttributeStorageNoticeForAttribute = + messageIsExternalAttributeStorageNoticeForAttribute diff --git a/src-electron/rest/user-data.js b/src-electron/rest/user-data.js index 468d1572d8..72a66f55ea 100644 --- a/src-electron/rest/user-data.js +++ b/src-electron/rest/user-data.js @@ -423,6 +423,63 @@ function httpPostForcedExternal(db) { } } +/** + * Set or delete external storage warnings when the user changes attribute storage. + * + * @param {*} db + * @param {*} sessionId + * @param {*} endpointTypeId + * @param {*} clusterRef + * @param {*} attributeId + * @param {*} storageValue + */ +async function handleAttributeStorageNotification( + db, + sessionId, + endpointTypeId, + clusterRef, + attributeId, + storageValue +) { + let attr = await queryZcl.selectAttributeById(db, attributeId) + let cluster = await queryZcl.selectClusterById(db, clusterRef) + let endpointIdentifiers = + await queryEndpoint.selectEndpointIdentifiersBySessionAndEndpointType( + db, + sessionId, + endpointTypeId + ) + let endpointDisplay = + endpointIdentifiers.length > 0 + ? String(endpointIdentifiers[0]) + : String(endpointTypeId) + let attributeName = attr?.name || attr?.label || 'Unknown' + let clusterName = cluster?.name || 'Unknown' + let userMessage = env.formatEmojiMessage( + '⚠️', + `On endpoint: ${endpointDisplay}, cluster: ${clusterName}, attribute ${attributeName} has ${dbEnum.warnings.externalStorageControl}` + ) + + if (storageValue === dbEnum.storageOption.external) { + await querySessionNotification.setWarningIfMessageNotExists( + db, + sessionId, + userMessage + ) + } else if ( + storageValue === dbEnum.storageOption.ram || + storageValue === dbEnum.storageOption.nvm + ) { + await querySessionNotification.deleteExternalAttributeStorageNotificationsForAttribute( + db, + sessionId, + endpointDisplay, + clusterName, + attributeName + ) + } +} + /** * HTTP POST attribute update * @@ -478,6 +535,18 @@ function httpPostAttributeUpdate(db) { ) ) ) + + if (listType === restApi.updateKey.attributeStorage) { + await handleAttributeStorageNotification( + db, + request.zapSessionId, + selectedEndpoint, + clusterRef, + id, + value + ) + } + // send latest value to frontend to update UI let eptAttr = await queryZcl.selectEndpointTypeAttribute( db, diff --git a/src-electron/validation/conformance-checker.js b/src-electron/validation/conformance-checker.js index c757730a5a..12b68dd4da 100644 --- a/src-electron/validation/conformance-checker.js +++ b/src-electron/validation/conformance-checker.js @@ -241,7 +241,7 @@ function generateWarningMessage( clusterPrefix + ` ${isSingular ? 'attribute' : 'attributes'} ${externalAttrNames},` + ` required by feature: ${featureData.name} (${featureData.code}),` + - ` ${isSingular ? 'has' : 'have'} external storage and ZAP does not have control over it.` + ` ${isSingular ? 'has' : 'have'} ${dbEnum.warnings.externalStorageControl}` ) result.displayWarning = true } diff --git a/src-shared/db-enum.js b/src-shared/db-enum.js index da3280577d..431e82e666 100644 --- a/src-shared/db-enum.js +++ b/src-shared/db-enum.js @@ -292,3 +292,9 @@ exports.logicalOperators = { or: 'or', not: 'not' } + +/** Text fragments shared by warning messages */ +exports.warnings = { + externalStorageControl: + 'external storage and ZAP does not have control over it.' +} diff --git a/test/feature.test.js b/test/feature.test.js index 50f8e9a04e..94cf2aade6 100644 --- a/test/feature.test.js +++ b/test/feature.test.js @@ -827,7 +827,7 @@ test( ) expectedWarning = clusterPrefix + - ` attribute CurrentHue, required by feature: ${featureHS.name} (${featureHS.code}), has external storage and ZAP does not have control over it.` + ` attribute CurrentHue, required by feature: ${featureHS.name} (${featureHS.code}), has ${dbEnum.warnings.externalStorageControl}` expect(result.displayWarning).toBeTruthy() expect(result.disableChange).toBeFalsy() expect(result.warningMessage).toContain(expectedWarning) diff --git a/test/notification.test.js b/test/notification.test.js index 2ab5e033c2..1c0e610a54 100644 --- a/test/notification.test.js +++ b/test/notification.test.js @@ -29,6 +29,7 @@ const queryPackage = require('../src-electron/db/query-package') const zclLoader = require('../src-electron/zcl/zcl-loader') const testQuery = require('./test-query') const conformChecker = require('../src-electron/validation/conformance-checker') +const dbEnum = require('../src-shared/db-enum') let db = null @@ -490,3 +491,90 @@ test( }, testUtil.timeout.long() ) + +test( + 'Notification: detect and delete external-storage warnings (feature + attribute toggle)', + async () => { + let sessionId = await querySession.createBlankSession(db) + let phrase = dbEnum.warnings.externalStorageControl + + let clusterPrefix = env.formatEmojiMessage( + '⚠️', + `Check Feature Compliance on endpoint: 7, cluster: Color Control,` + ) + let featureWarning = + clusterPrefix + + ` attribute CurrentHue, required by feature: HS (HS), has ${phrase}` + + let attributeToggleWarning = env.formatEmojiMessage( + '⚠️', + `On endpoint: 7, cluster: Color Control, attribute CurrentHue has ${phrase}` + ) + + // set both feature and attribute toggle warnings + await sessionNotification.setNotification( + db, + 'WARNING', + featureWarning, + sessionId, + 2, + 0 + ) + await sessionNotification.setNotification( + db, + 'WARNING', + attributeToggleWarning, + sessionId, + 2, + 0 + ) + + // feature toggle warning -> true + expect( + sessionNotification.messageIsExternalAttributeStorageNoticeForAttribute( + featureWarning, + 7, + 'Color Control', + 'CurrentHue' + ) + ).toBe(true) + // feature toggle warning on the wrong attribute -> false + expect( + sessionNotification.messageIsExternalAttributeStorageNoticeForAttribute( + featureWarning, + 7, + 'Color Control', + 'OtherAttr' + ) + ).toBe(false) + // feature toggle warning on the wrong endpoint -> false + expect( + sessionNotification.messageIsExternalAttributeStorageNoticeForAttribute( + featureWarning, + 8, + 'Color Control', + 'CurrentHue' + ) + ).toBe(false) + // attribute toggle warning -> true + expect( + sessionNotification.messageIsExternalAttributeStorageNoticeForAttribute( + attributeToggleWarning, + 7, + 'Color Control', + 'CurrentHue' + ) + ).toBe(true) + // should delete both feature and attribute toggle warnings + await sessionNotification.deleteExternalAttributeStorageNotificationsForAttribute( + db, + sessionId, + 7, + 'Color Control', + 'CurrentHue' + ) + let messages = await testQuery.getAllNotificationMessages(db, sessionId) + expect(messages.length).toBe(0) + }, + testUtil.timeout.long() +)