feat/ add API to update impersonation requests#2446
feat/ add API to update impersonation requests#2446iamitprakash merged 7 commits intoRealDevSquad:developfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThis update introduces the ability for a normal user to approve or reject impersonation requests from privileged users. It adds a PATCH API endpoint to update the status of impersonation requests, with input validation, service-layer authorization and update logic, Firestore model changes, and expanded type definitions to support the new workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant AuthMiddleware
participant Validator
participant Controller
participant Service
participant Model
participant Logger
Client->>Router: PATCH /impersonation/requests/:id
Router->>AuthMiddleware: Authenticate user
AuthMiddleware->>Validator: Validate request body (status)
Validator->>Controller: Pass validated request
Controller->>Service: validateUpdateImpersonationRequestService(requestId, userId)
Service->>Model: Fetch impersonation request by ID
Model-->>Service: Return request data
Service->>Service: Check authorization & status
Service-->>Controller: Validation success
Controller->>Service: updateImpersonationRequestServie(updateData)
Service->>Model: Update impersonation request in Firestore
Model-->>Service: Return updated request
Service->>Logger: addLog(action, details)
Logger-->>Service: Log saved
Service-->>Controller: Return updated request and message
Controller->>Client: Respond with 200 and updated request
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
controllers/impersonationRequests.ts(1 hunks)middlewares/validators/impersonationRequests.ts(1 hunks)models/impersonationRequests.ts(1 hunks)routes/impersonation.ts(1 hunks)routes/index.ts(1 hunks)services/impersonationRequests.ts(1 hunks)types/impersonationRequest.d.ts(1 hunks)
🧰 Additional context used
🪛 ESLint
routes/impersonation.ts
[error] 3-3: Import in body of module; reorder to top.
(import/first)
[error] 4-4: Import in body of module; reorder to top.
(import/first)
[error] 5-5: Import in body of module; reorder to top.
(import/first)
[error] 7-7: Replace "/requests/:id",·authenticate,updateImpersonationRequestValidator,updateImpersonationRequestStatusController with ⏎··"/requests/:id",⏎··authenticate,⏎··updateImpersonationRequestValidator,⏎··updateImpersonationRequestStatusController⏎
(prettier/prettier)
[error] 9-9: Insert ⏎
(prettier/prettier)
🪛 GitHub Check: CodeQL
routes/impersonation.ts
[failure] 7-7: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
🪛 GitHub Check: build (22.10.0)
services/impersonationRequests.ts
[failure] 13-13:
Module '"../models/impersonationRequests"' has no exported member 'getImpersonationRequestById'.
🪛 GitHub Actions: Tests
services/impersonationRequests.ts
[error] 13-13: TypeScript error TS2305: Module '../models/impersonationRequests' has no exported member 'getImpersonationRequestById'.
🪛 Biome (1.9.4)
types/impersonationRequest.d.ts
[error] 83-83: Shouldn't redeclare 'Request'. Consider to delete it or rename it.
'Request' is defined here:
(lint/suspicious/noRedeclare)
[error] 83-83: Shouldn't redeclare 'Response'. Consider to delete it or rename it.
'Response' is defined here:
(lint/suspicious/noRedeclare)
[error] 84-84: Shouldn't redeclare 'REQUEST_STATE'. Consider to delete it or rename it.
'REQUEST_STATE' is defined here:
(lint/suspicious/noRedeclare)
[error] 85-85: Shouldn't redeclare 'Boom'. Consider to delete it or rename it.
'Boom' is defined here:
(lint/suspicious/noRedeclare)
[error] 86-86: Shouldn't redeclare 'RequestQuery'. Consider to delete it or rename it.
'RequestQuery' is defined here:
(lint/suspicious/noRedeclare)
[error] 87-87: Shouldn't redeclare 'userData'. Consider to delete it or rename it.
'userData' is defined here:
(lint/suspicious/noRedeclare)
[error] 88-88: Shouldn't redeclare 'Timestamp'. Consider to delete it or rename it.
'Timestamp' is defined here:
(lint/suspicious/noRedeclare)
[error] 90-90: Shouldn't redeclare 'ImpersonationRequest'. Consider to delete it or rename it.
'ImpersonationRequest' is defined here:
(lint/suspicious/noRedeclare)
[error] 106-106: Shouldn't redeclare 'CreateImpersonationRequestBody'. Consider to delete it or rename it.
'CreateImpersonationRequestBody' is defined here:
(lint/suspicious/noRedeclare)
[error] 121-121: Shouldn't redeclare 'UpdateImpersonationRequestDataBody'. Consider to delete it or rename it.
'UpdateImpersonationRequestDataBody' is defined here:
(lint/suspicious/noRedeclare)
[error] 127-127: Shouldn't redeclare 'UpdateImpersonationRequestStatusBody'. Consider to delete it or rename it.
'UpdateImpersonationRequestStatusBody' is defined here:
(lint/suspicious/noRedeclare)
[error] 146-146: Shouldn't redeclare 'ImpersonationRequestQuery'. Consider to delete it or rename it.
'ImpersonationRequestQuery' is defined here:
(lint/suspicious/noRedeclare)
[error] 158-158: Shouldn't redeclare 'ImpersonationRequestResponse'. Consider to delete it or rename it.
'ImpersonationRequestResponse' is defined here:
(lint/suspicious/noRedeclare)
[error] 162-162: Shouldn't redeclare 'RequestParams'. Consider to delete it or rename it.
'RequestParams' is defined here:
(lint/suspicious/noRedeclare)
[error] 166-166: Shouldn't redeclare 'CreateImpersonationRequest'. Consider to delete it or rename it.
'CreateImpersonationRequest' is defined here:
(lint/suspicious/noRedeclare)
[error] 179-179: Shouldn't redeclare 'PaginatedImpersonationRequests'. Consider to delete it or rename it.
'PaginatedImpersonationRequests' is defined here:
(lint/suspicious/noRedeclare)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
routes/index.ts (1)
44-44: LGTM! Route integration follows established patterns.The new impersonation route is correctly integrated with the dev flag middleware, consistent with other feature-flagged routes like subscription.
models/impersonationRequests.ts (1)
14-31: Model function is well-implemented but verify document existence handling.The update function is correctly structured with good error handling and documentation. However, verify that the Firestore update behavior aligns with business requirements.
Firestore's
update()method will throw an error if the document doesn't exist, which is likely the desired behavior for updating impersonation requests. Can you confirm this is the intended behavior vs. usingset()with merge option?#!/bin/bash # Check if there are any error handling patterns for non-existent documents ast-grep --pattern 'update($$$)' rg -A 5 -B 5 "document.*not.*exist|document.*does.*not"services/impersonationRequests.ts (2)
32-57: Validation logic is well-structured.The function correctly validates business rules for impersonation request updates:
- Checks request existence
- Prevents updates to already processed requests
- Ensures only the impersonated user can approve/reject requests
13-13: ```shell
#!/bin/bashLocate model file
model_file=$(fd impersonationRequests.ts | grep 'models/impersonationRequests.ts' | head -n1)
echo "Model file: $model_file"List all exports in the model file
echo "Exports in the model file:"
rg '^export ' "$model_file" || echo "(none found)"Search for getImpersonationRequestById in the model file
echo "Occurrences of getImpersonationRequestById:"
rg -n 'getImpersonationRequestById' "$model_file" || echo "(not found)"Locate service file
service_file=$(fd impersonationRequests.ts | grep 'services/impersonationRequests.ts' | head -n1)
echo "Service file: $service_file"Check for typo in service function name
echo "Occurrences of updateImpersonationRequestServie:"
rg -n 'updateImpersonationRequestServie' "$service_file" || echo "(not found)"</details> <details> <summary>types/impersonationRequest.d.ts (3)</summary> `111-119`: **Approve the enhanced type definitions.** The new `CreateImpersonationRequestModelDto` type properly separates the DTO structure from the request body type, improving type safety and clarity. --- `133-144`: **Well-designed update types.** The `UpdateImpersonationRequestModelDto` and `UpdateImpersonationStatusModelResponse` types provide clear contracts for the update operations, supporting both data and status updates. --- `146-156`: **Enhanced query type supports comprehensive filtering.** The expanded `ImpersonationRequestQuery` type adds valuable filtering capabilities (createdBy, createdFor, status, pagination) that weren't available in the original version. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Typo in service name ▹ view | ✅ Fix detected |
|
Inconsistent Middleware Spacing ▹ view | ✅ Fix detected |
|
Unused import ▹ view | ✅ Fix detected |
 |
Redundant Object Property ▹ view | ✅ Fix detected |
 |
Missing Request Context in Error Logging ▹ view | ✅ Fix detected |
 |
Insufficient Error Context in Logs ▹ view | ✅ Fix detected |
 |
Misspelled Service Function Name ▹ view | ✅ Fix detected |
|
Untyped Generic Error Handling ▹ view | |
 |
Sequential Async Operations ▹ view | ✅ Fix detected |
 |
Unvalidated Object Spread in Database Update ▹ view | ✅ Fix detected |
Files scanned
| File Path | Reviewed |
|---|---|
| routes/impersonation.ts | ✅ |
| middlewares/validators/impersonationRequests.ts | ✅ |
| routes/index.ts | ✅ |
| models/impersonationRequests.ts | ✅ |
| controllers/impersonationRequests.ts | ✅ |
| services/impersonationRequests.ts | ✅ |
| types/impersonationRequest.d.ts | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
…ackend into feat/update_impersonation_requests
Suvidh-kaushik
force-pushed
the
feat/update_impersonation_requests
branch
from
af6daa7 to
ca40fcb
Compare
…ackend into feat/update_impersonation_requests
Suvidh-kaushik
dismissed stale reviews from Achintya-Chatterjee and MayankBansal12
via
715e67f
4 participants
Date: 21/06/2025
Developer Name: Suvidh Kaushik
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Recordings
Unauthorized to updateupdate_4.mp4
Rejected and already Rejected
update_2.mp4
Approved
update_1.mp4
Request does not exist
update_3.mp4
Test Coverage
Details
Unit Tests Coverage Report
Validator
Service
Model
Integration Tests Coverage Report
Additional Notes
Design Doc - LINK
PRD - LINK
Description by Korbit AI
What change is being made?
Add a new API endpoint to update the status of impersonation requests, including associated controllers, validators, services, models, and routes.
Why are these changes being made?
This change allows users with the appropriate roles to update the status of impersonation requests, enabling more dynamic management of these requests. The update functionality ensures compliance with request states and user roles, minimizing unauthorized actions, and improving the system's overall workflow efficiency.