feat: implement OOO approve/reject functionality #2462
Conversation
- Add OOO request approval/rejection logic - Update request validation and error handling - Add tests for OOO approval workflow
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 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 change introduces the ability to acknowledge Out-of-Office (OOO) requests, including new controller and service logic, validation middleware, type definitions, and updates to request models. It also adds and revises corresponding tests for these functionalities, including integration and unit tests, and extends constant definitions for error handling and logging. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Validator
participant Controller
participant Service
participant Model
participant Logger
Client->>Validator: Send PATCH /requests/:id (OOO)
Validator->>Controller: If valid, pass request
Controller->>Service: acknowledgeOooRequest(requestId, body, superUserId)
Service->>Model: getRequestById(requestId)
Model-->>Service: Return request data or error
Service->>Service: validateOooAcknowledgeRequest(type, status)
Service->>Model: updateRequest(requestId, updateData)
Model-->>Service: Return updated request
Service->>Logger: Log approval/rejection
Service-->>Controller: Return success message and data
Controller-->>Client: Respond with status and data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 |
|---|---|---|
 |
Inconsistent Error Handler Spacing ▹ view | ✅ Fix detected |
 |
Test comment in production code ▹ view | ✅ Fix detected |
|
Inconsistent Function Declaration Format ▹ view | ✅ Fix detected |
 |
Insufficient Error Log Context ▹ view | ✅ Fix detected |
 |
Double Response in Error Handler ▹ view | ✅ Fix detected |
 |
Sensitive Error Exposure ▹ view | ✅ Fix detected |
|
Incomplete Documentation for Acknowledge Function ▹ view | ✅ Fix detected |
 |
Missing Request Body Validation ▹ view | ✅ Fix detected |
|
Unclear Comment Field Purpose ▹ view | |
 |
