Fix the Legal Clusters view where users will only see clusters associated with a device type and not just enabled clusters which is wrong (#1573)

- Use the device types on the selected endpoint to figure out the device type clusters and then only show those clusters in the UI.
- The Legal clusters UI view will also not allow users to enable/disable non compliant clusters and also not show non compliant clusters.
- Adding cypress tests for filters of legal and enabled clusters
- Add more tests where the clusters are check for enablement when clicking on the client and server checkboxes
- Github: ZAP#316

Author: brdandu

cypress/e2e/clusters/cluster-filter.cy.js

@@ -13,19 +13,94 @@ describe('Testing cluster filters', () => {
     })
     cy.setZclProperties()
     cy.fixture('data').then((data) => {
-      cy.addEndpoint(data.endpoint1, data.cluster1)
+      // Selecting SE Range Extender for Zigbee
+      // Selecting Matter Dimmable Light (0x0101) for Matter
+      cy.addEndpoint(data.endpoint9)
     })
   })
   it(
     'filter enabled clusters and check clusters',
     { retries: { runMode: 2, openMode: 2 } },
     () => {
       cy.get('[data-test="filter-input"]').click()
+      // Selecting Enabled clusters
       cy.get('.q-virtual-scroll__content > :nth-child(3)').click({
         force: true
       })
       cy.fixture('data').then((data) => {
-        cy.get('tbody').children().contains(data.cluster2).should('not.exist')
+        // Power Configurator for Zigbee is disabled and should not show up
+        // Occupancy Sensing for Matter is disabled and should not exist
+        cy.get('tbody').children().contains(data.cluster10).should('not.exist')
+        // Basic for Zigbee is enabled and should show up
+        // Identify for Matter is enabled and should show up
+        cy.get('tbody').children().contains(data.cluster11).should('exist')
+      })
+    }
+  )
+  it(
+    'filter legal clusters and check clusters',
+    { retries: { runMode: 2, openMode: 2 } },
+    () => {
+      cy.get('[data-test="filter-input"]').click()
+      // Selecting Legal Clusters
+      cy.get('.q-virtual-scroll__content > :nth-child(4)').click({
+        force: true
+      })
+      cy.fixture('data').then((data) => {
+        // Power Configurator for Zigbee is legal and should show up
+        // Occupancy Sensing for Matter is legal and should show up
+        if (data.cluster10 == 'Power Configuration') {
+          cy.get('tbody')
+            .children()
+            .contains(data.cluster10)
+            .should('exist')
+            .parents('tr')
+            .within(() => {
+              cy.get('div[role="checkbox"][aria-label="Client"]')
+                .should('have.attr', 'aria-checked', 'false')
+                .click({ force: true })
+                .should('have.attr', 'aria-checked', 'false') // Even when clicking on the client checkbox it should not be enabled because of legal cluster filter setting does not allow non Device type clusters to be enabled.
+            })
+          cy.get('tbody')
+            .children()
+            .contains(data.cluster10)
+            .should('exist')
+            .parents('tr')
+            .within(() => {
+              cy.get('div[role="checkbox"][aria-label="Server"]')
+                .should('have.attr', 'aria-checked', 'false')
+                .click({ force: true })
+                .should('have.attr', 'aria-checked', 'true') // when clicking on the server checkbox it should be enabled because of legal cluster filter setting allows it to be selected
+            })
+        } else {
+          // Occupancy Sensing
+          cy.get('tbody')
+            .children()
+            .contains(data.cluster10)
+            .should('exist')
+            .parents('tr')
+            .within(() => {
+              cy.get('div[role="checkbox"][aria-label="Client"]')
+                .should('have.attr', 'aria-checked', 'false')
+                .click({ force: true })
+                .should('have.attr', 'aria-checked', 'true') // when clicking on the server checkbox it should be enabled because of legal cluster filter setting allows it to be selected
+            })
+          cy.get('tbody')
+            .children()
+            .contains(data.cluster10)
+            .should('exist')
+            .parents('tr')
+            .within(() => {
+              cy.get('div[role="checkbox"][aria-label="Server"]')
+                .should('have.attr', 'aria-checked', 'false')
+                .click({ force: true })
+                .should('have.attr', 'aria-checked', 'false') // Even when clicking on the client checkbox it should not be enabled because of legal cluster filter setting does not allow non Device type clusters to be enabled.
+            })
+        }
+
+        // Basic for Zigbee is legal and should show up
+        // Identify for Matter is legal and should show up
+        cy.get('tbody').children().contains(data.cluster11).should('exist')
       })
     }
   )

cypress/fixtures/data.json

@@ -7,6 +7,7 @@
   "endpoint6": "LO Dimmable Light (0x0101)",
   "endpoint7": "Billing Unit (0x0203)",
   "endpoint8": "Billing Unit (0x0203)",
+  "endpoint9": "SE Range Extender (0x0008)",
   "cluster1": "General",
   "cluster2": "Power Configuration",
   "cluster3": "Basic",
@@ -16,6 +17,8 @@
   "cluster7": "Identify",
   "cluster8": "Data Sharing",
   "cluster9": "General",
