-
Notifications
You must be signed in to change notification settings - Fork 273
feat: implement OOO approve/reject functionality #2461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add OOO request approval/rejection logic - Update request validation and error handling - Add tests for OOO approval workflow
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 acknowledgment flow for Out-of-Office (OOO) requests, including new controller, service, model, middleware, validation logic, and TypeScript types. It adds new constants for logging and error messages, implements request ID lookup, and provides comprehensive tests for the new feature and its validators. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Validator
participant Service
participant Model
participant Logger
User->>Controller: PATCH /requests/:id (OOO acknowledgment)
Controller->>Validator: Validate request body
Validator-->>Controller: Pass/fail (400 on fail)
Controller->>Service: acknowledgeOooRequest(requestId, body, superUserId)
Service->>Model: getRequestById(requestId)
Model-->>Service: Request data or error
Service->>Service: validateOooAcknowledgeRequest(type, status)
Service->>Model: updateRequestStatus(requestId, body, superUserId)
Service->>Logger: Log approved/rejected event
Service-->>Controller: Success message and updated request
Controller-->>User: 200 OK or error
Possibly related PRs
Suggested labels
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 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
(2 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.
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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 (3)
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.
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.
models/requests.ts (2)
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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/unit/models/requests.test.ts (6)
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: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: 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: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: 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.
middlewares/validators/requests.ts (6)
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: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: 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.
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: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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.
middlewares/validators/oooRequests.ts (6)
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.
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: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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.
test/unit/middlewares/oooRequests.test.ts (7)
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#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: 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.
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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.
controllers/requests.ts (7)
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: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: 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#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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.
controllers/oooRequests.ts (6)
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.
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: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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.
types/oooRequest.d.ts (5)
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.
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: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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.
services/oooRequest.ts (5)
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#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#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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.
constants/requests.ts (5)
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.
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: 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.
test/integration/requests.test.ts (7)
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#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: 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.
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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.
test/unit/services/oooRequest.test.ts (6)
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#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: Real-Dev-Squad/website-backend#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: Real-Dev-Squad/website-backend#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: 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.
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#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.
🧬 Code Graph Analysis (7)
models/requests.ts (1)
constants/requests.ts (2)
REQUEST_DOES_NOT_EXIST
(47-47)ERROR_WHILE_FETCHING_REQUEST
(41-41)
test/unit/models/requests.test.ts (3)
models/requests.ts (1)
getRequestById
(73-86)test/fixtures/oooRequest/oooRequest.ts (1)
createOooRequests3
(162-169)constants/requests.ts (1)
REQUEST_DOES_NOT_EXIST
(47-47)
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-203)types/oooRequest.d.ts (2)
AcknowledgeOooRequest
(56-61)OooRequestResponse
(37-37)
controllers/requests.ts (5)
constants/requests.ts (1)
REQUEST_TYPE
(13-19)middlewares/validators/oooRequests.ts (1)
acknowledgeOooRequest
(70-83)controllers/oooRequests.ts (1)
acknowledgeOooRequest
(164-198)services/oooRequest.ts (1)
acknowledgeOooRequest
(148-203)types/oooRequest.d.ts (2)
AcknowledgeOooRequest
(56-61)OooRequestResponse
(37-37)
controllers/oooRequests.ts (4)
middlewares/validators/oooRequests.ts (1)
acknowledgeOooRequest
(70-83)services/oooRequest.ts (1)
acknowledgeOooRequest
(148-203)types/oooRequest.d.ts (2)
AcknowledgeOooRequest
(56-61)OooRequestResponse
(37-37)constants/requests.ts (3)
UNAUTHORIZED_TO_UPDATE_REQUEST
(74-74)REQUEST_ID_REQUIRED
(46-46)ERROR_WHILE_ACKNOWLEDGING_REQUEST
(44-44)
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)
test/integration/requests.test.ts (2)
test/fixtures/oooRequest/oooRequest.ts (1)
testAcknowledgeOooRequest
(171-175)constants/requests.ts (2)
UNAUTHORIZED_TO_UPDATE_REQUEST
(74-74)REQUEST_STATE
(1-5)
🔇 Additional comments (24)
test/fixtures/oooRequest/oooRequest.ts (1)
171-171
: LGTM: Clear naming improvement for test fixture.Renaming
acknowledgeOooRequest
totestAcknowledgeOooRequest
makes it clearer that this is a test fixture, improving code readability.models/requests.ts (2)
11-11
: LGTM: Proper import for HTTP error handling.The
NotFound
import is correctly added to support the newgetRequestById
function.
73-86
: LGTM: Well-implemented function with proper error handling.The
getRequestById
function follows good practices:
- Clear and simple function signature
- Proper error handling with try-catch
- Throws specific HTTP
NotFound
error when document doesn't exist- Appropriate error logging with descriptive messages
- Rethrows errors to maintain error propagation
test/unit/models/requests.test.ts (2)
3-3
: LGTM: Proper imports for new test functionality.The imports correctly include the new
getRequestById
function, test fixturecreateOooRequests3
, and theREQUEST_DOES_NOT_EXIST
constant needed for the tests.Also applies to: 7-7, 12-12
184-198
: LGTM: Comprehensive test coverage forgetRequestById
.The test suite properly covers both success and failure scenarios:
- Successfully retrieves request data and validates it matches the expected fixture
- Properly tests error handling for non-existent requests with correct error message
- Uses appropriate assertions (
deep.include
and error message validation)- Follows established testing patterns in the file
controllers/requests.ts (2)
8-9
: LGTM: Proper imports for OOO acknowledge functionality.The imports correctly include the new
acknowledgeOooRequest
controller function, its types, and theNextFunction
type needed for the updated function signature.Also applies to: 19-19
123-129
: LGTM: Proper integration of OOO acknowledgment flow.The function signature correctly includes the
NextFunction
parameter, and the new case forREQUEST_TYPE.OOO
properly delegates to theacknowledgeOooRequest
controller with all required parameters. This maintains consistency with the existing controller pattern.test/integration/requests.test.ts (4)
18-18
: LGTM: Consistent fixture import update.The import correctly uses the renamed
testAcknowledgeOooRequest
fixture, maintaining consistency with the changes in the test fixtures file.
326-326
: LGTM: Enabling important integration tests.The previously skipped test suite for PATCH /requests/:id is now enabled, providing essential integration test coverage for the OOO request acknowledgment functionality.
33-33
: LGTM: Consistent authorization error constant usage.The update from the commented-out
UNAUTHORIZED_TO_ACKNOWLEDGE_OOO_REQUEST
toUNAUTHORIZED_TO_UPDATE_REQUEST
aligns with the actual error constants used in the implementation, ensuring test assertions match the expected behavior.Also applies to: 407-407
355-355
: LGTM: Consistent test fixture usage throughout.All references to the test payload have been consistently updated to use
testAcknowledgeOooRequest
, maintaining uniformity across all test cases and ensuring the tests use the correct fixture data.Also applies to: 369-369, 385-385, 401-401, 417-417, 433-433, 449-449, 465-465, 481-481, 498-498
middlewares/validators/oooRequests.ts (2)
4-4
: LGTM: Import statement updated correctly.The addition of
AcknowledgeOooRequest
type import aligns with the new acknowledge OOO request functionality.
42-60
: LGTM: Joi schema correctly validates acknowledge OOO request payload.The schema properly validates:
comment
as optional string with empty string validationstatus
restricted to APPROVED/REJECTED with clear error messagetype
must equal OOO with validationThe validation rules align with the acknowledge OOO request requirements.
middlewares/validators/requests.ts (2)
4-5
: LGTM: Import statements correctly updated.The imports properly include the new
AcknowledgeOooRequest
type andacknowledgeOooRequest
validator function needed for the OOO acknowledge functionality.
128-154
: LGTM: updateRequestValidator correctly integrates OOO acknowledge validation.The function signature update to include
AcknowledgeOooRequest
and the new case forREQUEST_TYPE.OOO
properly integrate the acknowledge OOO request validation into the existing request validation flow. The implementation follows the established pattern used for other request types.test/unit/middlewares/oooRequests.test.ts (2)
7-9
: LGTM: Import statements updated to use active validator and test data.The imports correctly reference the implemented
acknowledgeOooRequest
validator function and updated test data naming convention.
94-130
: LGTM: Test suite properly activated and updated.The test suite for
acknowledgeOooRequestsValidator
is now enabled and all test cases properly:
- Test invalid request type validation
- Test invalid status validation
- Test optional comment handling
- Test valid acknowledge requests
- Use the correct validator function and test data references
The test coverage adequately validates the middleware behavior.
controllers/oooRequests.ts (1)
15-17
: LGTM: Import statements properly updated for acknowledge functionality.The new imports for constants, service functions, types, and NextFunction are correctly added to support the acknowledge OOO request feature.
Also applies to: 26-26, 28-28, 30-30
constants/requests.ts (3)
29-30
: LGTM: New log types for already approved/rejected requests.The addition of
REQUEST_ALREADY_APPROVED
andREQUEST_ALREADY_REJECTED
log types follows the existing naming convention and supports the acknowledge OOO request validation flow.
44-44
: LGTM: Error message constant for acknowledge functionality.The
ERROR_WHILE_ACKNOWLEDGING_REQUEST
constant provides appropriate error messaging for the new acknowledge OOO request feature.
46-46
: LGTM: Validation message for required request ID.The
REQUEST_ID_REQUIRED
constant provides clear validation messaging when request ID is missing, supporting the acknowledge functionality validation.services/oooRequest.ts (2)
107-203
: Well-implemented acknowledgment flowThe new functions properly implement the OOO request acknowledgment flow with:
- Clear separation between validation and business logic
- Comprehensive error handling for all edge cases
- Proper logging of all actions
- Correct status transitions
182-190
: Verify naming consistency for addFutureStatus payloadI wasn’t able to locate the
addFutureStatus
definition in themodels/userStatus
module to confirm whether it expects amessage
orreason
field. Please double-check the function signature and update the call below if it should usereason
instead ofmessage
:• services/oooRequest.ts:182
await addFutureStatus({ requestId, state: REQUEST_TYPE.OOO, from: requestData.from, endsOn: requestData.until, userId: requestData.userId, message: body.comment, // ← verify if this should be `reason` });test/unit/services/oooRequest.test.ts (1)
117-264
: Comprehensive test coverage for acknowledgment functionalityExcellent test implementation that covers:
- All validation scenarios (invalid type, already approved/rejected)
- Success cases for both approval and rejection
- Error handling for various failure modes
- Proper stubbing and isolation of dependencies
There was a problem hiding this 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 |
---|---|---|
Inconsistent Validation Error Key ▹ view | ||
Inconsistent Module Import Pattern ▹ view |
Files scanned
File Path | Reviewed |
---|---|
types/oooRequest.d.ts | ✅ |
middlewares/validators/oooRequests.ts | ✅ |
constants/requests.ts | ✅ |
models/requests.ts | ✅ |
controllers/requests.ts | ✅ |
middlewares/validators/requests.ts | ✅ |
controllers/oooRequests.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.
fec48b1
to
83d07c7
Compare
|
||
const requestResult = await updateRequest(requestId, body, superUserId, REQUEST_TYPE.OOO); | ||
|
||
if ("error" in requestResult) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we doing this ?
userId: requestData.userId, | ||
message: body.comment, | ||
createdAt: Date.now() | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does both function do
Date: 19 Jul, 2025
Developer Name: @tejaskh3
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
Description by Korbit AI
What change is being made?
Implement the functionality to approve or reject Out-of-Office (OOO) requests, including the addition of a new API endpoint, request validation, logging enhancements, and integration with existing request handling logic.
Why are these changes being made?
To provide management with the ability to acknowledge OOO requests through approval or rejection, ensuring that requests are processed correctly and consistently. This includes validation to prevent already acknowledged requests from being processed again, aligning with business rules, and improving error handling and messaging.