Inconsistent Module Import Pattern ▹ view | ✅ Fix detected |
Files scanned
| File Path | Reviewed |
|---|---|
| types/oooRequest.d.ts | ✅ |
| middlewares/validators/oooRequests.ts | ✅ |
| constants/requests.ts | ✅ |
| controllers/requests.ts | ✅ |
| middlewares/validators/requests.ts | ✅ |
| controllers/oooRequests.ts | ✅ |
| models/requests.ts | ✅ |
| services/oooRequest.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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
controllers/requests.ts (1)
116-123: Update JSDoc to include thenextparameterThe function signature was updated to include
next: NextFunction, but the JSDoc comment is missing documentation for this parameter./** * Processes update requests before acknowledgment based on type. * * @param {Request} req - The request object. * @param {CustomResponse} res - The response object. + * @param {NextFunction} next - The next middleware function. * @returns {Promise<void>} Resolves or sends an error for invalid types. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
constants/requests.ts(2 hunks)controllers/oooRequests.ts(3 hunks)controllers/requests.ts(3 hunks)middlewares/validators/oooRequests.ts(2 hunks)middlewares/validators/requests.ts(3 hunks)models/requests.ts(5 hunks)services/oooRequest.ts(2 hunks)test/fixtures/oooRequest/oooRequest.ts(1 hunks)test/integration/requests.test.ts(13 hunks)test/unit/middlewares/oooRequests.test.ts(2 hunks)test/unit/models/requests.test.ts(2 hunks)test/unit/services/oooRequest.test.ts(5 hunks)types/oooRequest.d.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The `createOooRequests2` object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names `message` and `state` (instead of `reason` and `status`) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between `message` in types/userCurrentStatus.d.ts and `reason` in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
test/unit/services/oooRequest.test.ts (6)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
test/fixtures/oooRequest/oooRequest.ts (5)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
controllers/oooRequests.ts (6)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
test/unit/models/requests.test.ts (6)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
services/oooRequest.ts (5)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
test/integration/requests.test.ts (7)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/models/impersonationRequests.test.ts:13-13
Timestamp: 2025-06-19T16:38:41.156Z
Learning: When reviewing test PRs that depend on feature PRs, import errors may be temporary and expected until the feature PR is merged. Users may be updating type names or exports in the feature PR that the test PR depends on.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
controllers/requests.ts (7)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
Learnt from: Suvidh-kaushik
PR: #2442
File: routes/impersonation.ts:15-15
Timestamp: 2025-06-18T10:01:24.320Z
Learning: In the Real-Dev-Squad/website-backend codebase, route modules use ES6 imports internally but export using CommonJS module.exports syntax because the routes/index.ts file uses require-based imports to load route modules.
middlewares/validators/requests.ts (6)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
models/requests.ts (6)
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
constants/requests.ts (5)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
middlewares/validators/oooRequests.ts (5)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
types/oooRequest.d.ts (5)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
test/unit/middlewares/oooRequests.test.ts (7)
Learnt from: surajmaity1
PR: #2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The createOooRequests2 object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names message and state (instead of reason and status) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: surajmaity1
PR: #2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/middlewares/impersonationRequests.test.ts:12-19
Timestamp: 2025-06-19T17:06:24.352Z
Learning: In test/unit/middlewares/impersonationRequests.test.ts, the developer uses any type for req and res mock objects to enable reusability across different middleware tests (get, update, create) in the same file, as strict typing causes test failures when different middleware functions expect different request/response shapes.
Learnt from: Suvidh-kaushik
PR: #2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions updateImpersonationRequest and getImpersonationRequestById are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: surajmaity1
PR: #2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between message in types/userCurrentStatus.d.ts and reason in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
Learnt from: Suvidh-kaushik
PR: #2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion as UpdateImpersonationStatusModelResponse is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: Suvidh-kaushik
PR: #2443
File: test/unit/models/impersonationRequests.test.ts:13-13
Timestamp: 2025-06-19T16:38:41.156Z
Learning: When reviewing test PRs that depend on feature PRs, import errors may be temporary and expected until the feature PR is merged. Users may be updating type names or exports in the feature PR that the test PR depends on.
🧬 Code Graph Analysis (8)
test/unit/services/oooRequest.test.ts (3)
services/oooRequest.ts (2)
validateOooAcknowledgeRequest(115-137)acknowledgeOooRequest(148-213)constants/requests.ts (8)
REQUEST_TYPE(13-19)INVALID_REQUEST_TYPE(72-72)REQUEST_STATE(1-5)REQUEST_ALREADY_APPROVED(39-39)REQUEST_ALREADY_REJECTED(40-40)REQUEST_DOES_NOT_EXIST(48-48)REQUEST_APPROVED_SUCCESSFULLY(35-35)REQUEST_REJECTED_SUCCESSFULLY(36-36)test/fixtures/oooRequest/oooRequest.ts (1)
testAcknowledgeOooRequest(171-175)
controllers/oooRequests.ts (4)
services/oooRequest.ts (1)
acknowledgeOooRequest(148-213)middlewares/validators/oooRequests.ts (1)
acknowledgeOooRequest(70-83)types/oooRequest.d.ts (2)
AcknowledgeOooRequest(56-61)OooRequestResponse(37-37)constants/requests.ts (3)
UNAUTHORIZED_TO_UPDATE_REQUEST(75-75)REQUEST_ID_REQUIRED(47-47)ERROR_WHILE_ACKNOWLEDGING_REQUEST(45-45)
test/unit/models/requests.test.ts (3)
models/requests.ts (1)
getRequestById(74-87)test/fixtures/oooRequest/oooRequest.ts (1)
createOooRequests3(162-169)constants/requests.ts (1)
REQUEST_DOES_NOT_EXIST(48-48)
controllers/requests.ts (5)
constants/requests.ts (1)
REQUEST_TYPE(13-19)controllers/oooRequests.ts (1)
acknowledgeOooRequest(164-198)services/oooRequest.ts (1)
acknowledgeOooRequest(148-213)middlewares/validators/oooRequests.ts (1)
acknowledgeOooRequest(70-83)types/oooRequest.d.ts (2)
AcknowledgeOooRequest(56-61)OooRequestResponse(37-37)
middlewares/validators/requests.ts (5)
types/oooRequest.d.ts (2)
AcknowledgeOooRequest(56-61)OooRequestResponse(37-37)constants/requests.ts (1)
REQUEST_TYPE(13-19)controllers/oooRequests.ts (1)
acknowledgeOooRequest(164-198)services/oooRequest.ts (1)
acknowledgeOooRequest(148-213)middlewares/validators/oooRequests.ts (1)
acknowledgeOooRequest(70-83)
models/requests.ts (1)
constants/requests.ts (3)
REQUEST_DOES_NOT_EXIST(48-48)ERROR_WHILE_FETCHING_REQUEST(42-42)REQUEST_TYPE(13-19)
middlewares/validators/oooRequests.ts (4)
constants/requests.ts (2)
REQUEST_STATE(1-5)REQUEST_TYPE(13-19)controllers/oooRequests.ts (1)
acknowledgeOooRequest(164-198)services/oooRequest.ts (1)
acknowledgeOooRequest(148-213)types/oooRequest.d.ts (2)
AcknowledgeOooRequest(56-61)OooRequestResponse(37-37)
types/oooRequest.d.ts (2)
types/requests.d.ts (2)
RequestQuery(11-20)RequestParams(22-24)constants/requests.ts (2)
REQUEST_TYPE(13-19)REQUEST_STATE(1-5)
🪛 ESLint
constants/requests.ts
[error] 31-31: Expected space or tab after '//' in comment.
(spaced-comment)
🔇 Additional comments (21)
test/fixtures/oooRequest/oooRequest.ts (1)
171-175: LGTM! Good naming convention improvement.The rename from
acknowledgeOooRequesttotestAcknowledgeOooRequestimproves test fixture naming clarity by explicitly indicating this is test data.constants/requests.ts (1)
45-47: LGTM! Well-named error constants.The new error message constants
ERROR_WHILE_ACKNOWLEDGING_REQUESTandREQUEST_ID_REQUIREDare appropriately named and follow the existing naming convention.test/unit/models/requests.test.ts (4)
3-3: LGTM! Proper import addition.The
getRequestByIdfunction import is correctly added to support the new test cases.
7-7: LGTM! Appropriate test fixture import.The
createOooRequests3fixture import provides the necessary test data for the newgetRequestByIdtests.
12-12: LGTM! Required constant import.The
REQUEST_DOES_NOT_EXISTconstant import is needed for testing error scenarios in the new test cases.
184-198: LGTM! Comprehensive test coverage for getRequestById.The new test block provides excellent coverage for the
getRequestByIdfunction:
- Success case: Verifies that a valid request ID returns the correct request data
- Error case: Ensures that an invalid request ID throws the appropriate
REQUEST_DOES_NOT_EXISTerrorThe tests follow the existing patterns in the file and properly validate both happy path and error scenarios.
test/integration/requests.test.ts (4)
18-18: LGTM! Fixture import updated correctly.The import has been updated to use
testAcknowledgeOooRequestto align with the renamed fixture in the test fixtures file.
33-33: LGTM! Error constant updated appropriately.The import has been updated from
UNAUTHORIZED_TO_ACKNOWLEDGE_OOO_REQUESTtoUNAUTHORIZED_TO_UPDATE_REQUESTto reflect the updated error handling constants.
326-326: LGTM! Integration tests activated.The PATCH /requests/:id test suite has been activated (removed
.skip()), indicating the OOO acknowledge functionality is ready for integration testing. This is a positive step in the feature development process.
355-499: LGTM! Comprehensive integration test coverage.The test suite provides excellent end-to-end coverage for the OOO acknowledge functionality:
- Authentication tests: Proper 401 responses for unauthenticated users
- Authorization tests: Correct 403 responses for non-super users
- Validation tests: Appropriate error handling for invalid requests
- Business logic tests: Success scenarios for approve/reject operations
- Error handling tests: 500 responses for unexpected errors
All test requests consistently use the
testAcknowledgeOooRequestfixture, maintaining consistency across the test suite.middlewares/validators/requests.ts (4)
4-5: LGTM! Proper imports for OOO acknowledgment validation.The imports correctly add the
AcknowledgeOooRequesttype andacknowledgeOooRequestvalidator function needed for the new validation case.
128-128: LGTM! Documentation updated correctly.The JSDoc comment has been properly updated to reflect that the function now accepts both
UpdateOnboardingExtensionRequestandAcknowledgeOooRequesttypes.
134-134: LGTM! Type-safe function signature.The function parameter type has been correctly updated to use a union type, maintaining type safety while supporting both onboarding and OOO request validation.
141-145: LGTM! Consistent validation pattern.The new
REQUEST_TYPE.OOOcase follows the same pattern as the existing onboarding case, properly delegating to the specializedacknowledgeOooRequestvalidator with appropriate type casting.controllers/requests.ts (1)
127-129: LGTM!The OOO request acknowledgment case is properly integrated with correct type assertions and follows the existing pattern.
types/oooRequest.d.ts (1)
46-61: LGTM!The new type definitions are well-structured and properly extend existing types. They correctly define the shape of acknowledge OOO requests.
services/oooRequest.ts (3)
8-24: Imports look good!All new imports are properly used in the new functions for OOO request acknowledgment functionality.
160-162: Error handling guard is correct
The updateRequest model returns an{ error }object for logical failures (and only throws on unexpected errors), so theif ("error" in requestResult)check in services/oooRequest.ts is appropriate. No changes needed.
182-200: Future status creation pattern verifiedBoth calls target distinct models—
addFutureStatusrecords the request-level future status usingREQUEST_TYPE.OOO, andcreateUserFutureStatusrecords the user’s upcoming status withuserState.OOOandstatusState.UPCOMING. They write to separate tables, so no duplicate entries will occur and both enum usages are correct.test/unit/middlewares/oooRequests.test.ts (1)
7-9: Test implementation looks good!The new
acknowledgeOooRequestsValidatortest suite provides comprehensive coverage for the acknowledge OOO request validator, including:
- Invalid request type validation
- Incorrect status validation
- Optional comment field handling
- Valid request scenarios
The tests follow the established patterns and properly verify that
nextSpyis called or not called as expected.Also applies to: 94-130
test/unit/services/oooRequest.test.ts (1)
169-264: Comprehensive test coverage!The
acknowledgeOooRequesttest suite provides excellent coverage including:
- Invalid request ID handling
- Already approved/rejected validation
- Update failure scenarios
- Successful approval and rejection flows
- General error handling
The tests properly use sinon stubs to isolate the unit under test.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }; | ||
|
|
||
| export const getRequestById = async (id: string): Promise<{ id: string; [key: string]: any }> => { | ||
| try { |
There was a problem hiding this comment.
Can you please check with @Suvidh-kaushik what function he used for the impersonation feature to get request?
There was a problem hiding this comment.
fetch by id was handled in a seperate model function
| const superUserId = req.userData.id; | ||
| const requestId = req.params.id; | ||
|
|
||
| if (!requestId) { |
There was a problem hiding this comment.
we can perform this check if the validator
| } | ||
| } | ||
| } | ||
| allRequests = oooRequests; |
| }; | ||
|
|
||
| export const getRequestById = async (id: string): Promise<{ id: string; [key: string]: any }> => { | ||
| try { |
There was a problem hiding this comment.
fetch by id was handled in a seperate model function
Date: 03-08-2025
Developer Name: @RishiChaubey31
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
(it has already been changed in prev PR by Suraj and some index issue-2414 are also created
Breaking Changes
Development Tested?
Screenshots
Test Coverage
Additional Notes