From 828c5f69b9f678c2acb330a3be4cc0d99bce6315 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:12:11 +0100
Subject: [PATCH 01/19] Add settings to DeviceGroups table

---
 ...20241016-01-add-settings-to-devicegroup.js | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 forge/db/migrations/20241016-01-add-settings-to-devicegroup.js

diff --git a/forge/db/migrations/20241016-01-add-settings-to-devicegroup.js b/forge/db/migrations/20241016-01-add-settings-to-devicegroup.js
new file mode 100644
index 0000000000..30bbc2b3db
--- /dev/null
+++ b/forge/db/migrations/20241016-01-add-settings-to-devicegroup.js
@@ -0,0 +1,19 @@
+/**
+ * Add settings to DeviceGroups table
+ */
+const { DataTypes } = require('sequelize')
+
+module.exports = {
+    /**
+     * upgrade database
+     * @param {QueryInterface} context Sequelize.QueryInterface
+     */
+    up: async (context) => {
+        await context.addColumn('DeviceGroups', 'settings', {
+            type: DataTypes.TEXT,
+            defaultValue: null
+        })
+    },
+    down: async (context) => {
+    }
+}

From 321eee43d160ba55cd2178cee0b337ad6d011da7 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:20:14 +0100
Subject: [PATCH 02/19] add settings field to model

---
 forge/db/models/DeviceGroup.js | 272 +++++++++++++++++----------------
 1 file changed, 141 insertions(+), 131 deletions(-)

diff --git a/forge/db/models/DeviceGroup.js b/forge/db/models/DeviceGroup.js
index b1546b8896..6c0e8fe8fe 100644
--- a/forge/db/models/DeviceGroup.js
+++ b/forge/db/models/DeviceGroup.js
@@ -1,131 +1,141 @@
-/**
- * A DeviceGroup.
- * A logical grouping of devices for the primary intent of group deployments in the pipeline stages.
- * @namespace forge.db.models.DeviceGroup
- */
-
-const { DataTypes, literal } = require('sequelize')
-
-const { buildPaginationSearchClause } = require('../utils')
-const nameValidator = { msg: 'Device Group name cannot be empty' }
-
-module.exports = {
-    name: 'DeviceGroup',
-    schema: {
-        name: {
-            type: DataTypes.STRING,
-            allowNull: false,
-            validate: {
-                notEmpty: nameValidator,
-                notNull: nameValidator
-            }
-        },
-        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) {
-        const self = this
-        const deviceCountLiteral = literal(`(
-            SELECT COUNT(*)
-            FROM "Devices" AS "device"
-            WHERE
-            "device"."DeviceGroupId" = "DeviceGroup"."id"
-            AND
-            "device"."ApplicationId" = "DeviceGroup"."ApplicationId"    
-        )`)
-        return {
-            static: {
-                byId: async function (id) {
-                    if (typeof id === 'string') {
-                        id = M.DeviceGroup.decodeHashid(id)
-                    }
-                    // find one, include application, devices and device count
-                    return self.findOne({
-                        where: { id },
-                        include: [
-                            { model: M.Application, attributes: ['hashid', 'id', 'name', 'TeamId'] },
-                            {
-                                model: M.Device,
-                                attributes: ['hashid', 'id', 'name', 'type', 'TeamId', 'ApplicationId', 'ProjectId', 'ownerType'],
-                                where: {
-                                    ApplicationId: literal('"Devices"."ApplicationId" = "Application"."id"')
-                                },
-                                required: false
-                            },
-                            {
-                                model: M.ProjectSnapshot,
-                                as: 'targetSnapshot',
-                                attributes: ['hashid', 'id', 'name']
-                            }
-                        ],
-                        attributes: {
-                            include: [
-                                [
-                                    deviceCountLiteral,
-                                    'deviceCount'
-                                ]
-                            ]
-                        }
-                    })
-                },
-                getAll: async (pagination = {}, where = {}) => {
-                    const limit = parseInt(pagination.limit) || 1000
-                    if (pagination.cursor) {
-                        pagination.cursor = M.DeviceGroup.decodeHashid(pagination.cursor)
-                    }
-                    if (where.ApplicationId && typeof where.ApplicationId === 'string') {
-                        where.ApplicationId = M.Application.decodeHashid(where.ApplicationId)
-                    }
-                    const [rows, count] = await Promise.all([
-                        this.findAll({
-                            where: buildPaginationSearchClause(pagination, where, ['DeviceGroup.name', 'DeviceGroup.description']),
-                            include: [
-                                {
-                                    model: M.ProjectSnapshot,
-                                    as: 'targetSnapshot',
-                                    attributes: ['hashid', 'id', 'name']
-                                }
-                            ],
-                            attributes: {
-                                include: [
-                                    [
-                                        deviceCountLiteral,
-                                        'deviceCount'
-                                    ]
-                                ]
-                            },
-                            order: [['id', 'ASC']],
-                            limit
-                        }),
-                        this.count({ where })
-                    ])
-                    return {
-                        meta: {
-                            next_cursor: rows.length === limit ? rows[rows.length - 1].hashid : undefined
-                        },
-                        count,
-                        groups: rows
-                    }
-                }
-            },
-            instance: {
-                deviceCount: async function () {
-                    return await M.Device.count({ where: { DeviceGroupId: this.id, ApplicationId: this.ApplicationId } })
-                },
-                getDevices: async function () {
-                    return await M.Device.findAll({
-                        where: {
-                            DeviceGroupId: this.id,
-                            ApplicationId: this.ApplicationId
-                        }
-                    })
-                }
-            }
-        }
-    }
-}
+/**
+ * A DeviceGroup.
+ * A logical grouping of devices for the primary intent of group deployments in the pipeline stages.
+ * @namespace forge.db.models.DeviceGroup
+ */
+
+const { DataTypes, literal } = require('sequelize')
+
+const { buildPaginationSearchClause } = require('../utils')
+const nameValidator = { msg: 'Device Group name cannot be empty' }
+
+module.exports = {
+    name: 'DeviceGroup',
+    schema: {
+        name: {
+            type: DataTypes.STRING,
+            allowNull: false,
+            validate: {
+                notEmpty: nameValidator,
+                notNull: nameValidator
+            }
+        },
+        description: { type: DataTypes.TEXT },
+        targetSnapshotId: { type: DataTypes.INTEGER, allowNull: true },
+        settings: {
+            type: DataTypes.TEXT,
+            set (value) {
+                this.setDataValue('settings', JSON.stringify(value))
+            },
+            get () {
+                const rawValue = this.getDataValue('settings') || '{}'
+                return JSON.parse(rawValue)
+            }
+        }
+    },
+    associations: function (M) {
+        this.belongsTo(M.Application, { onDelete: 'CASCADE' })
+        this.belongsTo(M.ProjectSnapshot, { as: 'targetSnapshot' })
+        this.hasMany(M.Device)
+    },
+    finders: function (M) {
+        const self = this
+        const deviceCountLiteral = literal(`(
+            SELECT COUNT(*)
+            FROM "Devices" AS "device"
+            WHERE
+            "device"."DeviceGroupId" = "DeviceGroup"."id"
+            AND
+            "device"."ApplicationId" = "DeviceGroup"."ApplicationId"    
+        )`)
+        return {
+            static: {
+                byId: async function (id) {
+                    if (typeof id === 'string') {
+                        id = M.DeviceGroup.decodeHashid(id)
+                    }
+                    // find one, include application, devices and device count
+                    return self.findOne({
+                        where: { id },
+                        include: [
+                            { model: M.Application, attributes: ['hashid', 'id', 'name', 'TeamId'] },
+                            {
+                                model: M.Device,
+                                attributes: ['hashid', 'id', 'name', 'type', 'TeamId', 'ApplicationId', 'ProjectId', 'ownerType'],
+                                where: {
+                                    ApplicationId: literal('"Devices"."ApplicationId" = "Application"."id"')
+                                },
+                                required: false
+                            },
+                            {
+                                model: M.ProjectSnapshot,
+                                as: 'targetSnapshot',
+                                attributes: ['hashid', 'id', 'name']
+                            }
+                        ],
+                        attributes: {
+                            include: [
+                                [
+                                    deviceCountLiteral,
+                                    'deviceCount'
+                                ]
+                            ]
+                        }
+                    })
+                },
+                getAll: async (pagination = {}, where = {}) => {
+                    const limit = parseInt(pagination.limit) || 1000
+                    if (pagination.cursor) {
+                        pagination.cursor = M.DeviceGroup.decodeHashid(pagination.cursor)
+                    }
+                    if (where.ApplicationId && typeof where.ApplicationId === 'string') {
+                        where.ApplicationId = M.Application.decodeHashid(where.ApplicationId)
+                    }
+                    const [rows, count] = await Promise.all([
+                        this.findAll({
+                            where: buildPaginationSearchClause(pagination, where, ['DeviceGroup.name', 'DeviceGroup.description']),
+                            include: [
+                                {
+                                    model: M.ProjectSnapshot,
+                                    as: 'targetSnapshot',
+                                    attributes: ['hashid', 'id', 'name']
+                                }
+                            ],
+                            attributes: {
+                                include: [
+                                    [
+                                        deviceCountLiteral,
+                                        'deviceCount'
+                                    ]
+                                ]
+                            },
+                            order: [['id', 'ASC']],
+                            limit
+                        }),
+                        this.count({ where })
+                    ])
+                    return {
+                        meta: {
+                            next_cursor: rows.length === limit ? rows[rows.length - 1].hashid : undefined
+                        },
+                        count,
+                        groups: rows
+                    }
+                }
+            },
+            instance: {
+                deviceCount: async function () {
+                    return await M.Device.count({ where: { DeviceGroupId: this.id, ApplicationId: this.ApplicationId } })
+                },
+                getDevices: async function () {
+                    return await M.Device.findAll({
+                        where: {
+                            DeviceGroupId: this.id,
+                            ApplicationId: this.ApplicationId
+                        }
+                    })
+                }
+            }
+        }
+    }
+}

