Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement API for Device Groups #3157

Merged
merged 104 commits into from
Dec 20, 2023
Merged

Implement API for Device Groups #3157

merged 104 commits into from
Dec 20, 2023

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented Dec 4, 2023

Description

DB Schema (Updated following discussion here)

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
);

-- DeviceGroupId reference is the only change to `Device` ↓
CREATE TABLE Devices (
    id               INTEGER       PRIMARY KEY AUTOINCREMENT,
    name             VARCHAR (255) NOT NULL,
    type             VARCHAR (255) NOT NULL,
    credentialSecret VARCHAR (255) NOT NULL,
    state            VARCHAR (255) NOT NULL
                                   DEFAULT '',
    lastSeenAt       DATETIME,
    settingsHash     VARCHAR (255),
    agentVersion     VARCHAR (255),
    mode             VARCHAR (255) DEFAULT 'autonomous',
    createdAt        DATETIME      NOT NULL,
    updatedAt        DATETIME      NOT NULL,
    TeamId           INTEGER       REFERENCES Teams (id) ON DELETE SET NULL
                                                         ON UPDATE CASCADE,
    targetSnapshotId INTEGER       REFERENCES ProjectSnapshots (id) ON DELETE SET NULL
                                                                    ON UPDATE CASCADE,
    activeSnapshotId INTEGER       REFERENCES ProjectSnapshots (id) ON DELETE SET NULL
                                                                    ON UPDATE CASCADE,
    ApplicationId    INTEGER       REFERENCES Applications (id) ON DELETE SET NULL
                                                                ON UPDATE CASCADE,
    ProjectId        UUID          REFERENCES Projects (id) ON DELETE SET NULL
                                                            ON UPDATE CASCADE,
    DeviceGroupId    INTEGER       REFERENCES DeviceGroups (id) ON DELETE SET NULL
                                                                ON UPDATE CASCADE
);

(both the migration and a fresh start generate the exact same DDL)

image

For #2997

API End points added to forge/routes/api/applicationDeviceGroup.js:

  • GET /api/v1/applications/:applicationId/devicegroups
    • get a list of DeviceGroups in this application
  • POST /api/v1/applications/:applicationId/devicegroups
    • add a new Device Group to an Application
    • body: { name, [description] }
  • PUT /api/v1/applications/:applicationId/devicegroups/:groupId
    • update a device group settings
    • body: { name, [description] }
  • GET /api/v1/applications/:applicationId/devicegroups/:groupId
    • get a specific deviceGroup (must be assigned to this application)
  • PATCH /api/v1/applications/:applicationId/devicegroups/:groupId
    • update Device Group membership
    • OPTION1: body: { add: [deviceIds], remove: [deviceIds] }
    • OPTION2: body: { set: [deviceIds] }
  • DELETE /api/v1/applications/:applicationId/devicegroups/:groupId
    • delete app owned deviceGroup

Tests: UPDATED 2023-12-08

  Application API
    Application Device Groups
      Create Device Group
        ✔ Owner can create a device group (159ms)
        ✔ Cannot create a device group with empty name (125ms) ✨NEW 2023-12-08 ✨
        ✔ Non Owner can not create a device group (111ms)
        ✔ Non Member can not create a device group (125ms)
      Read Device Groups
        ✔ Owner can read device groups (131ms)
        ✔ Paginates device groups (123ms)
        ✔ Non Owner can read device groups (160ms)
        ✔ Can get a specific group and associated devices (163ms)
        ✔ 404s when getting a specific group that does not exist (112ms)
        ✔ Non Member can not read device groups (126ms)
      Update Device Group
        ✔ Owner can update a device group (129ms)
        ✔ Cannot update a device group with empty name (128ms) ✨NEW 2023-12-08 ✨
        ✔ Non Owner can not update a device group (107ms)
        ✔ Non Member can not update a device group (123ms)
      Delete Device Group
        ✔ Owner can delete a device group (113ms)
        ✔ Non Owner can not delete a device group (118ms)
        ✔ Non Member can not delete a device group (102ms)
      Update Device Group Membership
        ✔ Owner can add a device to a new group (146ms)
        ✔ Owner can add a device to an existing group (152ms)
        ✔ Owner can remove a device from an existing group (138ms)
        ✔ Owner can add and remove devices from an existing group (185ms)
        ✔ Owner can set the list of devices in existing group (186ms)
        ✔ Can not add a device to a group in a different team (136ms)
        ✔ Can not add a device to a group if they belong to different applications (133ms)
        ✔ Can not add a device to a group if already in a group (133ms) ✨NEW 2023-12-08 ✨
        ✔ Non Owner can not update a device group membership (104ms)
        ✔ Non Member can not update a device group membership (132ms)

