Feat: Add endpoints to retrieve site and device details by ID #4407
Conversation
📝 WalkthroughWalkthroughThis PR adds new asynchronous methods to fetch device and site details by ID and to find the nearest site. In the controllers, methods are added that check for an optional tenant parameter, call corresponding utility functions, and handle errors via HttpError. New GET endpoints are introduced in the routes with middleware validations for tenant and parameter formats. Additionally, utility functions are provided to retrieve database records for devices and sites, and validators using express-validator have been enhanced for proper ObjectId checks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant DC as DeviceController
participant DU as DeviceUtil
participant DB as DeviceModel
Client->>Server: GET /devices/:id
Server->>DC: getDeviceDetailsById(req, res, next)
DC->>DU: getDeviceById(req, next)
DU->>DB: Query device by ID
alt Device Found
DB-->>DU: Return device details
DU-->>DC: Return details
DC->>Server: handleResponse(details)
Server-->>Client: 200 OK with device details
else Error/Not Found
DB-->>DU: Return error
DU-->>DC: Propagate error
DC->>Server: next(HttpError)
Server-->>Client: Error response
end
sequenceDiagram
participant Client
participant Server
participant SC as SiteController
participant SU as SiteUtil
participant DB as SiteModel
Client->>Server: GET /sites/:id
Server->>SC: getSiteDetailsById(req, res, next)
SC->>SU: getSiteById(req, next)
SU->>DB: Query site by ID
alt Site Found
DB-->>SU: Return site details
SU-->>SC: Return details
SC->>Server: handleResponse(details)
Server-->>Client: 200 OK with site details
else Error/Not Found
DB-->>SU: Return error
SU-->>SC: Propagate error
SC->>Server: next(HttpError)
Server-->>Client: Error response
end
Possibly related issues
Possibly related PRs
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## staging #4407 +/- ##
===========================================
+ Coverage 11.22% 11.24% +0.01%
===========================================
Files 156 156
Lines 17955 18006 +51
Branches 388 388
===========================================
+ Hits 2016 2025 +9
- Misses 15937 15979 +42
Partials 2 2
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/device-registry/routes/v2/devices.routes.js (1)
153-159: LGTM! Consider adding JSDoc comments.The new route implementation follows the established pattern and includes proper validation middleware. To improve maintainability, consider adding JSDoc comments to document the route's purpose, parameters, and response format.
Add JSDoc comments above the route:
+/** + * @route GET /api/devices/:id + * @description Retrieve device details by ID + * @param {string} id - The device ID + * @param {string} [tenant] - Optional tenant parameter + * @returns {Object} Device details + */ router.get( "/:id", validateTenant, validateDeviceIdParam, validate, deviceController.getDeviceDetailsById );src/device-registry/controllers/device.controller.js (1)
53-73: LGTM! Consider adding input validation.The controller implementation follows the established pattern with proper error handling and consistent response format. However, consider adding input validation for the extracted parameters before processing.
Add parameter validation:
getDeviceDetailsById: async (req, res, next) => { try { const { id } = req.params; + if (!id) { + throw new HttpError("Device ID is required", httpStatus.BAD_REQUEST); + } const defaultTenant = constants.DEFAULT_TENANT || "airqo"; req.query.tenant = isEmpty(req.query.tenant) ? defaultTenant : req.query.tenant;src/device-registry/utils/device.util.js (1)
22-53: LGTM! Consider adding debug logging.The utility implementation follows best practices with proper error handling and consistent response format. Adding debug logging would help with troubleshooting.
Add debug logging:
getDeviceById: async (req, next) => { try { const { id } = req.params; const { tenant } = req.query; + logger.debug(`Fetching device with ID: ${id} for tenant: ${tenant}`); const device = await DeviceModel(tenant.toLowerCase()).findById(id); + logger.debug(`Device found: ${!!device}`); if (!device) { throw new HttpError("Device not found", httpStatus.NOT_FOUND); }src/device-registry/controllers/site.controller.js (1)
47-68: Consider enhancing error handling with specific error types.While the error handling is good, consider differentiating between different types of errors (e.g., validation errors, not found errors) for better client-side error handling.
Apply this diff to improve error handling:
getSiteDetailsById: async (req, res, next) => { try { const { id } = req.params; const defaultTenant = constants.DEFAULT_TENANT || "airqo"; req.query.tenant = isEmpty(req.query.tenant) ? defaultTenant : req.query.tenant; const result = await createSiteUtil.getSiteById(req, next); handleResponse({ result, res }); } catch (error) { logger.error(`🐛🐛 Internal Server Error ${error.message}`); + if (error instanceof HttpError) { + next(error); + return; + } next( new HttpError( "Internal Server Error", httpStatus.INTERNAL_SERVER_ERROR, { message: error.message } ) ); } },src/device-registry/utils/site.util.js (1)
32-63: Consider adding field selection for better performance.The current implementation fetches all fields from the database. Consider selecting only the required fields to optimize performance.
Apply this diff to optimize the database query:
getSiteById: async (req, next) => { try { const { id } = req.params; const { tenant } = req.query; - const site = await SiteModel(tenant.toLowerCase()).findById(id); + const site = await SiteModel(tenant.toLowerCase()) + .findById(id) + .select('name latitude longitude status created_at updated_at'); // Add required fields if (!site) { throw new HttpError("site not found", httpStatus.NOT_FOUND); } return { success: true, message: "site details fetched successfully", data: site, status: httpStatus.OK, }; } catch (error) { if (error instanceof HttpError) { next(error); return; } logger.error(`🐛🐛 Internal Server Error ${error.message}`); next( new HttpError( "Internal Server Error", httpStatus.INTERNAL_SERVER_ERROR, { message: error.message } ) ); } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/device-registry/controllers/device.controller.js(1 hunks)src/device-registry/controllers/site.controller.js(2 hunks)src/device-registry/routes/v2/devices.routes.js(2 hunks)src/device-registry/routes/v2/sites.routes.js(3 hunks)src/device-registry/utils/device.util.js(1 hunks)src/device-registry/utils/site.util.js(1 hunks)src/device-registry/validators/device.validators.js(3 hunks)src/device-registry/validators/site.validators.js(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-push-deploy-device-registry
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
src/device-registry/validators/device.validators.js (1)
79-89: LGTM! Well-structured validator implementation.The validator follows best practices with proper existence checks, format validation, and clear error messages. The use of
customSanitizerto convert the ID toObjectIdis a good practice.src/device-registry/routes/v2/sites.routes.js (1)
104-110: LGTM! Well-structured route with proper validation middleware.The route follows RESTful conventions and includes necessary validations for tenant and site ID parameters.
src/device-registry/validators/site.validators.js (2)
63-64: LGTM! Well-implemented parameter validation logic.The validation logic properly handles route parameters while maintaining compatibility with existing query and body validation.
Also applies to: 69-94
191-196: LGTM! Clear and concise site ID parameter validation.The validator ensures the site ID is present in the request path and is a valid MongoDB ObjectId.
Description
This pull request introduces new API endpoints to retrieve detailed information for both sites and devices using their respective IDs. These endpoints enhance the API's functionality by allowing clients to directly access specific site and device records.
Changes Made
Testing
Affected Services
Endpoints Ready for Testing
API Documentation Updated?
Summary by CodeRabbit