feat: store lmsInstalled in deviceInfo JSON column#905
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #905 +/- ##
==========================================
+ Coverage 41.59% 41.63% +0.03%
==========================================
Files 134 134
Lines 12340 12357 +17
==========================================
+ Hits 5133 5145 +12
- Misses 6661 6663 +2
- Partials 546 549 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds an isLMSAvailable boolean attribute to the Device model and persists it in the database, wiring it through DTO/entity conversions and SQL CRUD.
Changes:
- Add DB migration to create/drop
devices.islmsavailablewith a default ofFALSE. - Extend device entity + v1 DTO + usecase transforms to include
IsLMSAvailable. - Update SQL repo Select/Scan/Insert/Update paths (and test DB schemas) to handle the new column.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/usecase/sqldb/device.go | Adds islmsavailable to device Select/Scan and Insert/Update statements. |
| internal/usecase/sqldb/device_test.go | Updates SQLite test schemas to include the new column. |
| internal/usecase/devices/usecase.go | Maps IsLMSAvailable between DTO and entity in both directions. |
| internal/entity/dto/v1/device.go | Adds isLMSAvailable to the v1 Device JSON model. |
| internal/entity/device.go | Adds IsLMSAvailable to the core Device entity. |
| internal/app/migrations/20260424000000_add_lms_available.up.sql | Adds islmsavailable column to devices. |
| internal/app/migrations/20260424000000_add_lms_available.down.sql | Drops islmsavailable column from devices. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0b82b37 to
b412772
Compare
132cf0f to
58d4ebc
Compare
9ef5a3f to
4808050
Compare
4808050 to
a93657d
Compare
sudhir-intc
left a comment
There was a problem hiding this comment.
In addition to the changes done here. How about the updates to the database table ? I don't see those changes here.
Also you should consider the scenario of
- Console installed without this feature, have a device added
- Install Console with this feature, it should continue to work for the existing device with the database table upgraded with this new field
* Wire deviceInfo serialization/deserialization in dtoToEntity/entityToDTO * Merge isLMSAvailable from activation into deviceInfo.lmsInstalled * Add LMSInstalled field to DeviceInfo DTO struct Related to device-management-toolkit/rpc-go#1246
a93657d to
458c3a7
Compare
No database migration needed. The deviceinfo TEXT column already exists in the original schema (20210221023242_migrate_name.up.sql) but was never populated by older console versions (v1.27.x returned "deviceInfo": null). This PR adds the logic to store/retrieve deviceInfo via RPC sync:
Test output (verified, refer the description): Started with v1.27.2 console → DB deviceinfo column empty |
| d.DeviceInfo.LMSInstalled = d.IsLMSAvailable | ||
| } | ||
|
|
||
| // Serialize DeviceInfo struct to JSON for DB storage. |
There was a problem hiding this comment.
Please extract this serialization/deserialization logic into reusable helper functions instead of inlining it in entityToDTO.
Related to device-management-toolkit/rpc-go#1246
Migration from older console to this PR change
Run old console v1.27.2 and check deviceInfo
Edge node 1,2 in pre-provisioning state
Edge node 1 - lms = inactive/stopped
Edge node 2 - lms = running
Activated both edge node using older console v1.27.2
Now run sync cmd on both and check deviceInfo
stopped console v1.27.2, starting console from this PR, re-sync and check deviceInfo