-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor: Upgrade express-validator to v7+ #4384
Conversation
📝 WalkthroughWalkthroughThis pull request streamlines existing functionality in the device registry. The HealthTips model’s register method has been refactored to directly use the provided arguments instead of processing the aqi_category with a switch statement. The express-validator dependency has been upgraded in package.json. In the validators, unused imports have been removed from the kya module while the tips module now leverages validationResult and a simplified structure to handle validations. Error handling and overall control flows remain intact. Changes
Sequence Diagram(s)sequenceDiagram
participant R as Request
participant V as Tips Validation Middleware
participant VR as validationResult
participant N as Next Handler
R->>V: Submit tip-related request
V->>VR: Validate payload using validationResult
alt Validation errors exist
VR-->>V: Return error details
V->>N: Pass HttpError with BAD_REQUEST status
else No errors
VR-->>V: Return clean validation result
V->>N: Call next middleware in chain
end
Possibly related PRs
Suggested reviewers
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 #4384 +/- ##
===========================================
- Coverage 11.21% 11.21% -0.01%
===========================================
Files 156 156
Lines 17985 17968 -17
Branches 388 388
===========================================
- Hits 2017 2015 -2
+ Misses 15966 15951 -15
Partials 2 2
|
Device registry changes in this PR available for preview here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/device-registry/models/HealthTips.js (1)
70-70
: LGTM! Simplified tip registration logic.The direct use of args is cleaner and relies on express-validator's upstream validation.
However, consider adding debug logging for the created tip to aid in troubleshooting:
- const createdTip = await this.create({ ...args }); + const createdTip = await this.create({ ...args }); + logObject("created tip", createdTip);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/device-registry/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/device-registry/models/HealthTips.js
(1 hunks)src/device-registry/package.json
(1 hunks)src/device-registry/validators/kya.validators.js
(0 hunks)src/device-registry/validators/tips.validators.js
(2 hunks)
💤 Files with no reviewable changes (1)
- src/device-registry/validators/kya.validators.js
⏰ 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 (2)
src/device-registry/package.json (1)
60-60
: Verify express-validator v7 compatibility.The upgrade from v6 to v7 is a major version change that may introduce breaking changes.
src/device-registry/validators/tips.validators.js (1)
207-210
: LGTM! Simplified validation chain references.The removal of
oneOf
wrapper simplifies the validation logic while maintaining the same functionality.
Description
This PR refactors the validation middleware to use
express-validator
v7+ and removes the deprecatedoneOf
method which was causing compatibility issues. The upgrade simplifies the validation code and makes it more maintainable.Changes Made
oneOf
andhandleValidationErrors
fromtips.validators.js
: TheoneOf
construct and the separatehandleValidationErrors
function have been removed. Validation logic is now incorporated directly into the validation chains, improving readability.oneOf
andhandleValidationErrors
fromkya.validators.js
: Similar to thetips.validators.js
changes,oneOf
and its separate error handling have been removed, integrating validation directly into the chains.cohorts.validators.js
: RemovedoneOf
and integratedvalidationResult
into individual validation chains. This ensures consistency across validators and removes deprecated code. UpdateddeviceIdentifiers
to use optional checks and a consolidated error message if no identifiers are provided. SimplifiedfilterNonPrivateDevices
andlistCohorts
logic.express-validator
inpackage.json
: Theexpress-validator
dependency has been updated to the latest version, ensuring compatibility. (Or, if this was already done in a previous commit, indicate that it's not part of this PR but mention the upgrade as context.)Testing
Affected Services
Endpoints Ready for Testing
API Documentation Updated?
Additional Notes
These changes ensure consistency across our validation logic, improve readability, and resolve the "chain.run is not a function" error caused by the use of
oneOf
in express-validator v6 syntax with the newer version 7+.Summary by CodeRabbit
Refactor
Chore