Bulk update team devices API (bulk move support only)
closes #4335
Description
Adds backend API route for team devices bulk update:
-
test/unit/forge/routes/api/teamDevices_spec.js - Method:
PUT - URI:
/api/v1/teams/:teamId/devices/bulk
The route was debated in design phase in the issue but after much debate, I have stuck with the same verb and definition already established in the existing devices API (that are responsible for single device combined update / device move endpoint). More than happy to change this around if desired.
Tests Added
Team Devices API
Team Devices
Supports bulk operations
update
move devices
✔ Rejects invalid application (404)
✔ Rejects invalid instance (404)
✔ Rejects moving a device to another team application (400)
✔ Rejects moving a device to another team instance (400)
✔ Rejects invalid input
✔ Member cannot move devices (403)
✔ Non team member cannot move devices (404)
✔ Move all to Application 2 (48ms)
✔ Move all to Project 2 (50ms)
✔ Only updates devices not already assigned to the target application (46ms)
✔ Only updates devices not already assigned to the target project (49ms)
✔ Only unassigns devices not already unassigned
Related Issue(s)
Owner #4335 Parent #4290
Checklist
- [x] I have read the contribution guidelines
- [x] 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/helmto update ConfigMap Template - [ ] Issue/PR raised on
FlowFuse/CloudProjectto update values for Staging/Production
- [ ] Issue/PR raised on
Labels
- [ ] Includes a DB migration? -> add the
area:migrationlabel
Codecov Report
Attention: Patch coverage is 84.25197% with 20 lines in your changes missing coverage. Please review.
Project coverage is 78.33%. Comparing base (
bb8f0a3) to head (455a8b3).
| Files | Patch % | Lines |
|---|---|---|
| forge/routes/api/teamDevices.js | 64.10% | 14 Missing :warning: |
| forge/db/controllers/Device.js | 93.18% | 6 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #4336 +/- ##
==========================================
+ Coverage 78.25% 78.33% +0.08%
==========================================
Files 292 292
Lines 13439 13558 +119
Branches 3014 3050 +36
==========================================
+ Hits 10517 10621 +104
- Misses 2922 2937 +15
| Flag | Coverage Δ | |
|---|---|---|
| backend | 78.33% <84.25%> (+0.08%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
NOTE: I mentioned in Eng Meeting that there was a sensible refactoring - that is the controller method moveDevices should replace the code in https://github.com/FlowFuse/flowfuse/blob/f58bc20fb1ffa838dbf24b57240bb91f04eb5955/forge/routes/api/device.js#L435-L518
There reason I was in two minds was related to one of our guys raising a concern about devices being reset when moved (even though they were in developer mode) and whether I would include or exclude that "feature".
As it happens, the implementation of the bulk update is a refactoring of this original route (with obvious modifications for multiple device support) INCLUDING the switching of a device to "autonomous" (fleet) mode when unassigned.
As it happens, with the introduction of bulk move feature, the user can side step the unassignment when moving devices from application to application and thus the switching out of developer mode so it kinda becomes less of an issue
In short, we can and should refactor the single version in the API Route to use the new controller method.
Will raise an issue as a follow up.
@Steve-Mcl approved - I appreciate there are a few other PRs behind this. Will let you merge in the appropriate order.
- All branches merged into this.
- Tested operations on pre-staging
- Once all green - good to go.