Related Issue(s)

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Backport needed? -> add the backport label
  • Includes a DB migration? -> add the area:migration label

@Steve-Mcl Steve-Mcl linked an issue Dec 4, 2023 that may be closed by this pull request
Copy link

codecov bot commented Dec 4, 2023

Codecov Report

Attention: 471 lines in your changes are missing coverage. Please review.

Comparison is base (7175f06) 40.36% compared to head (4796eca) 40.76%.
Report is 10 commits behind head on main.

Files Patch % Lines
...tend/src/pages/application/DeviceGroup/devices.vue 0.00% 124 Missing ⚠️
frontend/src/pages/application/DeviceGroups.vue 0.00% 52 Missing ⚠️
...end/src/pages/application/DeviceGroup/settings.vue 0.00% 46 Missing ⚠️
frontend/src/pages/application/Pipelines.vue 0.00% 41 Missing ⚠️
...ntend/src/pages/application/PipelineStage/form.vue 0.00% 37 Missing ⚠️
...ontend/src/pages/application/DeviceGroup/index.vue 0.00% 35 Missing ⚠️
frontend/src/components/pipelines/PipelineRow.vue 0.00% 20 Missing ⚠️
forge/ee/db/controllers/Pipeline.js 84.90% 16 Missing ⚠️
...end/src/components/audit-log/AuditEntryVerbose.vue 0.00% 11 Missing ⚠️
frontend/src/store/account.js 0.00% 11 Missing ⚠️
... and 16 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3157      +/-   ##
==========================================
+ Coverage   40.36%   40.76%   +0.39%     
==========================================
  Files         592      603      +11     
  Lines       21666    22516     +850     
  Branches     5214     5441     +227     
==========================================
+ Hits         8746     9179     +433     
- Misses      12920    13337     +417     
Flag Coverage Δ
backend 77.54% <91.41%> (+0.62%) ⬆️
frontend 2.02% <2.56%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Co-authored-by: Pez Cuckow <email@pezcuckow.com>
forge/db/views/DeviceGroup.js Outdated Show resolved Hide resolved
forge/db/views/DeviceGroup.js Outdated Show resolved Hide resolved
@Steve-Mcl
Copy link
Contributor Author

@Pezmc @knolleary

Following answers to some questions I had, I have agreed to do the following:

  • Change the application cascade to delete a DeviceGroup
  • Uncomment schemas

Therefore, I will switch to draft until this is implemented. Once done and back out of draft, I will re-request reviews.

Thanks.

@Steve-Mcl Steve-Mcl marked this pull request as draft December 4, 2023 14:39
@Steve-Mcl Steve-Mcl requested a review from Pezmc December 19, 2023 11:50
@knolleary
Copy link
Member

Re #2997 (comment)

Re-reading the discussion, this feature should only be available to Enterprise Teams on the platform. This means it has to be added as a feature flag on the TeamType as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs renaming as we have more recent migrations already merged.

@knolleary knolleary self-requested a review December 20, 2023 13:04
@knolleary
Copy link
Member

Ideally we'd have all of the DeviceGroup content in the ee code tree - however as this adds a relation between Device and DeviceGroup, that isn't possible to do without more refactoring than we can accommodate now.

I have made one minor change to move a DeviceGroup specific function from controller.Device to controller.DeviceGroup. But any further moving of code into the EE tree is now out of scope.

@knolleary knolleary merged commit fb33603 into main Dec 20, 2023
10 checks passed
@knolleary knolleary deleted the 2997-device-groups-backend branch December 20, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:migration Involves a database migration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implementing Device Groups Management
3 participants