Skip to content

Comments

feat: add API to fetch impersonation requests#2442

Merged
prakashchoudhary07 merged 12 commits intoRealDevSquad:developfrom
Suvidh-kaushik:feat/get_impersonation_requests
Jun 25, 2025
Merged

feat: add API to fetch impersonation requests#2442
prakashchoudhary07 merged 12 commits intoRealDevSquad:developfrom
Suvidh-kaushik:feat/get_impersonation_requests

Conversation

@Suvidh-kaushik
Copy link
Contributor

@Suvidh-kaushik Suvidh-kaushik commented Jun 11, 2025

Date: 11/06/2025

Developer Name: Suvidh Kaushik


Issue Ticket Number

Description

Documentation Updated?

  • Yes
  • No

Under Feature Flag

  • Yes
  • No

Database Changes

  • Yes
  • No

Breaking Changes

  • Yes
  • No

Development Tested?

  • Yes
  • No

Screenshots

Recordings
fetch_by_createdFor.mp4
pagination_and_fetch_by_size.mp4
fetch_by_createdBy.mp4
fetch_by_status.mp4
fetch_by_id.mp4
fetch_by_createdBy.mp4

Test Coverage

Screenshot 1

Additional Notes

Design Doc - LINK

PRD - LINK

Description by Korbit AI

What change is being made?

Add new API endpoints to fetch impersonation requests, either by ID or with pagination and filtering options, including the necessary controllers, validators, models, and route integrations.

Why are these changes being made?

To enhance the API's capabilities by allowing clients to retrieve specific impersonation requests using their IDs and to support filtering and pagination for improved usability and data management. This implementation helps streamline access and organization of impersonation requests as part of our system's broader administrative and security functionalities.

Is this description stale? Ask me to generate a new description by commenting /korbit-generate-pr-description

@coderabbitai
Copy link

coderabbitai bot commented Jun 11, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Summary by CodeRabbit

  • New Features

    • Introduced a new API endpoint to fetch impersonation requests, supporting filters and pagination.
    • Added validation for query parameters when retrieving impersonation requests.
    • Users can now retrieve individual or multiple impersonation requests with flexible filtering options.
  • Bug Fixes

    • Improved error handling and response codes for invalid or missing data when accessing impersonation requests.

Walkthrough

This change introduces the backend infrastructure for fetching impersonation requests. It adds a controller, validation middleware, Firestore model functions, and route registration for a new GET /impersonation/requests endpoint. The implementation includes query parameter validation, pagination, error handling, and integration into the main router.

Changes

File(s) Change Summary
controllers/impersonationRequests.ts Added controller for handling GET requests to fetch impersonation requests, including query parsing and error handling
middlewares/validators/impersonationRequests.ts Introduced Joi-based middleware for validating impersonation requests query parameters
models/impersonationRequests.ts Added Firestore model functions for fetching single/multiple impersonation requests with pagination
routes/impersonation.ts Created new Express router for /impersonation/requests with authentication, validation, and controller
routes/index.ts Registered the new /impersonation route in the main router

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Router
    participant AuthMiddleware
    participant Validator
    participant Controller
    participant Model

    Client->>Router: GET /impersonation/requests
    Router->>AuthMiddleware: Authenticate request
    AuthMiddleware->>Validator: Validate query parameters
    Validator->>Controller: Call controller if valid
    Controller->>Model: Fetch request(s) from Firestore
    Model-->>Controller: Return impersonation request(s)
    Controller-->>Client: Respond with data or error
Loading

Assessment against linked issues

