fix[validation]: add profileById validation for user queries(BUG-3833) #827
Conversation
WalkthroughAdds a new English locale key and introduces a user profile validation function that validates optional route param id, optional query fields, and enforces cross-field requirements when id is absent. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant V as profileById Validator
participant R as Route Handler
C->>V: Request (params: id?, query: email?, username?, phone?, phone_code?, tenant_code?)
alt id provided
V->>V: Validate id is numeric (optional)
V-->>R: next()
else id not provided
V->>V: Check at least one: email OR username OR phone
alt phone provided
V->>V: Require phone_code
end
V-->>R: next() if valid
end
V-->>C: 400 on validation errors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/locales/en.json (1)
181-182: Avoid introducing redundant locale keys; align punctuation.You already have PROFILE_FETCHED_SUCCESSFULLY (Line 18). Consider reusing it to prevent duplication, unless a distinct nuance is required. Also, drop the exclamation mark for consistency with most success messages.
Apply this diff if you keep the new key but want consistent punctuation:
- "USER_PROFILE_FETCHED_SUCCESSFULLY": "User profile fetched successfully!" + "USER_PROFILE_FETCHED_SUCCESSFULLY": "User profile fetched successfully."src/validators/v1/user.js (1)
44-49: Use isInt() for numeric validation (more idiomatic and robust than regex).- .matches(/^[0-9]+$/) - .withMessage('id is invalid. Must be numeric') + .isInt() + .withMessage('id is invalid. Must be numeric')
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/locales/en.json(1 hunks)src/validators/v1/user.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/validators/**
⚙️ CodeRabbit configuration file
Validate all incoming data thoroughly. Check for missing or incomplete validation rules.
Files:
src/validators/v1/user.js
| if (!req.params.id) { | ||
| req.checkQuery(['email', 'username', 'phone', 'phone_code']).custom(() => { | ||
| const { email, username, phone, phone_code } = req.query | ||
|
|
||
| if (!email && !username && !phone) { | ||
| throw new Error('At least one of id, email, username, or phone must be provided') | ||
| } | ||
|
|
||
| if (phone && !phone_code) { | ||
| throw new Error('phone_code is required when phone is provided') | ||
| } | ||
|
|
||
| return true | ||
| }) | ||
| } |
There was a problem hiding this comment.
Array-scoped custom validator will duplicate errors; anchor cross-field rules to single fields.
Using checkQuery(['a','b','c']).custom(...) executes per field, yielding multiple identical errors. Replace with single-field custom checks to avoid duplication and clearer error attribution. Also refine message (id isn’t relevant inside the id-absent branch).
- if (!req.params.id) {
- req.checkQuery(['email', 'username', 'phone', 'phone_code']).custom(() => {
- const { email, username, phone, phone_code } = req.query
-
- if (!email && !username && !phone) {
- throw new Error('At least one of id, email, username, or phone must be provided')
- }
-
- if (phone && !phone_code) {
- throw new Error('phone_code is required when phone is provided')
- }
-
- return true
- })
- }
+ // Cross-field: require one of email/username/phone when id is absent
+ req.checkQuery('email').custom((_val, { req }) => {
+ const { id } = req.params || {}
+ const { email, username, phone } = req.query || {}
+ if (!id && !email && !username && !phone) {
+ throw new Error('At least one of email, username, or phone must be provided')
+ }
+ return true
+ })
+
+ // Cross-field: require phone_code when phone is provided (and id is absent)
+ req.checkQuery('phone_code').custom((_val, { req }) => {
+ const { id } = req.params || {}
+ const { phone, phone_code } = req.query || {}
+ if (!id && phone && !phone_code) {
+ throw new Error('phone_code is required when phone is provided')
+ }
+ return true
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!req.params.id) { | |
| req.checkQuery(['email', 'username', 'phone', 'phone_code']).custom(() => { | |
| const { email, username, phone, phone_code } = req.query | |
| if (!email && !username && !phone) { | |
| throw new Error('At least one of id, email, username, or phone must be provided') | |
| } | |
| if (phone && !phone_code) { | |
| throw new Error('phone_code is required when phone is provided') | |
| } | |
| return true | |
| }) | |
| } | |
| // Cross-field: require one of email/username/phone when id is absent | |
| req.checkQuery('email').custom((_val, { req }) => { | |
| const { id } = req.params || {} | |
| const { email, username, phone } = req.query || {} | |
| if (!id && !email && !username && !phone) { | |
| throw new Error('At least one of email, username, or phone must be provided') | |
| } | |
| return true | |
| }) | |
| // Cross-field: require phone_code when phone is provided (and id is absent) | |
| req.checkQuery('phone_code').custom((_val, { req }) => { | |
| const { id } = req.params || {} | |
| const { phone, phone_code } = req.query || {} | |
| if (!id && phone && !phone_code) { | |
| throw new Error('phone_code is required when phone is provided') | |
| } | |
| return true | |
| }) |
🤖 Prompt for AI Agents
In src/validators/v1/user.js around lines 82-96, the current use of
req.checkQuery([...]).custom(...) runs the custom validator for each listed
field and so produces duplicate identical errors; replace the array-scoped
validator with a single-field custom validator (e.g., attach the custom to
'email' or another single query field) that accesses req.query to perform the
cross-field checks when req.params.id is missing, update the error text to "At
least one of email, username, or phone must be provided" (remove id from that
message), and keep the phone/phone_code check (throw "phone_code is required
when phone is provided") so the rule runs once and attributes the error to a
single field.
Summary by CodeRabbit