Merge pull request #3330 from FlowFuse/3260-update-device-on-group-me…

…mbership-change

Update device on group membership change

docs/user/device-groups.md

@@ -12,6 +12,12 @@ navTitle: Device Groups
 When managing many devices that are intended to run the same [snapshot](./snapshots.md), Device Groups allow you
 to organise your devices into logical groups.
 These groups can then be set as the target of a [DevOps Pipelines](./devops-pipelines.md).
+
+Furthermore:
+* Devices added to an active Device Group will automatically be updated to the active pipeline snapshot
+* Devices removed from an active Device Group will have their active pipeline snapshot cleared
+More details are provided below in [Adding a Device to a group](#adding-a-device-to-a-group-which-has-an-active-pipeline-snapshot) and [Removing a Device from a group](#removing-a-device-from-a-group-which-has-an-active-pipeline-snapshot)
+
 This greatly simplifies deployments of the same configuration to one or even hundreds of devices with a single click.
 
 The following requirements apply:
@@ -41,7 +47,23 @@ _Note: Adding a description can help you better distinguish device groups._
    1. On the right, you will be shown devices that are already in the device group
 1. Place a checkmark next to the devices in the Available Devices list that you want to add to the Device Group then click "Add Devices"
 1. Place a checkmark next to the devices in the Device Group list that you want to remove then click "Remove Devices"
-1. Click "Save" to commit your changes
+1. Click "Save"
+1. You will be prompted to confirm your changes
+   1. Refer to the below information for more details about what happens when you add or remove devices from a device group
+   1. Click "Confirm" to continue or "Cancel" to abort
 
 _Note: If you make a mistake, you can cancel your changes at any time by clicking "Cancel"_
 _Note: When a device you want to add to a group doesn't appear in the list, it's likely already assigned to another group._
+
+### Adding a Device to a group which has an active pipeline snapshot
+
+When a pipeline stage is operated and it deploys to a device group, that device group remembers the snapshot that was deployed.
+
+Subsequently, if you add a device to a group, it will be instructed to update to the active snapshot.
+
+### Removing a Device from a group which has an active pipeline snapshot
+
+When a pipeline stage is operated and it deploys to a device group, that device group remembers the snapshot that was deployed.
+
+Subsequently, if you remove a device from a group and the device is running the active pipeline snapshot,
+the device snapshot will be cleared, effectively resetting the device to a blank state.

forge/db/controllers/Device.js

@@ -19,8 +19,8 @@ module.exports = {
             device.set('agentVersion', state.agentVersion)
         }
         device.set('lastSeenAt', literal('CURRENT_TIMESTAMP'))
-        if (!state.snapshot) {
-            if (device.currentSnapshot !== null) {
+        if (!state.snapshot || state.snapshot === '0') {
+            if (device.activeSnapshotId !== null) {
                 device.set('activeSnapshotId', null)
             }
         } else {

forge/db/migrations/20240116-01-add-targetSnapshot-to-DeviceGroup.js

@@ -0,0 +1,68 @@
+/* eslint-disable no-unused-vars */
+/**
+ * Add targetSnapshotId to DeviceGroups
+ *
+ * -- DDL for table DeviceGroups - before migration
+ * CREATE TABLE DeviceGroups (
+ *     id            INTEGER       PRIMARY KEY AUTOINCREMENT,
+ *     name          VARCHAR (255) NOT NULL,
+ *     description   TEXT,
+ *     createdAt     DATETIME      NOT NULL,
+ *     updatedAt     DATETIME      NOT NULL,
+ *     ApplicationId INTEGER       REFERENCES Applications (id) ON DELETE CASCADE
+ *                                                              ON UPDATE CASCADE
+ * );
+ *
+ *
+ * -- DDL for table DeviceGroups - after migration
+ * CREATE TABLE DeviceGroups (
+ *     id               INTEGER       PRIMARY KEY AUTOINCREMENT,
+ *     name             VARCHAR (255) NOT NULL,
+ *     description      TEXT,
+ *     createdAt        DATETIME      NOT NULL,
+ *     updatedAt        DATETIME      NOT NULL,
+ *     ApplicationId    INTEGER       REFERENCES Applications (id) ON DELETE CASCADE
+ *                                                                 ON UPDATE CASCADE,
+ *     targetSnapshotId INTEGER       REFERENCES ProjectSnapshots (id) ON DELETE SET NULL
+ *                                                                     ON UPDATE CASCADE
+ * );
+ *
+ *
+ * -- DDL for table DeviceGroups - auto created by sequelize
+ * CREATE TABLE DeviceGroups (
+ *     id               INTEGER       PRIMARY KEY AUTOINCREMENT,
+ *     name             VARCHAR (255) NOT NULL,
+ *     description      TEXT,
+ *     targetSnapshotId INTEGER       REFERENCES ProjectSnapshots (id) ON DELETE SET NULL
+ *                                                                     ON UPDATE CASCADE,
+ *     createdAt        DATETIME      NOT NULL,
+ *     updatedAt        DATETIME      NOT NULL,
+ *     ApplicationId    INTEGER       REFERENCES Applications (id) ON DELETE CASCADE
+ *                                                                 ON UPDATE CASCADE
+ * );
+ *
+ */
+
+const { Sequelize, QueryInterface } = require('sequelize')
+
+module.exports = {
+    /**
+     * upgrade database
+     * @param {QueryInterface} context QueryInterface
+     */
+    up: async (context) => {
+        await context.addColumn('DeviceGroups', 'targetSnapshotId', {
+            type: Sequelize.INTEGER,
+            references: {
+                model: 'ProjectSnapshots',
+                key: 'id'
+            },
+            onUpdate: 'CASCADE',
+            onDelete: 'SET NULL'
+        })
+    },
+
+    down: async (queryInterface, Sequelize) => {
+
+    }
+}

forge/db/models/DeviceGroup.js

@@ -20,10 +20,12 @@ module.exports = {
                 notNull: nameValidator
             }
         },
-        description: { type: DataTypes.TEXT }
+        description: { type: DataTypes.TEXT },
+        targetSnapshotId: { type: DataTypes.INTEGER, allowNull: true }
     },
     associations: function (M) {
         this.belongsTo(M.Application, { onDelete: 'CASCADE' })
+        this.belongsTo(M.ProjectSnapshot, { as: 'targetSnapshot' })
         this.hasMany(M.Device)
     },
     finders: function (M) {
@@ -54,6 +56,11 @@ module.exports = {
                                     ApplicationId: literal('"Devices"."ApplicationId" = "Application"."id"')
                                 },
                                 required: false
+                            },
+                            {
+                                model: M.ProjectSnapshot,
+                                as: 'targetSnapshot',
+                                attributes: ['hashid', 'id', 'name']
                             }
                         ],
                         attributes: {

forge/db/models/ProjectSnapshot.js

@@ -46,6 +46,7 @@ module.exports = {
         this.belongsTo(M.User)
         this.hasMany(M.Device, { foreignKey: 'targetSnapshotId' })
         this.hasMany(M.Device, { foreignKey: 'activeSnapshotId' })
+        this.hasMany(M.DeviceGroup, { foreignKey: 'targetSnapshotId' })
     },
     finders: function (M) {
         const self = this

forge/ee/db/controllers/DeviceGroup.js

@@ -96,18 +96,22 @@ module.exports = {
                 actualAddDevices = addDevices.filter(d => !currentMemberIds.includes(d))
             }
         }
-
-        // wrap the dual operation in a transaction to avoid inconsistent state
+        let changeCount = 0
+        // wrap the operations in a transaction to avoid inconsistent state
         const t = await app.db.sequelize.transaction()
+        const targetSnapshotId = deviceGroup.targetSnapshotId || undefined
         try {
             // add devices
             if (actualAddDevices.length > 0) {
-                await this.assignDevicesToGroup(app, deviceGroup, actualAddDevices, t)
+                changeCount += actualAddDevices.length
+                await this.assignDevicesToGroup(app, deviceGroup, actualAddDevices, targetSnapshotId, t)
             }
             // remove devices
             if (actualRemoveDevices.length > 0) {
-                await this.removeDevicesFromGroup(app, deviceGroup, actualRemoveDevices, t)
+                changeCount += actualRemoveDevices.length
+                await this.removeDevicesFromGroup(app, deviceGroup, actualRemoveDevices, targetSnapshotId, t)
             }
+
             // commit the transaction
             await t.commit()
         } catch (err) {
@@ -120,52 +124,89 @@ module.exports = {
             // otherwise, throw a friendly error message along with the original error
             throw new Error(`Failed to update device group membership: ${err.message}`)
         }
+        if (changeCount > 0) {
+            // clean up where necessary
+            // check to see if the group is now empty
+            const remainingDevices = await deviceGroup.deviceCount()
+            if (remainingDevices === 0) {
+                deviceGroup.targetSnapshotId = null
+                await deviceGroup.save()
+            }
+            // finally, inform the devices an update may be required
+            await this.sendUpdateCommand(app, deviceGroup, actualRemoveDevices)
+        }
     },
 
-    assignDevicesToGroup: async function (app, deviceGroup, deviceList, transaction = null) {
+    assignDevicesToGroup: async function (app, deviceGroup, deviceList, applyTargetSnapshot, transaction = null) {
         const deviceIds = await validateDeviceList(app, deviceGroup, deviceList, null)
-        await app.db.models.Device.update({ DeviceGroupId: deviceGroup.id }, { where: { id: deviceIds.addList }, transaction })
+        const updates = { DeviceGroupId: deviceGroup.id }
+        if (typeof applyTargetSnapshot !== 'undefined') {
+            updates.targetSnapshotId = applyTargetSnapshot
+        }
+        await app.db.models.Device.update(updates, { where: { id: deviceIds.addList }, transaction })
     },
 
     /**
      * Remove 1 or more devices from the specified DeviceGroup
+     * Specifying `activeDeviceGroupTargetSnapshotId` will null the `targetSnapshotId` of each device in `deviceList` where it matches
+     * This is used to remove the project from a device when being removed from a group where the active snapshot is the one applied by the DeviceGroup
      * @param {*} app The application object
-     * @param {*} deviceGroupId The device group id
-     * @param {*} deviceList A list of devices to remove from the group
+     * @param {number} deviceGroupId The device group id
+     * @param {number[]} deviceList A list of devices to remove from the group
+     * @param {number} activeDeviceGroupTargetSnapshotId If specified, null devices `targetSnapshotId` where it matches
      */
-    removeDevicesFromGroup: async function (app, deviceGroup, deviceList, transaction = null) {
+    removeDevicesFromGroup: async function (app, deviceGroup, deviceList, activeDeviceGroupTargetSnapshotId, transaction = null) {
         const deviceIds = await validateDeviceList(app, deviceGroup, null, deviceList)
+        // Before removing from the group, if activeDeviceGroupTargetSnapshotId is specified, null `targetSnapshotId` of each device in `deviceList`
+        // where the device ACTUALLY DOES HAVE the matching targetsnapshotid
+        if (typeof activeDeviceGroupTargetSnapshotId !== 'undefined') {
+            await app.db.models.Device.update({ targetSnapshotId: null }, { where: { id: deviceIds.removeList, DeviceGroupId: deviceGroup.id, targetSnapshotId: activeDeviceGroupTargetSnapshotId }, transaction })
+        }
         // null every device.DeviceGroupId row in device table where the id === deviceGroupId and device.id is in the deviceList
         await app.db.models.Device.update({ DeviceGroupId: null }, { where: { id: deviceIds.removeList, DeviceGroupId: deviceGroup.id }, transaction })
     },
+
     /**
-     * Sends the project id, snapshot hash and settings hash to all devices in the group
-     * so that they can determine what/if it needs to update
-     * NOTE: Only devices belonging to an application are present in a device group
-     * @param {forge.db.models.DeviceGroup} deviceGroup The device group to send an "update" command to
+     * Sends an update to all devices in the group and/or the specified list of devices
+     * so that they can determine what/if it needs to be updated
+     * NOTE: Since device groups only support application owned devices, this will only send updates to application owned devices
+     * @param {forge.db.models.DeviceGroup} [deviceGroup] A device group to send an "update" command to
+     * @param {Number[]} [deviceList] A list of device IDs to send an "update" command to
      */
-    sendUpdateCommand: async function (app, deviceGroup) {
+    sendUpdateCommand: async function (app, deviceGroup, deviceList) {
         if (app.comms) {
-            const application = await deviceGroup.getApplication({ include: [{ model: app.db.models.Team }] })
-            const targetSnapshot = deviceGroup.targetSnapshot || (await app.db.models.ProjectSnapshot.byId(deviceGroup.PipelineStageDeviceGroup.targetSnapshotId))
-            const payloadTemplate = {
-                ownerType: 'application',
-                application: application.hashid,
-                snapshot: targetSnapshot.hashid,
-                settings: null,
-                mode: null,
-                licensed: app.license.active()
+            if (deviceGroup) {
+                const devices = await deviceGroup.getDevices()
+                if (devices?.length) {
+                    // add them to the deviceList if not already present
+                    deviceList = deviceList || []
+                    for (const device of devices) {
+                        if (!deviceList.includes(device.id)) {
+                            deviceList.push(device.id)
+                        }
+                    }
+                }
             }
-            const devices = await deviceGroup.getDevices()
-            for (const device of devices) {
-                // If the device doesnt have the same target snapshot as the group, skip it
-                if (device.targetSnapshotId !== deviceGroup.PipelineStageDeviceGroup.targetSnapshotId) {
-                    continue
+            if (deviceList?.length) {
+                const devices = await app.db.models.Device.getAll({}, { id: deviceList })
+                if (!devices || !devices.devices || devices.devices.length === 0) {
+                    return
+                }
+                const licenseActive = app.license.active()
+                for (const device of devices.devices) {
+                    if (device.ownerType !== 'application') {
+                        continue // ensure we only send updates to application owned devices
+                    }
+                    const payload = {
+                        ownerType: device.ownerType,
+                        application: device.Application?.hashid || null,
+                        snapshot: device.targetSnapshot?.hashid || '0', // '0' means starter snapshot + flows
+                        settings: device.settingsHash || null,
+                        mode: device.mode,
+                        licensed: licenseActive
+                    }
+                    app.comms.devices.sendCommand(device.Team.hashid, device.hashid, 'update', payload)
                 }
-                const payload = { ...payloadTemplate }
-                payload.settings = device.settingsHash || null
-                payload.mode = device.mode
-                app.comms.devices.sendCommand(application.Team.hashid, device.hashid, 'update', payload)
             }
         }
     },

forge/ee/db/controllers/Pipeline.js

@@ -432,6 +432,9 @@ module.exports = {
                 // Update the targetSnapshot of the device group
                 await targetDeviceGroup.PipelineStageDeviceGroup.update({ targetSnapshotId: sourceSnapshot.id }, { transaction })
 
+                // Update the targetSnapshotId on the device group
+                await targetDeviceGroup.update({ targetSnapshotId: sourceSnapshot.id }, { transaction })
+
                 // update all devices targetSnapshotId
                 await app.db.models.Device.update({ targetSnapshotId: sourceSnapshot.id }, { where: { DeviceGroupId: targetDeviceGroup.id }, transaction })
                 // commit the transaction

forge/ee/db/models/PipelineStageDeviceGroup.js

@@ -18,5 +18,31 @@ module.exports = {
         slug: false,
         hashid: false,
         links: false
+    },
+    finders: function (M) {
+        return {
+            static: {
+                byId: async function (idOrHash) {
+                    let id = idOrHash
+                    if (typeof idOrHash === 'string') {
+                        id = M.PipelineStageDeviceGroup.decodeHashid(idOrHash)
+                    }
+                    return this.findOne({
+                        where: { PipelineStageId: id },
+                        include: [
+                            {
+                                model: M.DeviceGroup,
+                                attributes: ['hashid', 'id', 'name', 'description', 'ApplicationId']
+                            },
+                            {
+                                model: M.PipelineStage,
+                                attributes: ['hashid', 'id', 'name', 'action', 'NextStageId', 'PipelineId']
+                            },
+                            { model: M.ProjectSnapshot, as: 'targetSnapshot', attributes: ['id', 'hashid', 'name'] }
+                        ]
+                    })
+                }
+            }
+        }
     }
 }

frontend/src/components/pipelines/Stage.vue

@@ -107,10 +107,10 @@
             </div>
             <div v-if="stage.stageType === StageType.DEVICEGROUP" class="ff-pipeline-stage-row">
                 <label>Deployed:</label>
-                <div v-ff-tooltip="stage.deviceGroup?.hasTargetSnapshot && (stage.deviceGroup?.activeMatchCount === stage.deviceGroup?.deviceCount) ? 'All devices have the latest pipeline snapshot deployed' : 'Some devices do not have the latest pipeline snapshot deployed'">
+                <div v-ff-tooltip="stage.state?.hasTargetSnapshot && (stage.state?.activeMatchCount === stage.deviceGroup?.deviceCount) ? 'All devices have the latest pipeline snapshot deployed' : 'Some devices do not have the latest pipeline snapshot deployed'">
                     <StatusBadge
-                        :text="stage.deviceGroup?.activeMatchCount"
-                        :status="stage.deviceGroup?.hasTargetSnapshot && (stage.deviceGroup?.activeMatchCount === stage.deviceGroup?.deviceCount) ? 'success' : 'warning'"
+                        :text="stage.state?.activeMatchCount"
+                        :status="stage.state?.hasTargetSnapshot && (stage.state?.activeMatchCount === stage.deviceGroup?.deviceCount) ? 'success' : 'warning'"
                     />
                 </div>
             </div>