Objective Addressed Explanation
Implement backend for fetching impersonation requests (filtering, pagination) (#2200)
Validate and handle query parameters for impersonation requests (#2200)
Integrate new endpoint into routing and authentication flow (#2200)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes found.

Possibly related PRs

Suggested reviewers

  • Shyam-Vishwakarma
  • AnujChhikara

Poem

A bunny hops through backend code,
With routes and models newly sowed.
Impersonation requests now in view,
With pagination hopping through.
Validators sniff each query string,
Controllers answer, models bring—
A warren of features, ready for spring! 🐇✨


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Performance Inefficient Array Operation ▹ view ✅ Fix detected
Performance Unnecessary Async Validation ▹ view ✅ Fix detected
Error Handling Missing Error Context in getImpersonationRequestById ▹ view ✅ Fix detected
Security Unconstrained ID Parameter Validation ▹ view ✅ Fix detected
Design Poor Type Safety with 'any' Usage ▹ view ✅ Fix detected
Readability Unexplained Magic Number ▹ view ✅ Fix detected
Documentation Incomplete query parameter documentation ▹ view ✅ Fix detected
Functionality Conflicting Pagination Methods ▹ view ✅ Fix detected
Performance Unnecessary Extra Queries for Pagination ▹ view
Design Inconsistent Module System Usage ▹ view ✅ Fix detected
Files scanned
File Path Reviewed
routes/impersonation.ts
routes/index.ts
middlewares/validators/impersonationRequests.ts
controllers/impersonationRequests.ts
models/impersonationRequests.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.

Loving Korbit!? Share us on LinkedIn Reddit and X

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6369916 and 478c3fc.

📒 Files selected for processing (5)
  • 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)
🧰 Additional context used
🧬 Code Graph Analysis (1)
middlewares/validators/impersonationRequests.ts (2)
types/impersonationRequest.d.ts (2)
  • CreateImpersonationRequest (62-66)
  • ImpersonationRequestResponse (54-56)
constants/requests.ts (1)
  • REQUEST_STATE (1-5)
🪛 GitHub Check: CodeQL
routes/impersonation.ts

[failure] 10-10: 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.

🪛 ESLint
routes/impersonation.ts

[error] 8-13: Replace ⏎··"/requests",⏎··authenticate,⏎··getImpersonationRequestsValidator,⏎··getImpersonationRequestsController⏎ with "/requests",·authenticate,·getImpersonationRequestsValidator,·getImpersonationRequestsController

(prettier/prettier)


[error] 15-15: Insert

(prettier/prettier)

🪛 Biome (1.9.4)
middlewares/validators/impersonationRequests.ts

[error] 30-30: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 35-35: Do not add then to an object.

(lint/suspicious/noThenProperty)

models/impersonationRequests.ts

[error] 77-77: Declare variables separately

Unsafe fix: Break out into multiple declarations

(lint/style/useSingleVarDeclarator)


[error] 84-84: This let declares a variable that is only assigned once.

'allRequests' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build (22.10.0)
🔇 Additional comments (3)
controllers/impersonationRequests.ts (1)

16-19: Feature flag implementation acknowledged.

The dev flag check aligns with the PR objectives for feature flag implementation.

middlewares/validators/impersonationRequests.ts (2)

28-38: Joi conditional validation is correctly implemented.

The static analysis warning about the then property is a false positive. This is the standard Joi syntax for conditional validation, and the mutual exclusivity rules for pagination parameters are properly implemented.

🧰 Tools
🪛 Biome (1.9.4)

[error] 30-30: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 35-35: Do not add then to an object.

(lint/suspicious/noThenProperty)


20-20: TODO comment acknowledged.

The temporary dev validator aligns with the feature flag implementation mentioned in the PR objectives.

@Suvidh-kaushik Suvidh-kaushik force-pushed the feat/get_impersonation_requests branch from c38a745 to 0901c70 Compare June 19, 2025 13:10
@Suvidh-kaushik Suvidh-kaushik force-pushed the feat/get_impersonation_requests branch from 1ecb1dd to 561db31 Compare June 19, 2025 15:05
@Suvidh-kaushik Suvidh-kaushik force-pushed the feat/get_impersonation_requests branch from 670cf97 to cd63722 Compare June 19, 2025 15:26
@Suvidh-kaushik Suvidh-kaushik force-pushed the feat/get_impersonation_requests branch from b002389 to 2a53bca Compare June 20, 2025 18:44
Copy link
Contributor

@Achintya-Chatterjee Achintya-Chatterjee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments have been resolved, so I am approving this PR.

Copy link
Contributor

@Achintya-Chatterjee Achintya-Chatterjee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my comments has been resolved, hence approving this PR

Comment on lines 72 to 81
if (query.id) {
const request = await getImpersonationRequestById(query.id);
if (!request) {
return res.status(204).send();
}
return res.status(200).json({
message: REQUEST_FETCHED_SUCCESSFULLY,
data: request,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why query param for a single record?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I use id as a path param instead of a query param, i would need to create another route which handles this specifically else in the same route I cannot send both the request by id and filter the requests also I have tried keeping this route consistent with the existing requests endpoint which handles this similarly

Comment on lines 132 to 134
if (page) {
const startAfter = (page - 1) * size;
requestQueryDoc = requestQueryDoc.offset(startAfter);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is page required?
Do you know the limitations of using page in firesotre

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry sir, I did not about this but from what I learned now,
in firestore, the offset skips the documents, but it internally still reads all these skipped docs and we are billed for this extra reading also
I added this because exisitng requests has both cursor and page based pagination
I will remove page based pagination and keep only the cursor based

Comment on lines +113 to +120
if (createdBy) {
requestQuery = requestQuery.where("createdBy", "==", createdBy);
}
if (status) {
requestQuery = requestQuery.where("status", "==", status);
}
if (createdFor) {
requestQuery = requestQuery.where("createdFor", "==", createdFor);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we need to create index for the queries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, These queries require indexes I have created an issue link for this and tagged you - ISSUE LINK

@Achintya-Chatterjee Achintya-Chatterjee self-requested a review June 24, 2025 07:10
@prakashchoudhary07 prakashchoudhary07 merged commit 8c27d4c into RealDevSquad:develop Jun 25, 2025
4 of 5 checks passed
@Achintya-Chatterjee Achintya-Chatterjee mentioned this pull request Jul 4, 2025
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Impersonation Feature For RDS priviledged users(super_user) #2

4 participants