From 6529352443fc6fe0de4c33a9a42c785c4bf343aa Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:20:28 +0100
Subject: [PATCH 03/19] add event logger for device group settings update

---
 forge/auditLog/application.js | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/forge/auditLog/application.js b/forge/auditLog/application.js
index 4464512cdf..4c67bb1b5c 100644
--- a/forge/auditLog/application.js
+++ b/forge/auditLog/application.js
@@ -55,6 +55,11 @@ module.exports = {
                 },
                 async membersChanged (actionedBy, error, application, deviceGroup, updates, info) {
                     await log('application.deviceGroup.members.changed', actionedBy, application?.id, generateBody({ error, application, deviceGroup, updates, info }))
+                },
+                settings: {
+                    async updated (actionedBy, error, application, deviceGroup, updates) {
+                        await log('application.deviceGroup.settings.updated', actionedBy, application?.id, generateBody({ error, application, deviceGroup, updates }))
+                    }
                 }
             }
         }

From 3ec5fcc144c8864d4550d1d4f5f2081c831e0bd5 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:22:00 +0100
Subject: [PATCH 04/19] device settings env should merge device group env  (if
 any)

---
 forge/db/models/Device.js | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/forge/db/models/Device.js b/forge/db/models/Device.js
index 444efccef7..76b6915a12 100644
--- a/forge/db/models/Device.js
+++ b/forge/db/models/Device.js
@@ -148,17 +148,36 @@ module.exports = {
                     })
                 },
                 async updateSettingsHash (settings) {
-                    const _settings = settings || await this.getAllSettings()
+                    const _settings = settings || await this.getAllSettings({ mergeDeviceGroupSettings: true })
                     delete _settings.autoSnapshot // autoSnapshot is not part of the settings hash
                     this.settingsHash = hashSettings(_settings)
                 },
