Conversation
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com>
…alidations_for_session_api
WalkthroughIntroduces EmptyStringParamPipe into AuthzController route parameter validation for userId and sessionId, augmenting the existing TrimStringParamPipe and ParseUUIDPipe chain. Enhances EmptyStringParamPipe error messaging logic in cast.helper.ts without altering successful flow or method signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant G as API Gateway
participant AC as AuthzController
participant P as Param Pipes
participant H as Handler
C->>G: HTTP request (userId/sessionId)
G->>AC: Route match
AC->>P: Apply EmptyStringParamPipe → TrimStringParamPipe → ParseUUIDPipe
alt Empty or missing value
P-->>AC: Throw BadRequestException (message varies)
AC-->>G: 400 response
G-->>C: 400 Bad Request
else Valid UUID
P-->>AC: Validated param
AC->>H: Invoke controller method
H-->>AC: Response
AC-->>G: 200/204 response
G-->>C: Success
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/common/src/cast.helper.ts (1)
187-195: Guard against undefined/non-string before calling trim to avoid runtime TypeError
value.trim()will throw if the param is missing or not a string. Add a type/emptiness guard before trimming and reuse the same required message logic to keep responses consistent.Apply this diff:
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/explicit-function-return-type transform(value: string) { - const trimmedValue = value.trim(); - - if (!trimmedValue) { - if (!this.paramName || 'string' !== typeof this.paramName) { - throw new BadRequestException(`Parameter is required and cannot be empty`); - } - throw new BadRequestException(`${this.paramName[0].toUpperCase() + this.paramName.slice(1)} is required`); - } + const hasName = 'string' === typeof this.paramName && this.paramName.trim().length > 0; + const requiredMsg = hasName + ? `${this.paramName.trim()[0].toUpperCase() + this.paramName.trim().slice(1)} is required` + : 'Parameter is required and cannot be empty'; + + if (value === undefined || value === null || 'string' !== typeof value) { + throw new BadRequestException(requiredMsg); + } + + const trimmedValue = value.trim(); + if (!trimmedValue) { + throw new BadRequestException(requiredMsg); + } return plainToClass(String, trimmedValue); }
🧹 Nitpick comments (2)
apps/api-gateway/src/authz/authz.controller.ts (2)
383-392: Good addition of EmptyStringParamPipe before UUID parsingPlacing
EmptyStringParamPipe.forParam('userId')before trim andParseUUIDPipeensures clear “required” errors for blank/space-only inputs.You can drop
new TrimStringParamPipe()here sinceEmptyStringParamPipealready returns a trimmed string.
426-435: Consistent validation for sessionIdSame pattern applied to
sessionIdis correct and improves error clarity.
new TrimStringParamPipe()is redundant afterEmptyStringParamPipe; consider removing to simplify the chain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api-gateway/src/authz/authz.controller.ts(3 hunks)libs/common/src/cast.helper.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api-gateway/src/authz/authz.controller.ts (1)
libs/common/src/cast.helper.ts (1)
EmptyStringParamPipe(175-199)
🔇 Additional comments (1)
apps/api-gateway/src/authz/authz.controller.ts (1)
57-57: Import looks correctImporting
EmptyStringParamPipealongsideTrimStringParamPipefrom common is consistent with usage below.
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: Ankita Patidar <ankita.patidar@ayanworks.com>
2 participants
What
Summary by CodeRabbit