refactored the site refresh functionality and fixed site update #4394
Conversation
📝 WalkthroughWalkthroughThis pull request updates the site operations by enforcing a mandatory site identifier validation on both the PUT /refresh and DELETE endpoints. New asynchronous utility methods have been introduced to manage site details, including fetching, preparing, and cleaning data. Additionally, the validation logic has been refactored to use a oneOf chain for the mandatory site identifier and simplified tenant validation for refresh and delete operations. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant R as Router
participant V as Validator
participant SC as Site Controller
participant SU as Site Utility
C->>R: PUT /refresh (with site identifier)
R->>V: validateMandatorySiteIdentifier
V-->>R: Validation result
R->>V: validateRefreshSite (tenant optional)
V-->>R: Validation result
R->>SC: Refresh request
SC->>SU: fetchSiteDetails
SU-->>SC: Site details
SC->>SU: prepareSiteRequestBody
SU-->>SC: Prepared request body
SC->>SU: fetchAdditionalSiteDetails
SU-->>SC: Additional site info
SC-->>R: Refresh outcome
R-->>C: Response
sequenceDiagram
participant C as Client
participant R as Router
participant V as Validator
participant SC as Site Controller
C->>R: DELETE Request (with site identifier)
R->>V: validateMandatorySiteIdentifier
V-->>R: Validation result
R->>V: validateDeleteSite (tenant optional)
V-->>R: Validation result
R->>SC: Delete request
SC-->>R: Deletion confirmation
R-->>C: Response
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 #4394 +/- ##
========================================
Coverage 11.21% 11.22%
========================================
Files 156 156
Lines 17970 17955 -15
Branches 388 388
========================================
Hits 2016 2016
+ Misses 15952 15937 -15
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 (3)
src/device-registry/validators/site.validators.js (1)
345-345: Consider retaining site identifier validation.The simplified validators now only check for optional tenant validation. While the mandatory site identifier validation is moved to the route level, consider keeping additional validations specific to refresh and delete operations.
-const validateRefreshSite = [createTenantValidation({ isOptional: true })]; +const validateRefreshSite = [ + createTenantValidation({ isOptional: true }), + body("status") + .optional() + .isIn(["active", "decommissioned"]) + .withMessage("status must be either 'active' or 'decommissioned'") +]; -const validateDeleteSite = [createTenantValidation({ isOptional: true })]; +const validateDeleteSite = [ + createTenantValidation({ isOptional: true }), + body("force") + .optional() + .isBoolean() + .withMessage("force must be a boolean value") +];Also applies to: 347-347
src/device-registry/utils/site.util.js (2)
51-81: Optimize object property deletion.The delete operator usage on lines 53-54 can impact performance. Consider using object destructuring or creating a new object with desired properties.
-let request = { ...siteDetails }; -delete request._id; -delete request.devices; +const { _id, devices, ...request } = siteDetails;🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 54-54: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
105-120: Consider using object destructuring for better performance.The
cleanWeatherStationDatafunction could be optimized using object destructuring.-return Object.keys(station) - .filter((key) => !fieldsToRemove.includes(key)) - .reduce((acc, key) => { - acc[key] = station[key]; - return acc; - }, {}); +const { elevation, countrycode, timezoneoffset, name, type, ...cleanedStation } = station; +return cleanedStation;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/device-registry/routes/v2/sites.routes.js(1 hunks)src/device-registry/utils/site.util.js(3 hunks)src/device-registry/validators/site.validators.js(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/device-registry/utils/site.util.js
[error] 53-53: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 54-54: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ 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 (6)
src/device-registry/routes/v2/sites.routes.js (1)
72-77: LGTM! Enhanced validation for site operations.The addition of
validateMandatorySiteIdentifierto both the refresh and delete routes improves input validation by ensuring that a valid site identifier is provided before proceeding with these operations.Also applies to: 78-83
src/device-registry/validators/site.validators.js (1)
233-241: LGTM! Improved validation chain for site identification.The
oneOfvalidation chain provides flexible site identification through either an ID, lat_long, or generated_name, making the API more versatile while maintaining strict validation.src/device-registry/utils/site.util.js (4)
32-50: LGTM! Well-structured site details retrieval.The
fetchSiteDetailsfunction provides clear error handling and logging while maintaining a consistent response structure.
83-103: LGTM! Efficient concurrent API calls.The
fetchAdditionalSiteDetailsfunction effectively uses Promise.all for concurrent API calls to optimize performance.
855-897: LGTM! Well-refactored refresh functionality.The refresh method has been effectively broken down into smaller, focused functions with proper error handling and data flow.
909-938: Review the delete functionality implementation.The delete functionality is currently disabled with a "coming soon" message. Consider implementing proper deletion logic or providing a more specific timeline for implementation.
Would you like me to help implement the delete functionality with proper validation and safety checks?
🧰 Tools
🪛 Biome (1.9.4)
[error] 918-929: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
|
Device registry changes in this PR available for preview here |
Description
refactored the site refresh functionality and fixed site update
Changes Made
validateMandatorySiteIdentifiervalidation so that it doesn't requireTesting
Affected Services
Endpoints Ready for Testing
API Documentation Updated?
Summary by CodeRabbit
New Features
Refactor