-                async getAllSettings () {
+                async getAllSettings (options = { mergeDeviceGroupSettings: false }) {
+                    const mergeDeviceGroupSettings = options.mergeDeviceGroupSettings || false
                     const result = {}
                     const settings = await this.getDeviceSettings()
                     settings.forEach(setting => {
                         result[setting.key] = setting.value
                     })
                     result.env = Controllers.Device.insertPlatformSpecificEnvVars(this, result.env) // add platform specific device env vars
+                    // if the device is a group member, we need to merge the group settings
+                    if (mergeDeviceGroupSettings && this.DeviceGroupId) {
+                        const group = this.DeviceGroup || await M.DeviceGroup.byId(this.DeviceGroupId)
+                        if (group) {
+                            const groupEnv = await group.settings.env || []
+                            // Merge rule: If the device has an env var AND it has a value, it remains unchanged.
+                            // Otherwise, the value is taken from the group.
+                            // This is to allow the device to override a (global) group env setting.
+                            groupEnv.forEach(env => {
+                                const existing = result.env.find(e => e.name === env.name)
+                                if (!existing) {
+                                    result.env.push(env)
+                                } else if (existing && !existing.value) {
+                                    existing.value = env.value
+                                }
+                            })
+                        }
+                    }
                     if (!Object.prototype.hasOwnProperty.call(result, 'autoSnapshot')) {
                         result.autoSnapshot = DEFAULT_SETTINGS.autoSnapshot
                     }

From bdb9097d715739580309b04fa03c9f7c52fcbc8f Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:41:42 +0100
Subject: [PATCH 05/19] Update member devices settings hash if the group
 settings are updated

---
 forge/db/models/DeviceGroup.js | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/forge/db/models/DeviceGroup.js b/forge/db/models/DeviceGroup.js
index 6c0e8fe8fe..5a2951944a 100644
--- a/forge/db/models/DeviceGroup.js
+++ b/forge/db/models/DeviceGroup.js
@@ -38,6 +38,37 @@ module.exports = {
         this.belongsTo(M.ProjectSnapshot, { as: 'targetSnapshot' })
         this.hasMany(M.Device)
     },
+    hooks: function (M, app) {
+        return {
+            afterUpdate: async (deviceGroup, options) => {
+                // if `settings` is updated, we need to update the settings hash
+                // of any member devices
+                if (deviceGroup.changed('settings')) {
+                    const include = [
+                        { model: M.ProjectSnapshot, as: 'targetSnapshot', attributes: ['id', 'hashid', 'name'] },
+                        {
+                            model: M.DeviceGroup,
+                            attributes: ['hashid', 'id', 'ApplicationId', 'settings']
+                        },
+                        { model: M.Application, attributes: ['hashid', 'id', 'name', 'links'] }
+                    ]
+                    const where = {
+                        DeviceGroupId: deviceGroup.id,
+                        ApplicationId: deviceGroup.ApplicationId
+                    }
+                    const devices = await M.Device.findAll({ where, include })
+                    const updateAndSave = async (device) => {
+                        await device.updateSettingsHash()
+                        await device.save({
+                            hooks: false, // skip the afterSave hook for device model (we have only updated the settings hash)
+                            transaction: options?.transaction // pass the transaction (if any)
+                        })
+                    }
+                    await Promise.all(devices.map(updateAndSave))
+                }
+            }
+        }
+    },
     finders: function (M) {
         const self = this
         const deviceCountLiteral = literal(`(

From 6f37cf5261a7151ac86bf41d869cc49a4ca986be Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:42:25 +0100
Subject: [PATCH 06/19] add support for updating a device group settings

---
 forge/ee/db/controllers/DeviceGroup.js | 95 ++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 7 deletions(-)

diff --git a/forge/ee/db/controllers/DeviceGroup.js b/forge/ee/db/controllers/DeviceGroup.js
index 9cab468f61..991fb21426 100644
--- a/forge/ee/db/controllers/DeviceGroup.js
+++ b/forge/ee/db/controllers/DeviceGroup.js
@@ -1,6 +1,9 @@
 const { Op, ValidationError } = require('sequelize')
 
 const { ControllerError } = require('../../../lib/errors')
+
+const hasProperty = (obj, key) => obj && Object.prototype.hasOwnProperty.call(obj, key)
+
 class DeviceGroupMembershipValidationError extends ControllerError {
     /**
      * @param {string} code
@@ -50,7 +53,7 @@ module.exports = {
      * @param {string} [options.description] - The new description of the Device Group. Exclude to keep the current description.
      * @param {number} [options.targetSnapshotId] - The new target snapshot id of the Device Group. Exclude to keep the current snapshot. Send null to clear the current target snapshot.
      */
-    updateDeviceGroup: async function (app, deviceGroup, { name = undefined, description = undefined, targetSnapshotId = undefined } = {}) {
+    updateDeviceGroup: async function (app, deviceGroup, { name = undefined, description = undefined, targetSnapshotId = undefined, settings = undefined } = {}) {
         // * deviceGroup is required.
         // * name, description, color are optional
         if (!deviceGroup) {
@@ -67,6 +70,32 @@ module.exports = {
             changed = true
         }
 
+        if (typeof settings !== 'undefined' && hasProperty(settings, 'env')) {
+            // NOTE: For now, device group settings only support environment variables
+
+            // validate settings
+            if (!Array.isArray(settings.env)) {
+                throw new ValidationError('Invalid settings')
+            }
+            settings.env.forEach((envVar) => {
+                if (!envVar?.name?.match(/^[a-zA-Z_]+[a-zA-Z0-9_]*$/)) {
+                    throw new ValidationError(`Invalid Env Var name '${envVar.name}'`)
+                }
+            })
+            // find duplicates
+            const seen = new Set()
+            const duplicates = settings.env.some(item => { return seen.size === seen.add(item.name).size })
+            if (duplicates) {
+                throw new ValidationError('Duplicate Env Var names provided')
+            }
+
+            deviceGroup.settings = {
+                ...deviceGroup.settings,
+                env: settings.env
+            }
+            changed = true
+        }
+
         if (typeof targetSnapshotId !== 'undefined') {
             let snapshotId = targetSnapshotId
             // ensure the snapshot exists (if targetSnapshotId is not null)
@@ -90,21 +119,21 @@ module.exports = {
                 }
                 await transaction.commit()
                 saved = true
+                changed = true
             } catch (err) {
                 await transaction.rollback()
                 throw err
             }
-
-            // inform the devices an update is required
-            if (devices?.length) {
-                await this.sendUpdateCommand(app, deviceGroup)
-            }
         }
 
         if (changed && !saved) {
             await deviceGroup.save()
         }
         await deviceGroup.reload()
+
+        if (changed) {
+            await this.sendUpdateCommand(app, deviceGroup)
+        }
         return deviceGroup
     },
 
@@ -178,8 +207,8 @@ module.exports = {
             const remainingDevices = await deviceGroup.deviceCount()
             if (remainingDevices === 0) {
                 deviceGroup.targetSnapshotId = null
-                await deviceGroup.save()
             }
+            await deviceGroup.save()
             // finally, inform the devices an update may be required
             await this.sendUpdateCommand(app, deviceGroup, actualRemoveDevices)
         }
@@ -214,6 +243,58 @@ module.exports = {
         await app.db.models.Device.update({ DeviceGroupId: null }, { where: { id: deviceIds.removeList, DeviceGroupId: deviceGroup.id }, transaction })
     },
 
+    // updateSettings: async function (app, deviceGroup, settings, user) {
+    //     // NOTE: For now, device group settings only support environment variables
+    //     if (!hasProperty(settings, 'env')) {
+    //         return // nothing to do
+    //     }
+
+    //     // validate settings
+    //     if (!Array.isArray(settings.env)) {
+    //         throw new ValidationError('Invalid settings')
+    //     }
+    //     settings.env.forEach((envVar) => {
+    //         if (!envVar?.name?.match(/^[a-zA-Z_]+[a-zA-Z0-9_]*$/)) {
+    //             throw new ValidationError(`Invalid Env Var name '${envVar.name}'`)
+    //         }
+    //     })
+    //     // find duplicates
+    //     const seen = new Set()
+    //     const duplicates = settings.env.some(item => { return seen.size === seen.add(item.name).size })
+    //     if (duplicates) {
+    //         throw new ValidationError('Duplicate Env Var names provided')
+    //     }
+
+    //     // for audit log
+    //     const deviceGroupLogger = getApplicationLogger(app)
+    //     const updates = new app.auditLog.formatters.UpdatesCollection()
+    //     if (!deviceGroup.Application) {
+    //         await deviceGroup.reload({ include: [{ model: app.db.models.Application }] })
+    //     }
+    //     // transform the env arrays to a map for better logging format
+    //     const currentEnv = (deviceGroup.settings?.env || []).reduce((acc, e) => {
+    //         acc[e.name] = e.value
+    //         return acc
+    //     }, {})
+    //     const newEnv = settings.env.reduce((acc, e) => {
+    //         acc[e.name] = e.value
+    //         return acc
+    //     }, {})
+    //     updates.pushDifferences({ env: currentEnv }, { env: newEnv })
+
+    //     // perform update & log
+    //     if (updates.length === 0) {
+    //         return // nothing to do
+    //     }
+    //     deviceGroup.settings = {
+    //         ...deviceGroup.settings,
+    //         env: settings.env
+    //     }
+    //     await deviceGroup.save()
+
+    //     await deviceGroupLogger.application.deviceGroup.settings.updated(user, null, deviceGroup.Application, deviceGroup, updates)
+    // },
+
     /**
      * 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

From 192f9d309a955b98f9d50ef09bea027bbe666ef0 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:42:50 +0100
Subject: [PATCH 07/19] Add device group settings to view

---
 forge/db/views/DeviceGroup.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/forge/db/views/DeviceGroup.js b/forge/db/views/DeviceGroup.js
index 9f678109eb..f88ae3b624 100644
--- a/forge/db/views/DeviceGroup.js
+++ b/forge/db/views/DeviceGroup.js
@@ -97,6 +97,7 @@ module.exports = function (app) {
                 application: item.Application ? app.db.views.Application.applicationSummary(item.Application) : null,
                 deviceCount: item.deviceCount || 0,
                 devices: item.Devices ? item.Devices.map(app.db.views.Device.device) : [],
+                settings: item.settings,
                 targetSnapshot: app.db.views.ProjectSnapshot.snapshotSummary(item.targetSnapshot)
             }
             return filtered

From dc4f010ff87c50f6d2783dd1223cfdfe0bd0ef91 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:44:50 +0100
Subject: [PATCH 08/19] Add route for device group settings update

---
 .../routes/applicationDeviceGroups/index.js   | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/forge/ee/routes/applicationDeviceGroups/index.js b/forge/ee/routes/applicationDeviceGroups/index.js
index c8c80a6644..87f45a0147 100644
--- a/forge/ee/routes/applicationDeviceGroups/index.js
+++ b/forge/ee/routes/applicationDeviceGroups/index.js
@@ -10,6 +10,7 @@
 const { ValidationError } = require('sequelize')
 
 const { UpdatesCollection } = require('../../../auditLog/formatters.js')
+const { Roles } = require('../../../lib/roles.js')
 const { DeviceGroupMembershipValidationError } = require('../../db/controllers/DeviceGroup.js')
 
 // Declare getLogger function to provide type hints / quick code nav / code completion
@@ -371,6 +372,80 @@ module.exports = async function (app) {
         reply.send({})
     })
 
+    /**
+     * Update Device Group Settings (Environment Variables)
+     * @method PUT
+     * @name /api/v1/applications/:applicationId/device-groups/:groupId/settings
+     * @memberof forge.routes.api.application
+     */
+    app.put('/:groupId/settings', {
+        preHandler: app.needsPermission('application:device-group:update'), // re-use update permission (owner only)
+        schema: {
+            summary: 'Update a Device Group Settings',
+            tags: ['Application Device Groups'],
+            body: {
+                type: 'object',
+                properties: {
+                    env: { type: 'array', items: { type: 'object', additionalProperties: true } }
+                }
+            },
+            params: {
+                type: 'object',
+                properties: {
+                    applicationId: { type: 'string' },
+                    groupId: { type: 'string' }
+                }
+            },
+            response: {
+                200: {
+                    $ref: 'APIStatus'
+                },
+                '4xx': {
+                    $ref: 'APIError'
+                }
+            }
+        }
+    }, async (request, reply) => {
+        /** @type {Model} */
+        const deviceGroup = request.deviceGroup
+        const isMember = request.teamMembership.role === Roles.Member
+        /** @type {import('../../db/controllers/DeviceGroup.js')} */
+        const deviceGroupController = app.db.controllers.DeviceGroup
+        try {
+            let bodySettings
+            if (isMember) {
+                bodySettings = {
+                    env: request.body.env
+                }
+            } else {
+                bodySettings = request.body
+            }
+
+            // for audit log
+            const updates = new app.auditLog.formatters.UpdatesCollection()
+            // transform the env arrays to a map for better logging format
+            const currentEnv = (deviceGroup.settings?.env || []).reduce((acc, e) => {
+                acc[e.name] = e.value
+                return acc
+            }, {})
+            const newEnv = bodySettings.env.reduce((acc, e) => {
+                acc[e.name] = e.value
+                return acc
+            }, {})
+            updates.pushDifferences({ env: currentEnv }, { env: newEnv })
+
+            // perform update & log
+            await deviceGroupController.updateDeviceGroup(deviceGroup, { settings: bodySettings })
+            if (updates.length > 0) {
+                await deviceGroupLogger.settings.updated(request.session.User, null, request.application, deviceGroup, updates)
+            }
+
+            reply.send({})
+        } catch (err) {
+            return handleError(err, reply)
+        }
+    })
+
     function handleError (err, reply) {
         let statusCode = 500
         let code = 'unexpected_error'

From 3790772318cb1ba56e57aa15d7b0cb4bdc6f7b3c Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:46:19 +0100
Subject: [PATCH 09/19] Merge group env into device live settings api response

---
 forge/routes/api/deviceLive.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/forge/routes/api/deviceLive.js b/forge/routes/api/deviceLive.js
index b4239ff85b..b031b2b4cd 100644
--- a/forge/routes/api/deviceLive.js
+++ b/forge/routes/api/deviceLive.js
@@ -217,7 +217,9 @@ module.exports = async function (app) {
             hash: request.device.settingsHash,
             env: {}
         }
-        const settings = await request.device.getAllSettings()
+        const settings = await request.device.getAllSettings({
+            mergeDeviceGroupSettings: true
+        })
         Object.keys(settings).forEach(key => {
             if (key === 'env') {
                 settings.env.forEach(envVar => {

From 6bf7127984bdd3d4f123cd4776c66f94bd7b9401 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 16:47:26 +0100
Subject: [PATCH 10/19] add unit tests for device group settings

---
 test/unit/forge/auditLog/application_spec.js  |  18 ++
 .../api/applicationDeviceGroups_spec.js       | 219 ++++++++++++++++++
 2 files changed, 237 insertions(+)

diff --git a/test/unit/forge/auditLog/application_spec.js b/test/unit/forge/auditLog/application_spec.js
index 93015481d8..6ad759d932 100644
--- a/test/unit/forge/auditLog/application_spec.js
+++ b/test/unit/forge/auditLog/application_spec.js
@@ -278,5 +278,23 @@ describe('Audit Log > Application', async function () {
         logEntry.body.info.should.have.property('info', info)
     })
 
+    it('Provides a logger for updating device group settings', async function () {
+        const updates = new UpdatesCollection()
+        updates.pushDifferences({ name: 'before' }, { name: 'after' })
+        await logger.application.deviceGroup.settings.updated(ACTIONED_BY, null, APPLICATION, DEVICEGROUP, updates)
+
+        // check log stored
+        const logEntry = await getLog()
+        logEntry.should.have.property('event', 'application.deviceGroup.settings.updated')
+        logEntry.should.have.property('scope', { id: APPLICATION.hashid, type: 'application' })
+        logEntry.should.have.property('trigger', { id: ACTIONED_BY.hashid, type: 'user', name: ACTIONED_BY.username })
+        logEntry.should.have.property('body')
+        logEntry.body.should.only.have.keys('application', 'deviceGroup', 'updates')
+        logEntry.body.application.should.only.have.keys('id', 'name')
+        logEntry.body.application.id.should.equal(APPLICATION.id)
+        logEntry.body.deviceGroup.should.only.have.keys('id', 'name')
+        logEntry.body.updates.should.be.an.Array().and.have.length(1)
+    })
+
     // #endregion
 })
diff --git a/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js b/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js
index cb01e56902..99c4458aa3 100644
--- a/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js
+++ b/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js
@@ -580,6 +580,225 @@ describe('Application Device Groups API', function () {
         })
     })
 
+    describe('Update Device Settings', async function () {
+        async function prepare () {
+            const sid = await login('bob', 'bbPassword')
+            const application = await factory.createApplication({ name: generateName('app') }, TestObjects.BTeam)
+            const deviceGroup = await factory.createApplicationDeviceGroup({ name: generateName('device-group') + ' original name', description: 'original desc' }, application)
+            deviceGroup.should.have.property('name').and.endWith('original name')
+            deviceGroup.should.have.property('description', 'original desc')
+
+            const device1of2 = await factory.createDevice({ name: generateName('device 1') }, TestObjects.BTeam, null, application)
+            const device2of2 = await factory.createDevice({ name: generateName('device 2') }, TestObjects.BTeam, null, application)
+
+            // add the devices to the group
+            await controller.updateDeviceGroupMembership(deviceGroup, { addDevices: [device1of2.id, device2of2.id] })
+            await device1of2.reload({ include: [app.db.models.Team, app.db.models.Application] })
+            await device2of2.reload({ include: [app.db.models.Team, app.db.models.Application] })
+
+            // manually set some env-vars on the group
+            deviceGroup.settings = {
+                env: [{ name: 'ENV1', value: 'group value' }, { name: 'ENV2', value: 'group value' }, { name: 'ENV3', value: 'group value' }]
+            }
+            await deviceGroup.save()
+
+            // set some env-vars on the devices
+            await device1of2.updateSettings({ env: [{ name: 'ENV1', value: 'device 1 value 1' }, { name: 'ENV2', value: 'device 1 value 2' }] })
+            await device2of2.updateSettings({ env: [{ name: 'ENV1', value: 'device 2 value 2' }, { name: 'ENV2', value: '' }] })
+
+            const tokens = {
+                device1: await device1of2.refreshAuthTokens({ refreshOTC: false }),
+                device2: await device2of2.refreshAuthTokens({ refreshOTC: false })
+            }
+
+            // reset the mock call history
+            app.comms.devices.sendCommand.resetHistory()
+
+            return { sid, tokens, application, deviceGroup, device1of2, device2of2, device1SettingsHash: device1of2.settingsHash, device2SettingsHash: device2of2.settingsHash }
+        }
+        /**
+         * Call the API to update the device group settings
+         * e.g http://xxxxx/api/v1/applications/ZdEbXMg9mD/device-groups/50z9ynpNdO/settings
+         * @param {*} sid - session id
+         * @param {Object} application - application object
+         * @param {Object} deviceGroup - device group object
+         * @param {{ env: [{name: String, value: String}]}} settings - settings to update
+         * @returns {Promise} - the response object
+         */
+        async function callSettingsUpdate (sid, application, deviceGroup, settings) {
+            return app.inject({
+                method: 'PUT',
+                url: `/api/v1/applications/${application.hashid}/device-groups/${deviceGroup.hashid}/settings`,
+                cookies: { sid },
+                payload: settings
+            })
+        }
+        /** /api/v1/devices/:deviceId/live/settings */
+        async function callDeviceAgentLiveSettingsAPI (token, device) {
+            return app.inject({
+                method: 'GET',
+                url: `/api/v1/devices/${device.hashid}/live/settings`,
+                headers: {
+                    authorization: `Bearer ${token}`,
+                    'content-type': 'application/json'
+                }
+            })
+        }
+
+        it('Owner can update a device group settings', async function () {
+            const { sid, tokens, application, deviceGroup, device1of2, device2of2, device1SettingsHash, device2SettingsHash } = await prepare()
+
+            const groupEnv = deviceGroup.settings.env.map(e => ({ ...e })) // clone the group env vars before modifying
+            groupEnv.find(e => e.name === 'ENV3').value = 'group value updated' // update the value of ENV3
+
+            // reset the mock call history
+            app.comms.devices.sendCommand.resetHistory()
+
+            // update an env var that is not overridden by the devices
+            const response = await callSettingsUpdate(sid, application, deviceGroup, {
+                env: groupEnv
+            })
+            response.statusCode.should.equal(200) // ensure success
+
+            // check the group env is updated
+            const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid)
+            updatedDeviceGroup.should.have.property('settings').and.be.an.Object()
+            updatedDeviceGroup.settings.should.have.property('env').and.be.an.Array()
+            const gEnv3 = updatedDeviceGroup.settings.env.find(e => e.name === 'ENV3')
+            gEnv3.should.have.property('value', 'group value updated')
+
+            // check device settings hash has changed
+            const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid)
+            const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid)
+            updatedDevice1.should.have.property('settingsHash').and.not.equal(device1SettingsHash) // device 1 should have a new settings hash
+            updatedDevice2.should.have.property('settingsHash').and.not.equal(device2SettingsHash) // device 2 should have a new settings hash
+
+            // check the settings.env delivered to the devices are merged with the group settings
+            const d1SettingsResponse = await callDeviceAgentLiveSettingsAPI(tokens.device1.token, updatedDevice1)
+            const d1Settings = d1SettingsResponse.json()
+            d1Settings.should.have.property('hash', updatedDevice1.settingsHash)
+            d1Settings.should.have.property('env').and.be.an.Object()
+            d1Settings.env.should.have.property('ENV1', 'device 1 value 1') // device value should not be changed
+            d1Settings.env.should.have.property('ENV2', 'device 1 value 2') // device value should not be changed
+            d1Settings.env.should.have.property('ENV3', 'group value updated') // device value should be overridden by the updated group value since ENV3 is not set on the device
+
+            const d2SettingsResponse = await callDeviceAgentLiveSettingsAPI(tokens.device2.token, updatedDevice2)
+            const d2Settings = d2SettingsResponse.json()
+            d2Settings.should.have.property('hash', updatedDevice2.settingsHash)
+            d2Settings.should.have.property('env').and.be.an.Object()
+            d2Settings.env.should.have.property('ENV1', 'device 2 value 2') // device value should not be changed
+            d2Settings.env.should.have.property('ENV2', 'group value') // device value should be overridden by original group value (since it is empty on the device)
+            d2Settings.env.should.have.property('ENV3', 'group value updated') // device value should be overridden by the updated group value since ENV3 is not set on the device
+
+            // check devices got an update request
+            app.comms.devices.sendCommand.callCount.should.equal(2)
+            const calls = app.comms.devices.sendCommand.getCalls()
+            checkDeviceUpdateCall(calls, device1of2)
+            checkDeviceUpdateCall(calls, device2of2)
+        })
+
+        it('Merges new Group Env Var', async function () {
+            const { sid, tokens, application, deviceGroup, device1of2, device2of2, device1SettingsHash, device2SettingsHash } = await prepare()
+
+            const groupEnv = deviceGroup.settings.env.map(e => ({ ...e })) // clone the group env vars before modifying
+            groupEnv.push({ name: 'ENV4', value: 'group value 4' }) // add a new env var
+
+            // update an env var that is not overridden by the devices
+            const response = await callSettingsUpdate(sid, application, deviceGroup, {
+                env: groupEnv
+            })
+            response.statusCode.should.equal(200) // ensure success
+
+            // check the group env is updated
+            const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid)
+            updatedDeviceGroup.should.have.property('settings').and.be.an.Object()
+            updatedDeviceGroup.settings.should.have.property('env').and.be.an.Array()
+            updatedDeviceGroup.settings.env.length.should.equal(4)
+            const gEnv4 = updatedDeviceGroup.settings.env.find(e => e.name === 'ENV4')
+            gEnv4.should.have.property('value', 'group value 4')
+
+            // check device settings hash has changed
+            const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid)
+            const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid)
+            updatedDevice1.should.have.property('settingsHash').and.not.equal(device1SettingsHash)
+            updatedDevice2.should.have.property('settingsHash').and.not.equal(device2SettingsHash)
+
+            // check the settings.env delivered to the devices are merged with the group settings
+            const d1SettingsResponse = await callDeviceAgentLiveSettingsAPI(tokens.device1.token, updatedDevice1)
+            const d1Settings = d1SettingsResponse.json()
+            d1Settings.should.have.property('hash', updatedDevice1.settingsHash)
+            d1Settings.env.should.have.keys('ENV1', 'ENV2', 'ENV3', 'ENV4')
+            d1Settings.env.should.have.property('ENV4', 'group value 4') // device should inherit the new group value
+
+            const d2SettingsResponse = await callDeviceAgentLiveSettingsAPI(tokens.device2.token, updatedDevice2)
+            const d2Settings = d2SettingsResponse.json()
+            d2Settings.should.have.property('hash', updatedDevice2.settingsHash)
+            d2Settings.env.should.have.keys('ENV1', 'ENV2', 'ENV3', 'ENV4')
+            d2Settings.env.should.have.property('ENV4', 'group value 4') // device should inherit the new group value
+        })
+
+        it('Only updates a device settings hash if the group env var change affects the merged settings env', async function () {
+            const { sid, application, deviceGroup, device1of2, device2of2, device1SettingsHash, device2SettingsHash } = await prepare()
+
+            // reset the mock call history
+            app.comms.devices.sendCommand.resetHistory()
+
+            const groupEnv = deviceGroup.settings.env.map(e => ({ ...e })) // clone the group env vars before modifying
+            groupEnv.find(e => e.name === 'ENV2').value = 'device 1 value 2' // set the value of ENV2 to the same as the device 1s current value
+            const response = await callSettingsUpdate(sid, application, deviceGroup, {
+                env: groupEnv
+            })
+            response.statusCode.should.equal(200) // ensure success
+
+            // check the group env is updated
+            const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid)
+            updatedDeviceGroup.should.have.property('settings').and.be.an.Object()
+            updatedDeviceGroup.settings.should.have.property('env').and.be.an.Array()
+            const gEnv2 = updatedDeviceGroup.settings.env.find(e => e.name === 'ENV2')
+            gEnv2.should.have.property('value', 'device 1 value 2') // the group value should now be the same as the device 1 value
+
+            // check device 1 settings hash has NOT changed but device 2 has!
+            const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid)
+            const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid)
+            updatedDevice1.should.have.property('settingsHash').and.equal(device1SettingsHash)
+            updatedDevice2.should.have.property('settingsHash').and.not.equal(device2SettingsHash)
+
+            // check devices got an update request
+            app.comms.devices.sendCommand.callCount.should.equal(2)
+            const calls = app.comms.devices.sendCommand.getCalls()
+            checkDeviceUpdateCall(calls, device1of2)
+            checkDeviceUpdateCall(calls, device2of2)
+        })
+
+        it('Member can not update a device group settings (403)', async function () {
+            const sid = await login('chris', 'ccPassword')
+            const application = await factory.createApplication({ name: generateName('app') }, TestObjects.BTeam)
+            const deviceGroup = await factory.createApplicationDeviceGroup({ name: generateName('device-group') }, application)
+            const response = await callSettingsUpdate(sid, application, deviceGroup, {
+                env: [{ name: 'ENV1', value: 'new group value' }]
+            })
+
+            response.statusCode.should.equal(403)
+
+            const result = response.json()
+            result.should.have.property('code', 'unauthorized')
+            result.should.have.property('error')
+        })
+
+        it('Non Member can not update a device group settings (404)', async function () {
+            const sid = await login('dave', 'ddPassword')
+            const application = await factory.createApplication({ name: generateName('app') }, TestObjects.BTeam)
+            const deviceGroup = await factory.createApplicationDeviceGroup({ name: generateName('device-group') }, application)
+            const response = await callSettingsUpdate(sid, application, deviceGroup, {
+                name: 'updated name',
+                description: 'updated description',
+                targetSnapshotId: null
+            })
+
+            response.statusCode.should.be.equal(404)
+        })
+    })
+
     describe('Delete Device Group', async function () {
         it('Owner can delete a device group', async function () {
             const sid = await login('bob', 'bbPassword')

From e4354436b250a9e684ec3d3e29b9ae06258668c1 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 17:09:02 +0100
Subject: [PATCH 11/19] add frontend API for updating dev group env vars

---
 frontend/src/api/application.js | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/frontend/src/api/application.js b/frontend/src/api/application.js
index 22919a28b5..159021160f 100644
--- a/frontend/src/api/application.js
+++ b/frontend/src/api/application.js
@@ -342,6 +342,17 @@ const updateDeviceGroupMembership = async (applicationId, groupId, { add, remove
     return client.patch(`/api/v1/applications/${applicationId}/device-groups/${groupId}`, { add, remove, set })
 }
 
+/**
+ * Update the settings of a device group
+ * NOTE: Only ENV VARS are supported at the moment
+ * @param {string} applicationId - The ID of application
+ * @param {string} groupId - The ID of the group
+ * @param {{ env: [{name: String, value: String}]}} settings - The new settings for the group
+ */
+const updateDeviceGroupSettings = async (applicationId, groupId, settings) => {
+    return client.put(`/api/v1/applications/${applicationId}/device-groups/${groupId}/settings`, settings)
+}
+
 /**
  * Get a list of Dependencies / Bill of Materials
  * @param applicationId
@@ -373,5 +384,6 @@ export default {
     deleteDeviceGroup,
     updateDeviceGroup,
     updateDeviceGroupMembership,
+    updateDeviceGroupSettings,
     getDependencies
 }

From 7e90dd6334466e40dd7e9eec0c7b766cb1c34c6a Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 17:13:27 +0100
Subject: [PATCH 12/19] Add audit log entry for group settings updated

---
 frontend/src/components/audit-log/AuditEntryIcon.vue    | 3 ++-
 frontend/src/components/audit-log/AuditEntryVerbose.vue | 7 ++++++-
 frontend/src/data/audit-events.json                     | 3 ++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/frontend/src/components/audit-log/AuditEntryIcon.vue b/frontend/src/components/audit-log/AuditEntryIcon.vue
index 465430c6e2..af22649118 100644
--- a/frontend/src/components/audit-log/AuditEntryIcon.vue
+++ b/frontend/src/components/audit-log/AuditEntryIcon.vue
@@ -208,7 +208,8 @@ const iconMap = {
         'application.deviceGroup.created',
         'application.deviceGroup.updated',
         'application.deviceGroup.deleted',
-        'application.deviceGroup.members.changed'
+        'application.deviceGroup.members.changed',
+        'application.deviceGroup.settings.updated'
     ],
     beaker: [
         'team.device.developer-mode.enabled',
diff --git a/frontend/src/components/audit-log/AuditEntryVerbose.vue b/frontend/src/components/audit-log/AuditEntryVerbose.vue
index b5bb65c53f..ae3b32e15f 100644
--- a/frontend/src/components/audit-log/AuditEntryVerbose.vue
+++ b/frontend/src/components/audit-log/AuditEntryVerbose.vue
@@ -441,7 +441,12 @@
     </template>
     <template v-else-if="entry.event === 'application.deviceGroup.members.changed'">
         <label>{{ AuditEvents[entry.event] }}</label>
-        <span v-if="!error && entry.body?.deviceGroup">Device Group '{{ entry.body.deviceGroup?.name }}' members in Application '{{ entry.body.application?.name }} updated: {{ entry.body?.info?.info ?? 'No changes' }}.</span>
+        <span v-if="!error && entry.body?.deviceGroup">Device Group '{{ entry.body.deviceGroup?.name }}' members in Application '{{ entry.body.application?.name }}' updated: {{ entry.body?.info?.info ?? 'No changes' }}.</span>
+        <span v-else-if="!error">Device Group data not found in audit entry.</span>
+    </template>
+    <template v-else-if="entry.event === 'application.deviceGroup.settings.updated'">
+        <label>{{ AuditEvents[entry.event] }}</label>
+        <span v-if="!error && entry.body?.deviceGroup">Device Group '{{ entry.body.deviceGroup?.name }}' settings in Application '{{ entry.body.application?.name }}' updated.</span>
         <span v-else-if="!error">Device Group data not found in audit entry.</span>
     </template>
 
diff --git a/frontend/src/data/audit-events.json b/frontend/src/data/audit-events.json
index da43468996..347fa6348d 100644
--- a/frontend/src/data/audit-events.json
+++ b/frontend/src/data/audit-events.json
@@ -55,7 +55,8 @@
         "application.deviceGroup.created": "Device Group Created",
         "application.deviceGroup.updated": "Device Group Updated",
         "application.deviceGroup.deleted": "Device Group Deleted",
-        "application.deviceGroup.members.changed": "Device Group Members Changed"
+        "application.deviceGroup.members.changed": "Device Group Members Changed",
+        "application.deviceGroup.settings.updated": "Device Group Settings Updated"
     },
     "project": {
         "project.created": "Instance Created",

From 019a18c0942367f9a538f664282c07ee8fef1e1b Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 17:29:24 +0100
Subject: [PATCH 13/19] re-organise settings to sub-tab of device group
 settings

---
 .../{settings.vue => Settings/General.vue}    | 14 ++--
 .../DeviceGroup/Settings/index.vue            | 68 +++++++++++++++++++
 .../pages/application/DeviceGroup/index.vue   |  3 +-
 3 files changed, 76 insertions(+), 9 deletions(-)
 rename frontend/src/pages/application/DeviceGroup/{settings.vue => Settings/General.vue} (94%)
 create mode 100644 frontend/src/pages/application/DeviceGroup/Settings/index.vue

diff --git a/frontend/src/pages/application/DeviceGroup/settings.vue b/frontend/src/pages/application/DeviceGroup/Settings/General.vue
similarity index 94%
rename from frontend/src/pages/application/DeviceGroup/settings.vue
rename to frontend/src/pages/application/DeviceGroup/Settings/General.vue
index fe2a455481..cbefb077ee 100644
--- a/frontend/src/pages/application/DeviceGroup/settings.vue
+++ b/frontend/src/pages/application/DeviceGroup/Settings/General.vue
@@ -37,15 +37,15 @@
 <script>
 import { mapState } from 'vuex'
 
-import ApplicationApi from '../../../api/application.js'
-import FormHeading from '../../../components/FormHeading.vue'
-import FormRow from '../../../components/FormRow.vue'
-import permissionsMixin from '../../../mixins/Permissions.js'
-import Alerts from '../../../services/alerts.js'
-import Dialog from '../../../services/dialog.js'
+import ApplicationApi from '../../../../api/application.js'
+import FormHeading from '../../../../components/FormHeading.vue'
+import FormRow from '../../../../components/FormRow.vue'
+import permissionsMixin from '../../../../mixins/Permissions.js'
+import Alerts from '../../../../services/alerts.js'
+import Dialog from '../../../../services/dialog.js'
 
 export default {
-    name: 'ApplicationDeviceGroupSettings',
+    name: 'ApplicationDeviceGroupSettingsGeneral',
     components: {
         FormRow,
         FormHeading
diff --git a/frontend/src/pages/application/DeviceGroup/Settings/index.vue b/frontend/src/pages/application/DeviceGroup/Settings/index.vue
new file mode 100644
index 0000000000..376ca4654d
--- /dev/null
+++ b/frontend/src/pages/application/DeviceGroup/Settings/index.vue
@@ -0,0 +1,68 @@
+<template>
+    <div class="flex flex-col sm:flex-row">
+        <SectionSideMenu :options="sideNavigation" />
+        <div class="flex-grow">
+            <router-view :deviceGroup="deviceGroup" :application="application" @device-group-updated="onDeviceGroupUpdated" />
+        </div>
+    </div>
+</template>
+
+<script>
+import { useRouter } from 'vue-router'
+import { mapState } from 'vuex'
+
+import SectionSideMenu from '../../../../components/SectionSideMenu.vue'
+
+import permissionsMixin from '../../../../mixins/Permissions.js'
+
+export default {
+    name: 'DeviceGroupSettings',
+    components: {
+        SectionSideMenu
+    },
+    mixins: [permissionsMixin],
+    props: {
+        application: {
+            type: Object,
+            required: true
+        },
+        deviceGroup: {
+            type: Object,
+            required: true
+        }
+    },
+    emits: ['device-group-updated'],
+    data: function () {
+        return {
+            sideNavigation: []
+        }
+    },
+    computed: {
+        ...mapState('account', ['teamMembership', 'team'])
+    },
+    watch: {
+        deviceGroup: function (newVal, oldVal) {
+            this.checkAccess()
+        }
+    },
+    mounted () {
+        this.checkAccess()
+    },
+    methods: {
+        checkAccess: async function () {
+            if (!this.teamMembership) {
+                useRouter().push({ replace: true, path: 'overview' })
+                return false
+            }
+            this.sideNavigation = [
+                { name: 'General', path: './general' },
+                { name: 'Environment', path: './environment' }
+            ]
+            return true
+        },
+        onDeviceGroupUpdated: function () {
+            this.$emit('device-group-updated')
+        }
+    }
+}
+</script>
diff --git a/frontend/src/pages/application/DeviceGroup/index.vue b/frontend/src/pages/application/DeviceGroup/index.vue
index 5688a40b58..2df9b7c1c4 100644
--- a/frontend/src/pages/application/DeviceGroup/index.vue
+++ b/frontend/src/pages/application/DeviceGroup/index.vue
@@ -116,7 +116,7 @@ export default {
                 {
                     label: 'Settings',
                     to: {
-                        name: 'ApplicationDeviceGroupSettings',
+                        name: 'ApplicationDeviceGroupSettingsGeneral',
                         params: {
                             applicationId: this.application?.id,
                             deviceGroupId: this.deviceGroup?.id
@@ -154,7 +154,6 @@ export default {
     methods: {
         async load () {
             const applicationId = this.$route.params.applicationId
-
             // See https://github.com/FlowFuse/flowfuse/issues/2929
             if (!applicationId) {
                 return

From 4012f3858c96c9d27f14dbc6d8a48ffde1e82633 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 17:29:45 +0100
Subject: [PATCH 14/19] Support info section on env vars component

---
 .../admin/Template/sections/Environment.vue   | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/frontend/src/pages/admin/Template/sections/Environment.vue b/frontend/src/pages/admin/Template/sections/Environment.vue
index 2878cc287d..5239df8ddb 100644
--- a/frontend/src/pages/admin/Template/sections/Environment.vue
+++ b/frontend/src/pages/admin/Template/sections/Environment.vue
@@ -3,9 +3,21 @@
         <FormHeading>
             <div class="flex">
                 <div class="mr-4">Environment Variables</div>
+                <div v-if="hasInfoDialog" class="flex justify-center mr-4"><InformationCircleIcon class="w-5 cursor-pointer hover:text-blue-700" @click="openInfoDialog()" /></div>
                 <div class="flex justify-center"><ChangeIndicator :value="editable.changed.env" /></div>
             </div>
         </FormHeading>
+        <ff-dialog v-if="hasInfoDialog" ref="help-dialog" class="ff-dialog-box--info" :header="helpHeader || 'FlowFuse Info'">
+            <template #default>
+                <div class="flex gap-8">
+                    <slot name="pictogram"><img src="../../../../images/pictograms/snapshot_red.png"></slot>
+                    <div><slot name="helptext" /></div>
+                </div>
+            </template>
+            <template #actions>
+                <ff-button @click="$refs['help-dialog'].close()">Close</ff-button>
+            </template>
+        </ff-dialog>
         <div class="min-w-min">
             <!-- NOTE:  `:columns:[,,,]` is necessary to instruct the empty row to apply a col-span of 4 -->
             <ff-data-table
@@ -100,8 +112,7 @@
 </template>
 
 <script>
-
-import { DocumentDownloadIcon, ExclamationIcon, LockClosedIcon, PlusSmIcon, TrashIcon } from '@heroicons/vue/outline'
+import { DocumentDownloadIcon, ExclamationIcon, InformationCircleIcon, LockClosedIcon, PlusSmIcon, TrashIcon } from '@heroicons/vue/outline'
 
 import FormHeading from '../../../../components/FormHeading.vue'
 import FormRow from '../../../../components/FormRow.vue'
@@ -121,7 +132,8 @@ export default {
         TrashIcon,
         PlusSmIcon,
         LockClosedIcon,
-        ExclamationIcon
+        ExclamationIcon,
+        InformationCircleIcon
     },
     props: {
         editTemplate: {
@@ -135,6 +147,11 @@ export default {
         readOnly: {
             type: Boolean,
             default: false
+        },
+        helpHeader: {
+            // for the dialog that opens, e.g. "FlowFuse - Device Group Environment Variables"
+            type: String,
+            default: null
         }
     },
     emits: ['update:modelValue'],
@@ -164,6 +181,9 @@ export default {
         },
         noDataMessage: function () {
             return this.search === '' ? 'No environment variables' : 'Not found! Try a different search term.'
+        },
+        hasInfoDialog () {
+            return !!this.$slots.helptext
         }
     },
     watch: {
@@ -196,6 +216,9 @@ export default {
         this.validate()
     },
     methods: {
+        openInfoDialog () {
+            this.$refs['help-dialog'].show()
+        },
         validate () {
             const envVars = this.editable?.settings?.env || []
             this.updateLookup()

From e009deaf189d475008b29a2107de308edcb6123a Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 17:30:03 +0100
Subject: [PATCH 15/19] update routes to reflect new layout

---
 frontend/src/pages/application/routes.js | 28 ++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/frontend/src/pages/application/routes.js b/frontend/src/pages/application/routes.js
index a33c3a1d82..588eb124b2 100644
--- a/frontend/src/pages/application/routes.js
+++ b/frontend/src/pages/application/routes.js
@@ -6,9 +6,11 @@
  */
 import ApplicationActivity from './Activity.vue'
 import Dependencies from './Dependencies/Dependencies.vue'
+import ApplicationDeviceGroupSettingsEnvironment from './DeviceGroup/Settings/Environment.vue'
+import ApplicationDeviceGroupSettingsGeneral from './DeviceGroup/Settings/General.vue'
+import ApplicationDeviceGroupSettings from './DeviceGroup/Settings/index.vue'
 import ApplicationDeviceGroupDevices from './DeviceGroup/devices.vue'
 import ApplicationDeviceGroupIndex from './DeviceGroup/index.vue'
-import ApplicationDeviceGroupSettings from './DeviceGroup/settings.vue'
 import ApplicationDeviceGroups from './DeviceGroups.vue'
 import ApplicationDevices from './Devices.vue'
 import ApplicationLogs from './Logs.vue'
@@ -183,7 +185,29 @@ export default [
                 component: ApplicationDeviceGroupSettings,
                 meta: {
                     title: 'Application - Device Group - Settings'
-                }
+                },
+                redirect: to => {
+                    return `/application/${to.params.applicationId}/device-group/${to.params.deviceGroupId}/settings/general`
+                },
+                children: [
+                    {
+                        path: 'general',
+                        name: 'ApplicationDeviceGroupSettingsGeneral',
+                        component: ApplicationDeviceGroupSettingsGeneral,
+                        meta: {
+                            title: 'Application - Device Group - Settings - General'
+                        }
+                    },
+                    {
+                        path: 'environment',
+                        name: 'ApplicationDeviceGroupSettingsEnvironment',
+                        component: ApplicationDeviceGroupSettingsEnvironment,
+                        meta: {
+                            title: 'Application - Device Group - Settings - Environment'
+                        }
+                    }
+
+                ]
             }
         ]
     }

From 11068047dba836d664cf941adbdbe7cf5280abd1 Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 17:30:22 +0100
Subject: [PATCH 16/19] Add Env Var Editor for device group

---
 .../DeviceGroup/Settings/Environment.vue      | 179 ++++++++++++++++++
 1 file changed, 179 insertions(+)
 create mode 100644 frontend/src/pages/application/DeviceGroup/Settings/Environment.vue

diff --git a/frontend/src/pages/application/DeviceGroup/Settings/Environment.vue b/frontend/src/pages/application/DeviceGroup/Settings/Environment.vue
new file mode 100644
index 0000000000..15a123e736
--- /dev/null
+++ b/frontend/src/pages/application/DeviceGroup/Settings/Environment.vue
@@ -0,0 +1,179 @@
+<template>
+    <form class="space-y-6">
+        <TemplateSettingsEnvironment v-model="editable" :readOnly="!hasPermission('device:edit-env')" :editTemplate="false" :helpHeader="'Device Group Environment Variables'">
+            <template #helptext>
+                <p>Environment variables entered here will be merged with the environment variables defined in the member devices.</p>
+                <p>
+                    The following rules apply:
+                    <ul class="list-disc pl-5">
+                        <li>Values set in the Device take precedence over values set in the Device Group.</li>
+                        <li>Removing a device from the group will remove these variables from the device.</li>
+                        <li>The devices environment variables are never modified, they are only merged at runtime.</li>
+                    </ul>
+                </p>
+                <p>Updating these environment variables will cause devices in the group to be restarted when a change is detected.</p>
+            </template>
+        </TemplateSettingsEnvironment>
+        <div v-if="hasPermission('device:edit-env')" class="space-x-4 whitespace-nowrap">
+            <ff-button size="small" :disabled="!unsavedChanges || hasError" @click="saveSettings()">Save Settings</ff-button>
+        </div>
+        <div class="ff-banner ff-banner-warning w-full max-w-5xl">
+            <span>
+                <ExclamationCircleIcon class="ff-icon mr-2" />
+                <span class="relative top-0.5">Note: Updating environment variables can cause devices in the group to be restarted.</span>
+            </span>
+
+            <ChevronRightIcon class="ff-icon align-self-right" />
+        </div>
+    </form>
+</template>
+
+<script>
+import { ExclamationCircleIcon } from '@heroicons/vue/outline'
+import { mapState } from 'vuex'
+
+import applicationApi from '../../../../api/application.js'
+import permissionsMixin from '../../../../mixins/Permissions.js'
+import alerts from '../../../../services/alerts.js'
+import dialog from '../../../../services/dialog.js'
+import TemplateSettingsEnvironment from '../../../admin/Template/sections/Environment.vue'
+
+/**
+ * @typedef {Object} EnvVar
+ * @property {string} name
+ * @property {string} value
+ */
+
+export default {
+    name: 'ApplicationDeviceGroupSettingsEnvironment',
+    components: {
+        ExclamationCircleIcon,
+        TemplateSettingsEnvironment
+    },
+    mixins: [permissionsMixin],
+    beforeRouteLeave: async function (_to, _from, next) {
+        if (this.unsavedChanges) {
+            const dialogOpts = {
+                header: 'Unsaved changes',
+                kind: 'danger',
+                text: 'You have unsaved changes. Are you sure you want to leave?',
+                confirmLabel: 'Yes, lose changes'
+            }
+            const answer = await dialog.showAsync(dialogOpts)
+            if (answer === 'confirm') {
+                next()
+            } else {
+                next(false)
+            }
+        } else {
+            next()
+        }
+    },
+    props: {
+        application: {
+            type: Object,
+            required: true
+        },
+        deviceGroup: {
+            type: Object,
+            required: true
+        }
+    },
+    emits: ['device-group-updated'],
+    data () {
+        return {
+            hasError: false,
+            editable: {
+                name: '',
+                settings: {
+                    /** @type {EnvVar[]} */ env: []
+                },
+                policy: {},
+                changed: {
+                    name: false,
+                    description: false,
+                    settings: {
+                        /** @type {EnvVar[]} */ env: []
+                    },
+                    env: false,
+                    policy: {}
+                },
+                errors: {}
+            },
+            original: {
+                settings: {
+                    /** @type {EnvVar[]} */ envMap: {}
+                }
+            },
+            templateEnvValues: {}
+        }
+    },
+    computed: {
+        ...mapState('account', ['teamMembership']),
+        unsavedChanges () {
+            return this.editable.changed.env
+        }
+    },
+    watch: {
+        deviceGroup: 'getSettings',
+        'editable.settings.env': {
+            deep: true,
+            handler (v) {
+                let changed = false
+                let error = false
+
+                this.editable.settings?.env.forEach(field => {
+                    // if we do not recognise the env var name from our original settings,
+                    // or if we do recognise it, but the value is different
+                    if (!this.original.settings.envMap[field.name] || field.value !== this.original.settings.envMap[field.name].value) {
+                        changed = true
+                    }
+                    // there is an issue with he key/value
+                    if (field.error) {
+                        error = true
+                    }
+                })
+
+                // some env vars have been deleted
+                if (this.editable.settings.env.length !== Object.keys(this.original.settings.envMap).length) {
+                    changed = true
+                }
+
+                this.hasError = error
+                this.editable.changed.env = changed
+            }
+        }
+    },
+    mounted () {
+        this.getSettings()
+    },
+    methods: {
+        getSettings: async function () {
+            if (this.deviceGroup) {
+                this.original.settings.envMap = {}
+                this.editable.settings.env = []
+                const settings = this.deviceGroup.settings
+                settings.env?.forEach(envVar => {
+                    this.editable.settings.env.push(Object.assign({}, envVar))
+                    // make a map of the key:value so it's easier to check for changes
+                    this.original.settings.envMap[envVar.name] = envVar
+                })
+            }
+        },
+        saveSettings: async function () {
+            const settings = {
+                env: []
+            }
+            this.editable.settings.env.forEach(field => {
+                settings.env.push({
+                    name: field.name,
+                    value: field.value
+                })
+            })
+            await applicationApi.updateDeviceGroupSettings(this.application.id, this.deviceGroup.id, settings)
+            this.$emit('device-group-updated')
+            alerts.emit('Device Group settings successfully updated. NOTE: changes will be applied to the devices in the group once they restart.', 'confirmation', 6000)
+        }
+    }
+}
+</script>

From 8ffe6ddad5a443c7a5d1b1532e00d0d60db0539a Mon Sep 17 00:00:00 2001
From: Steve-Mcl <sdmclaughlin@gmail.com>
Date: Fri, 18 Oct 2024 17:30:50 +0100
Subject: [PATCH 17/19] Add missing await - found while testing

---
 frontend/src/pages/device/Settings/Environment.vue | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frontend/src/pages/device/Settings/Environment.vue b/frontend/src/pages/device/Settings/Environment.vue
index f764521311..034367e4e4 100644
--- a/frontend/src/pages/device/Settings/Environment.vue
+++ b/frontend/src/pages/device/Settings/Environment.vue
@@ -125,7 +125,7 @@ export default {
                     value: field.value
                 })
             })
-            deviceApi.updateSettings(this.device.id, settings)
+            await deviceApi.updateSettings(this.device.id, settings)
             this.$emit('device-updated')
             alerts.emit('Device settings successfully updated. NOTE: changes will be applied once the device restarts.', 'confirmation', 6000)
         }

From c93366e0483411062e5545d4e3e4976b6c88d1b5 Mon Sep 17 00:00:00 2001
From: Joe Pavitt <joepavitt@flowforge.com>
Date: Mon, 21 Oct 2024 10:53:28 +0100
Subject: [PATCH 18/19] Dial back the note to an "info", and change button/note
 ordering and sizing

---
 .../application/DeviceGroup/Settings/Environment.vue      | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/frontend/src/pages/application/DeviceGroup/Settings/Environment.vue b/frontend/src/pages/application/DeviceGroup/Settings/Environment.vue
index 15a123e736..4365ede9e0 100644
--- a/frontend/src/pages/application/DeviceGroup/Settings/Environment.vue
+++ b/frontend/src/pages/application/DeviceGroup/Settings/Environment.vue
@@ -14,10 +14,7 @@
                 <p>Updating these environment variables will cause devices in the group to be restarted when a change is detected.</p>
             </template>
         </TemplateSettingsEnvironment>
-        <div v-if="hasPermission('device:edit-env')" class="space-x-4 whitespace-nowrap">
-            <ff-button size="small" :disabled="!unsavedChanges || hasError" @click="saveSettings()">Save Settings</ff-button>
-        </div>
-        <div class="ff-banner ff-banner-warning w-full max-w-5xl">
+        <div class="ff-banner ff-banner-info w-full max-w-5xl">
             <span>
                 <ExclamationCircleIcon class="ff-icon mr-2" />
                 <span class="relative top-0.5">Note: Updating environment variables can cause devices in the group to be restarted.</span>
@@ -25,6 +22,9 @@
 
             <ChevronRightIcon class="ff-icon align-self-right" />
         </div>
+        <div v-if="hasPermission('device:edit-env')" class="space-x-4 whitespace-nowrap">
+            <ff-button :disabled="!unsavedChanges || hasError" @click="saveSettings()">Save Settings</ff-button>
+        </div>
     </form>
 </template>
 

From f2aad783993211f47a918dd82d14a3951d8f181d Mon Sep 17 00:00:00 2001
From: Stephen McLaughlin <44235289+Steve-Mcl@users.noreply.github.com>
Date: Mon, 21 Oct 2024 12:05:19 +0100
Subject: [PATCH 19/19] Remove unused code

---
 forge/ee/db/controllers/DeviceGroup.js | 52 --------------------------
 1 file changed, 52 deletions(-)

diff --git a/forge/ee/db/controllers/DeviceGroup.js b/forge/ee/db/controllers/DeviceGroup.js
index 991fb21426..be9f9aae68 100644
--- a/forge/ee/db/controllers/DeviceGroup.js
+++ b/forge/ee/db/controllers/DeviceGroup.js
@@ -243,58 +243,6 @@ module.exports = {
         await app.db.models.Device.update({ DeviceGroupId: null }, { where: { id: deviceIds.removeList, DeviceGroupId: deviceGroup.id }, transaction })
     },
 
-    // updateSettings: async function (app, deviceGroup, settings, user) {
-    //     // NOTE: For now, device group settings only support environment variables
-    //     if (!hasProperty(settings, 'env')) {
-    //         return // nothing to do
-    //     }
-
-    //     // validate settings
-    //     if (!Array.isArray(settings.env)) {
-    //         throw new ValidationError('Invalid settings')
-    //     }
-    //     settings.env.forEach((envVar) => {
-    //         if (!envVar?.name?.match(/^[a-zA-Z_]+[a-zA-Z0-9_]*$/)) {
-    //             throw new ValidationError(`Invalid Env Var name '${envVar.name}'`)
-    //         }
-    //     })
-    //     // find duplicates
-    //     const seen = new Set()
-    //     const duplicates = settings.env.some(item => { return seen.size === seen.add(item.name).size })
-    //     if (duplicates) {
-    //         throw new ValidationError('Duplicate Env Var names provided')
-    //     }
-
-    //     // for audit log
-    //     const deviceGroupLogger = getApplicationLogger(app)
-    //     const updates = new app.auditLog.formatters.UpdatesCollection()
-    //     if (!deviceGroup.Application) {
-    //         await deviceGroup.reload({ include: [{ model: app.db.models.Application }] })
-    //     }
-    //     // transform the env arrays to a map for better logging format
-    //     const currentEnv = (deviceGroup.settings?.env || []).reduce((acc, e) => {
-    //         acc[e.name] = e.value
-    //         return acc
-    //     }, {})
-    //     const newEnv = settings.env.reduce((acc, e) => {
-    //         acc[e.name] = e.value
-    //         return acc
-    //     }, {})
-    //     updates.pushDifferences({ env: currentEnv }, { env: newEnv })
-
-    //     // perform update & log
-    //     if (updates.length === 0) {
-    //         return // nothing to do
-    //     }
-    //     deviceGroup.settings = {
-    //         ...deviceGroup.settings,
-    //         env: settings.env
-    //     }
-    //     await deviceGroup.save()
-
-    //     await deviceGroupLogger.application.deviceGroup.settings.updated(user, null, deviceGroup.Application, deviceGroup, updates)
-    // },
-
     /**
      * 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