+  "cluster10": "Power Configuration",
+  "cluster11": "Basic",
   "attribute1": "ZCL version",
   "attribute2": "application version",
   "attribute3": "OTA Upgrade Server ID",

cypress/matterFixtures/data.json

@@ -7,6 +7,7 @@
   "endpoint6": "Matter Dimmable Light (0x0101)",
   "endpoint7": "Matter Control Bridge",
   "endpoint8": "Matter Color Temperature Light (0x010C)",
+  "endpoint9": "Matter Dimmable Light (0x0101)",
   "cluster1": "General",
   "cluster2": "On/off Switch Configuration",
   "cluster3": "Basic",
@@ -16,6 +17,8 @@
   "cluster7": "Identify",
   "cluster8": "Pulse Width Modulation",
   "cluster9": "General",
+  "cluster10": "Occupancy Sensing",
+  "cluster11": "Identify",
   "attribute1": "GeneratedCommandList",
   "attribute2": "IdentifyTime",
   "attribute3": "ClusterRevision",

src/App.vue

@@ -248,6 +248,10 @@ export default defineComponent({
         }
         this.$store.dispatch('zap/updateSelectedEndpoint', endpoint.id)
         this.$store.commit('zap/toggleEndpointModal', false)
+        this.$store.dispatch(
+          'zap/updateDeviceTypeClustersForSelectedEndpoint',
+          this.endpointDeviceTypeRef[this.endpointType[endpoint.id]]
+        )
       }
     },
     getAppData() {

src/components/ZclClusterManager.vue

@@ -199,23 +199,26 @@ export default {
     },
     relevantClusters: {
       get() {
+        let relevantClusters = []
         if (this.clusters.clusterData) {
-          return this.clusters.clusterData.filter((cluster) =>
+          relevantClusters = this.clusters.clusterData.filter((cluster) =>
             this.filterString == ''
               ? true
               : cluster.label
                   .toLowerCase()
                   .includes(this.filterString.toLowerCase())
           )
         } else {
-          return this.clusters.filter((cluster) =>
+          relevantClusters = this.clusters.filter((cluster) =>
             this.filterString == ''
               ? true
               : cluster.label
                   .toLowerCase()
                   .includes(this.filterString.toLowerCase())
           )
         }
+        this.$store.commit('zap/setRelevantClusters', relevantClusters)
+        return relevantClusters
       }
     },
     enabledClusters: {
@@ -227,6 +230,12 @@ export default {
         return clusters
       }
     },
+    deviceTypeClustersForSelectedEndpoint: {
+      get() {
+        return this.$store.state.zap.endpointTypeView
+          .deviceTypeClustersForSelectedEndpoint
+      }
+    },
     filterOptions: {
       get() {
         return this.$store.state.zap.clusterManager.filterOptions
@@ -302,7 +311,12 @@ export default {
         .filter((a) => {
           return typeof this.filter.clusterFilterFn === 'function'
             ? this.filter.clusterFilterFn(a, {
-                enabledClusters: this.enabledClusters
+                enabledClusters: this.enabledClusters,
+                relevantClusters: this.relevantClusters,
+                deviceTypeRefsForSelectedEndpoint:
+                  this.endpointDeviceTypeRef[this.selectedEndpointId],
+                deviceTypeClustersForSelectedEndpoint:
+                  this.deviceTypeClustersForSelectedEndpoint
               })
             : true
         })
@@ -339,7 +353,12 @@ export default {
     changeDomainFilter(filter) {
       this.$store.dispatch('zap/setDomainFilter', {
         filter: filter,
-        enabledClusters: this.enabledClusters
+        enabledClusters: this.enabledClusters,
+        relevantClusters: this.relevantClusters,
+        deviceTypeRefsForSelectedEndpoint:
+          this.endpointDeviceTypeRef[this.selectedEndpointId],
+        deviceTypeClustersForSelectedEndpoint:
+          this.deviceTypeClustersForSelectedEndpoint
       })
     },
     doActionFilter(filter) {

src/components/ZclCreateModifyEndpoint.vue

@@ -780,6 +780,10 @@ export default {
                 deviceTypeRefs: deviceTypeRef,
                 endpointTypeRef: res.id
               })
+              this.$store.dispatch(
+                'zap/updateDeviceTypeClustersForSelectedEndpoint',
+                this.endpointDeviceTypeRef[this.endpointType[res.id]]
+              )
             })
         })
         .catch((err) => console.log('Error in newEpt: ' + err.message))
@@ -861,6 +865,10 @@ export default {
         deviceTypeRefs: deviceTypeRef,
         endpointTypeRef: this.endpointReference
       })
+      this.$store.dispatch(
+        'zap/updateDeviceTypeClustersForSelectedEndpoint',
+        deviceTypeRef
+      )
     },
     getDeviceOptionLabel(item) {
       if (item == null || item.deviceTypeRef == null) return ''

src/components/ZclDomainClusterView.vue

@@ -166,6 +166,14 @@ limitations under the License.
               :options="optionsForCheckboxes"
               color="primary"
               type="checkbox"
+              :disable="{
+                client:
+                  getClusterDisableStatus(props.row.id).client ||
+                  !getClusterEnableStatus(props.row.id).client,
+                server:
+                  getClusterDisableStatus(props.row.id).server ||
+                  !getClusterEnableStatus(props.row.id).server
+              }"
               @update:model-value="handleClusterSelection(props.row.id, $event)"
               inline
             />
@@ -239,6 +247,11 @@ export default {
   props: ['domainName', 'clusters'],
   mixins: [CommonMixin, uiOptions, EditableAttributesMixin],
   computed: {
+    isLegalClusterFilterActive() {
+      return (
+        this.$store.state.zap.clusterManager.filter.label === 'Legal Clusters'
+      )
+    },
     isInStandalone: {
       get() {
         return this.$store.state.zap.standalone
@@ -254,6 +267,18 @@ export default {
         return this.$store.state.zap.clustersView.recommendedServers
       }
     },
+    // Includes client clusters which are optional for a device type
+    optionalClients: {
+      get() {
+        return this.$store.state.zap.clustersView.optionalClients
+      }
+    },
+    // Includes server clusters which are optional for a device type
+    optionalServers: {
+      get() {
+        return this.$store.state.zap.clustersView.optionalServers
+      }
+    },
     visibleColumns: function () {
       let names = this.columns.map((x) => x.name)
       let statusColumn = 'status'
@@ -281,6 +306,38 @@ export default {
     }
   },
   methods: {
+    getClusterEnableStatus(id) {
+      // Only enforce constraints if the active filter is "Legal Clusters"
+      if (!this.isLegalClusterFilterActive) {
+        return { client: true, server: true } // Allow all actions
+      }
+      let isClientRecommended = this.recommendedClients.includes(id)
+      let isServerRecommended = this.recommendedServers.includes(id)
+      let isClientOptional = this.optionalClients.includes(id)
+      let isServerOptional = this.optionalServers.includes(id)
+      let isClientEnabled = this.selectionClients.includes(id)
+      let isServerEnabled = this.selectionServers.includes(id)
+
+      return {
+        client: isClientRecommended || isClientOptional || isClientEnabled, // Allow enabling only if recommended, optional, or already enabled
+        server: isServerRecommended || isServerOptional || isServerEnabled // Allow enabling only if recommended, optional, or already enabled
+      }
+    },
+    getClusterDisableStatus(id) {
+      // Only enforce constraints if the active filter is "Legal Clusters"
+      if (!this.isLegalClusterFilterActive) {
+        return { client: false, server: false } // No constraints
+      }
+
+      let isClientRecommended = this.recommendedClients.includes(id)
+      let isServerRecommended = this.recommendedServers.includes(id)
+      let isClientOptional = this.optionalClients.includes(id)
+      let isServerOptional = this.optionalServers.includes(id)
+      return {
+        client: isClientRecommended && !isClientOptional, // Disable if recommended and not optional
+        server: isServerRecommended && !isServerOptional // Disable if recommended and not optional
+      }
+    },
     enableAllClusters() {
       this.clusters.forEach(async (singleCluster) => {
         await this.updateZclRolesByClusterSelection(singleCluster.id, [
@@ -499,6 +556,57 @@ export default {
       return tmpEvent
     },
     async handleClusterSelection(id, event) {
+      const activeFilter = this.$store.state.zap.clusterManager.filter.label
+
+      // Only enforce constraints if the active filter is "Legal Clusters"
+      if (activeFilter === 'Legal Clusters') {
+        const disableStatus = this.getClusterDisableStatus(id)
+        const enableStatus = this.getClusterEnableStatus(id)
+
+        // Prevent disabling recommended client
+        if (disableStatus.client && event.includes('client') === false) {
+          this.$q.notify({
+            type: 'warning',
+            position: 'top',
+            message:
+              'You cannot disable a required client cluster for the device types on this endpoint when using a Legal Clusters Filter.'
+          })
+          return
+        }
+
+        // Prevent disabling recommended server
+        if (disableStatus.server && event.includes('server') === false) {
+          this.$q.notify({
+            type: 'warning',
+            position: 'top',
+            message:
+              'You cannot disable a required server cluster for the device types on this endpoint when using a Legal Clusters Filter.'
+          })
+          return
+        }
+
+        // Prevent enabling non-recommended client
+        if (!enableStatus.client && event.includes('client') === true) {
+          this.$q.notify({
+            type: 'warning',
+            position: 'top',
+            message:
+              'You cannot enabled a client cluster not required by the device types on this endpoint when using a Legal Clusters Filter.'
+          })
+          return
+        }
+
+        // Prevent enabling non-recommended server
+        if (!enableStatus.server && event.includes('server') === true) {
+          this.$q.notify({
+            type: 'warning',
+            position: 'top',
+            message:
+              'You cannot enabled a server cluster not required by the device types on this endpoint when using a Legal Clusters Filter.'
+          })
+          return
+        }
+      }
       let modifiedEvent = this.parseCheckboxEventToSelectionEvent(event)
       let selectionEvents = this.processZclSelectionEvent(id, modifiedEvent)