feat: implement OOO acknowledgement functionality #2471
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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
WalkthroughAdds OOO request acknowledgement flow: new constants and types; controller to acknowledge OOO requests; validators and conditional middleware; routing update to insert conditional checks; services extended to validate/acknowledge and refactor OOO creation signature; model helper to fetch request by ID; controllers wired to new flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant R as Router
participant Auth as authenticate
participant Cond as conditionalOooChecks
participant Val as updateRequestValidator
participant Ctrl as updateRequestBeforeAcknowledgedController
participant OCtrl as acknowledgeOooRequest (controller)
participant S as oooRequestService
participant M as Models (getRequestById/updateRequest)
participant FS as Future Status Services
C->>R: PATCH /requests/:id
R->>Auth: authenticate
Auth-->>R: ok
R->>Cond: conditionalOooChecks
alt type === OOO
Cond->>Cond: devFlagMiddleware
Cond->>Cond: authorizeRoles([SUPERUSER])
else other types
Cond-->>R: next()
end
R->>Val: updateRequestValidator
alt type === OOO
Val-->>Ctrl: next()
Ctrl->>OCtrl: delegate (req,res,next)
OCtrl->>S: acknowledgeOooRequest(id, body, superUserId)
S->>M: getRequestById(id)
S->>S: validateOooAcknowledgeRequest()
S->>M: updateRequest(...)
alt status APPROVED
S->>FS: addFutureStatus(...)
S->>FS: createUserFutureStatus(...)
end
S-->>OCtrl: {message}
OCtrl-->>C: 200 {message}
else other types
Ctrl->>Ctrl: existing flows
Ctrl-->>C: response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. Comment |
![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 |
|---|---|---|
 |
Missing Required Parameter in Service Call ▹ view | |
 |
Inconsistent Module Import Pattern ▹ view | ✅ Fix detected |
Files scanned
| File Path | Reviewed |
|---|---|
| middlewares/conditionalOooChecks.ts | ✅ |
| types/oooRequest.d.ts | ✅ |
| routes/requests.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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
routes/requests.ts (1)
1-6: Fix ESLint “import/first”: move router initialization below imports
const router = express.Router();(Line 2) precedes animportstatement on Line 6, triggering import/first. Move router initialization after all imports.-import express from "express"; -const router = express.Router(); +import express from "express"; const authorizeRoles = require("../middlewares/authorizeRoles"); const { SUPERUSER } = require("../constants/roles"); import authenticate from "../middlewares/authenticate"; import { conditionalOooChecks } from "../middlewares/conditionalOooChecks"; +const router = express.Router();controllers/requests.ts (2)
1-6: Reuse constant for invalid type to keep responses consistentUse
INVALID_REQUEST_TYPEfrom constants instead of string literals.import { ERROR_WHILE_FETCHING_REQUEST, REQUEST_FETCHED_SUCCESSFULLY, - REQUEST_TYPE, + REQUEST_TYPE, + INVALID_REQUEST_TYPE, } from "../constants/requests";
124-136: Wire OOO acknowledge path looks correct; unify default error messageThe OOO branch correctly forwards to
acknowledgeOooRequestwithnext. For the default branch, prefer the shared constant.default: - return res.boom.badRequest("Invalid request"); + return res.boom.badRequest(INVALID_REQUEST_TYPE);Optional:
return await acknowledgeOooRequest(...)to make flow explicit (not required since you break immediately).types/oooRequest.d.ts (1)
17-18: createdAt/updatedAt type mismatch with actual model valuesThe models set
createdAt/updatedAtviaDate.now()(number). Declaring them asTimestamphere is inconsistent and can mislead callers.- createdAt: Timestamp; - updatedAt: Timestamp; + createdAt: number; + updatedAt: number;Also change
OooRequestUpdateBody.updatedAtfromadmin.firestore.Timestamptonumberfor consistency.controllers/oooRequests.ts (1)
48-53: Remove unused variable
usernameis no longer used after changing the service signature.- const { id: userId, username } = req.userData; + const { id: userId } = req.userData;middlewares/validators/requests.ts (1)
132-151: Delegate OOO updates to the acknowledge validator, but ensure type narrowingGood call to route OOO updates through the new acknowledge validator. For clarity to TS, cast
reqtoAcknowledgeOooRequestat the call site.- case REQUEST_TYPE.OOO: - await acknowledgeOooRequest(req, res as OooRequestResponse, next); + case REQUEST_TYPE.OOO: + await acknowledgeOooRequest(req as AcknowledgeOooRequest, res as OooRequestResponse, next); break;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
constants/requests.ts(2 hunks)controllers/oooRequests.ts(4 hunks)controllers/requests.ts(3 hunks)middlewares/conditionalOooChecks.ts(1 hunks)middlewares/validators/oooRequests.ts(2 hunks)middlewares/validators/requests.ts(4 hunks)models/requests.ts(2 hunks)routes/requests.ts(2 hunks)services/oooRequest.ts(4 hunks)types/oooRequest.d.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 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.
📚 Learning: 2025-03-16T05:23:33.460Z
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.
Applied to files:
types/oooRequest.d.tscontrollers/requests.tscontrollers/oooRequests.tsmiddlewares/validators/oooRequests.tsconstants/requests.tsservices/oooRequest.tsmiddlewares/validators/requests.ts
📚 Learning: 2025-03-09T06:30:20.120Z
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.
Applied to files:
types/oooRequest.d.tscontrollers/requests.tscontrollers/oooRequests.tsservices/oooRequest.ts
📚 Learning: 2025-06-26T20:08:47.146Z
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.
Applied to files:
models/requests.ts
📚 Learning: 2025-03-16T05:28:26.722Z
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.
Applied to files:
controllers/requests.tscontrollers/oooRequests.tsservices/oooRequest.tsmiddlewares/validators/requests.ts
📚 Learning: 2025-06-22T16:48:33.847Z
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.
Applied to files:
controllers/oooRequests.tsservices/oooRequest.ts
🧬 Code graph analysis (9)
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)
middlewares/conditionalOooChecks.ts (2)
constants/requests.ts (1)
REQUEST_TYPE(13-19)middlewares/devFlag.ts (1)
devFlagMiddleware(4-15)
routes/requests.ts (3)
middlewares/conditionalOooChecks.ts (1)
conditionalOooChecks(16-32)middlewares/validators/requests.ts (1)
updateRequestValidator(137-155)controllers/requests.ts (1)
updateRequestBeforeAcknowledgedController(124-136)
models/requests.ts (1)
constants/requests.ts (1)
ERROR_WHILE_FETCHING_REQUEST(41-41)
controllers/requests.ts (4)
constants/requests.ts (1)
REQUEST_TYPE(13-19)controllers/oooRequests.ts (1)
acknowledgeOooRequest(163-197)middlewares/validators/oooRequests.ts (1)
acknowledgeOooRequest(71-84)types/oooRequest.d.ts (2)
AcknowledgeOooRequest(56-61)OooRequestResponse(37-37)
controllers/oooRequests.ts (4)
services/oooRequest.ts (2)
createOooRequest(67-101)acknowledgeOooRequest(142-197)middlewares/validators/oooRequests.ts (1)
acknowledgeOooRequest(71-84)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)
middlewares/validators/oooRequests.ts (5)
constants/requests.ts (2)
REQUEST_STATE(1-5)REQUEST_TYPE(13-19)controllers/oooRequests.ts (1)
acknowledgeOooRequest(163-197)services/oooRequest.ts (1)
acknowledgeOooRequest(142-197)test/fixtures/oooRequest/oooRequest.ts (1)
acknowledgeOooRequest(170-174)types/oooRequest.d.ts (2)
AcknowledgeOooRequest(56-61)OooRequestResponse(37-37)
services/oooRequest.ts (6)
types/oooRequest.d.ts (3)
OooStatusRequestBody(21-26)OooStatusRequest(7-20)AcknowledgeOooRequestBody(50-54)models/requests.ts (3)
createRequest(15-32)getRequestById(74-85)updateRequest(34-72)constants/requests.ts (9)
REQUEST_TYPE(13-19)INVALID_REQUEST_TYPE(71-71)REQUEST_STATE(1-5)REQUEST_ALREADY_APPROVED(38-38)REQUEST_ALREADY_REJECTED(39-39)REQUEST_LOG_TYPE(21-31)REQUEST_APPROVED_SUCCESSFULLY(34-34)REQUEST_REJECTED_SUCCESSFULLY(35-35)LOG_ACTION(7-11)models/userStatus.js (1)
addFutureStatus(678-706)models/userFutureStatus.ts (1)
createUserFutureStatus(11-26)constants/userStatus.ts (2)
userState(1-6)statusState(8-12)
middlewares/validators/requests.ts (5)
constants/requests.ts (2)
REQUEST_STATE(1-5)REQUEST_TYPE(13-19)types/oooRequest.d.ts (2)
AcknowledgeOooRequest(56-61)OooRequestResponse(37-37)controllers/oooRequests.ts (1)
acknowledgeOooRequest(163-197)middlewares/validators/oooRequests.ts (1)
acknowledgeOooRequest(71-84)services/oooRequest.ts (1)
acknowledgeOooRequest(142-197)
🪛 ESLint
routes/requests.ts
[error] 6-6: Import in body of module; reorder to top.
(import/first)
[error] 26-26: Replace "/:id",·authenticate,·conditionalOooChecks,·updateRequestValidator,·updateRequestBeforeAcknowledgedController with ⏎··"/:id",⏎··authenticate,⏎··conditionalOooChecks,⏎··updateRequestValidator,⏎··updateRequestBeforeAcknowledgedController⏎
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
10 tasks
Suvidh-kaushik
requested changes
Aug 28, 2025
- Removed unnecessary checks for development mode and user roles in createOooRequestController. - Improved documentation for acknowledgeOooRequest function to clarify parameters and return types. - Added validation for request parameters in acknowledgeOooRequest middleware. - Updated error handling in getRequestById to use a constant for not found messages. - Cleaned up imports in requests model to remove unused constants.
- Added role validation to ensure only users part of Discord can create OOO requests. - Implemented a check for development mode to restrict access to the feature. - Updated the controller to handle unauthorized access appropriately.
Suvidh-kaushik
requested changes
Aug 30, 2025
Contributor
|
@RishiChaubey31 please attach the working proof that user status is getting changed after approving the request |
Contributor
Author
I have added the proofs in screenshots |
AnujChhikara
reviewed
Sep 1, 2025
- Updated createOooRequestController to use username instead of roles for user identification. - Introduced conditionalOooChecks middleware to apply role and dev checks specifically for OOO requests. - Improved validation error handling in acknowledgeOooRequest and added constants for error messages. - Refactored request validation schemas for clarity and consistency. - Cleaned up imports and ensured proper middleware usage in routes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Removed redundant variable assignments for 'from', 'until', and 'requestedBy' in acknowledgeOooRequest function. - Directly used the parameters in subsequent function calls to enhance code clarity and reduce unnecessary lines.
- Updated the function name from acknowledgeOooRequest to acknowledgeOooRequestController for clarity and to align with naming conventions in the codebase. - Adjusted import statements accordingly in the requests controller to reflect the new function name.
…nability - Added a TODO comment to indicate the need for future removal of legacy request format handling. - Simplified the return structure by removing unnecessary data wrapping around requestResult.
- Updated requestData type assertion to include oldOooStatusRequest for better type safety. - Removed legacy request format normalization logic to streamline the function and improve clarity.
MayankBansal12
previously approved these changes
Sep 3, 2025
Suvidh-kaushik
approved these changes
Sep 3, 2025
iamitprakash
requested changes
Sep 4, 2025
iamitprakash
reviewed
Sep 4, 2025
iamitprakash
reviewed
Sep 4, 2025
iamitprakash
requested changes
Sep 4, 2025
- Enhanced the oooRoleCheckMiddleware for better error handling and code clarity. - Updated the acknowledgeOooRequestValidator to include specific Joi validation error handling. - Simplified the validateOooAcknowledgeRequest function by removing unnecessary try-catch nesting and improving logging for invalid request types.
- Updated acknowledgeOooRequestController to remove unnecessary return statement. - Enhanced acknowledgeOooRequestValidator to streamline Joi validation error handling. - Improved type assertions in updateRequestValidator for better clarity and type safety. - Refined transformRequestResponse logic to simplify request transformation based on environment conditions.
* initial commit * added-more-test * fix-test-title * removed-try/catch-block * fix-try-catch-cases * added-try-catch * removed-functon * test: update assertions in oooRequest tests to use include for response validation --------- Co-authored-by: Mayank Bansal <mayankbansal125@gmail.com>
iamitprakash
approved these changes
Sep 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Date: 25-08-2025
Developer Name: Rishi Chaubey
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
acknow-test.mp4
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Implement an Out-of-Office (OOO) acknowledgment functionality to update the status of OOO requests, including related middleware, validation, and service enhancements.
Why are these changes being made?
These changes allow superusers to acknowledge OOO requests by approving or rejecting them, thereby improving request management efficiency. This implementation ensures a clear and consistent process for handling OOO requests with proper validation and error handling.