refactor: expose activated/discovered stats and device filters - #1214
refactor: expose activated/discovered stats and device filters#1214ShradhaGupta31 wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## denormalize-mode-discovered #1214 +/- ##
===============================================================
+ Coverage 50.82% 50.94% +0.11%
===============================================================
Files 149 149
Lines 13876 14101 +225
===============================================================
+ Hits 7053 7184 +131
- Misses 6219 6296 +77
- Partials 604 621 +17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
e30d3e3 to
6aef488
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the v1 Devices APIs to expose activated and discovered device state metrics in the stats response, and adds query-parameter filtering (?activated=true, ?discovered=true) to the devices list endpoint, wiring the behavior through the devices use case, repository interface, and both SQL + Mongo backends, with accompanying OpenAPI and Postman updates.
Changes:
- Add
activatedCountanddiscoveredCounttoGET /api/v1/devices/stats. - Add
?activated=true/?discovered=truefiltering toGET /api/v1/devices. - Implement repository + usecase support for activated/discovered queries and counts, including SQL and Mongo implementations, plus tests and API tooling updates.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/usecase/sqldb/device.go | Adds SQL-backed repo methods for activated/discovered listing and state counts. |
| internal/usecase/sqldb/device_test.go | Adds unit tests for new SQL repo filtering/counting behavior. |
| internal/usecase/nosqldb/mongo/fields.go | Adds Mongo field/operator constants needed for new filters. |
| internal/usecase/nosqldb/mongo/device.go | Adds Mongo-backed repo methods for activated/discovered listing and state counts. |
| internal/usecase/nosqldb/mongo/device_test.go | Adds unit tests for new Mongo repo methods. |
| internal/usecase/devices/repo.go | Adds usecase methods to expose repo activated/discovered queries/counts as DTOs. |
| internal/usecase/devices/repo_test.go | Adds usecase-level tests for the new methods. |
| internal/usecase/devices/interfaces.go | Extends the repository + feature interfaces with activated/discovered methods. |
| internal/mocks/devicemanagement_mocks.go | Regenerates mocks to include the new interface methods. |
| internal/entity/dto/v1/device.go | Extends device stats DTO with activated/discovered counts. |
| internal/controller/openapi/devices.go | Updates OpenAPI to document new query params and stats fields. |
| internal/controller/httpapi/v1/devices.go | Implements stats expansion and device list filtering in the Gin handlers. |
| internal/controller/httpapi/v1/devices_test.go | Adds controller tests for new filtering and stats behavior. |
| integration-test/collections/console_mps_apis.postman_collection.json | Updates Postman collection tests/requests for new fields and filters. |
Files not reviewed (1)
- internal/mocks/devicemanagement_mocks.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return nil, ErrDeviceDatabase.Wrap(op, "r.Pool.Query", err) | ||
| } | ||
|
|
||
| if rows.Err() != nil { |
There was a problem hiding this comment.
rows.Err() is checked before iterating, and rows.Close() is deferred after that check. This could leave rows unclosed on error. Also, there is no rows.Err() check after the loop. Can we use the same pattern as GetByColumn and GetByTags?
There was a problem hiding this comment.
Updated. Defer is now placed after all row fetch related checks are done.
12984d5 to
a4002ea
Compare
sudhir-intc
left a comment
There was a problem hiding this comment.
Initial comments. but need to test these changes.
| // Legacy rows with a NULL currentmode and the "not activated" sentinel are excluded. | ||
| activatedWhere = "currentmode IS NOT NULL AND currentmode <> '' AND LOWER(currentmode) <> 'not activated'" | ||
| // discoveredWhere matches devices flagged as discovered on the network. | ||
| discoveredWhere = "discovered = ?" |
There was a problem hiding this comment.
My understanding is that devices which are not yet activated would part of the discoveredWhere so it would the reverse of activatedWhere
There was a problem hiding this comment.
Updated logic
| fuego.OptionQuery("method", "Method to filter tags (any/all)"), | ||
| fuego.OptionQuery("hostname", "Filter devices by host name"), | ||
| fuego.OptionQuery("friendlyName", "Filter devices by friendly name"), | ||
| fuego.OptionQueryBool("activated", "Return only devices provisioned into an AMT control mode"), |
There was a problem hiding this comment.
| fuego.OptionQueryBool("activated", "Return only devices provisioned into an AMT control mode"), | |
| fuego.OptionQueryBool("activated", "Return devices activated into client or admin control mode"), |
| fuego.OptionQuery("hostname", "Filter devices by host name"), | ||
| fuego.OptionQuery("friendlyName", "Filter devices by friendly name"), | ||
| fuego.OptionQueryBool("activated", "Return only devices provisioned into an AMT control mode"), | ||
| fuego.OptionQueryBool("discovered", "Return only devices discovered on the network"), |
There was a problem hiding this comment.
| fuego.OptionQueryBool("discovered", "Return only devices discovered on the network"), | |
| fuego.OptionQueryBool("discovered", "Return devices discovered on the network but not yet activated"), |
| func discoveredFilter(tenantID string) bson.M { | ||
| return bson.M{ | ||
| fieldTenantID: tenantID, | ||
| fieldDiscovered: true, |
There was a problem hiding this comment.
We could use fieldCurrentMode and check for not-activated here. This depends on the comment given in the other PR related to denormalize to check if discovered field is really needed?
There was a problem hiding this comment.
Please review comment & confirm if this change is needed
There was a problem hiding this comment.
Yes lets remove the fieldDiscovered here and use fieldCurrentMode
c20cd8b to
7034e21
Compare
a4002ea to
74e74a4
Compare
7034e21 to
e17b536
Compare
a8ce286 to
a33dec0
Compare
Addresses: #1210 - Add activatedCount and discoveredCount to the device stats response, and - Add support ?activated=true / ?discovered=true filtering on the devices list endpoint - Implements the Stats API exposure and Devices filtering across all three storage backends (Postgres, SQLite, MongoDB), - Update OpenAPI/Fuego declarations and Postman collection entries.
- modified discovered device modify logic Signed-off-by: ShradhaGupta31 <[email protected]>
Addresses: #1210
ADR: https://github.com/device-management-toolkit/console/wiki/API-contract-update-&-DB-query-mechanism-to-fetch-Device-Stats
Stats API responses:
API to filter using Activated/Discovered
curl -sk "https://localhost:8181/api/v1/devices?discovered=true" -H "Authorization: Bearer $TOKEN" | jq .curl -sk "https://localhost:8181/api/v1/devices?activated=true" -H "Authorization: Bearer $TOKEN" | jq .