-
Notifications
You must be signed in to change notification settings - Fork 279
Feature flag removed : user OOO approve reject #2526
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
Feature flag removed : user OOO approve reject #2526
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRemoved feature flag gating from OOO request creation controller, enabling the endpoint by default. Updated integration test endpoints from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
middlewares/validators/requests.ts (1)
84-84: Consider updating or clarifying the TODO comment.The TODO comment mentions removing the validator once the feature is "tested and ready to be used." Since this PR enables the OOO creation feature by default, you may want to update this comment to clarify that the validator now serves backward compatibility (accepting
devparameter without error) rather than being a temporary measure.test/integration/requests.test.ts (1)
163-185: Update the test description to reflect current behavior.The test description states "should create a new request when dev is true", but the endpoint no longer requires or uses the
devflag. Update the description to reflect that this tests standard OOO request creation.🔎 Suggested fix
- it("should create a new request when dev is true", function (done) { + it("should create a new OOO request", function (done) {
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
controllers/oooRequests.tsmiddlewares/validators/requests.tstest/integration/requests.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
controllers/oooRequests.ts (1)
constants/requests.ts (3)
OOO_STATUS_ALREADY_EXIST(49-49)REQUEST_ALREADY_PENDING(46-46)ERROR_WHILE_ACKNOWLEDGING_REQUEST(42-42)
🔇 Additional comments (8)
middlewares/validators/requests.ts (2)
54-70: LGTM - Formatting improvements.The indentation and spacing adjustments improve code readability without affecting the validation logic.
91-91: LGTM - Formatting improvements.The formatting adjustments maintain consistency and improve readability without changing the validation behavior.
Also applies to: 141-154
test/integration/requests.test.ts (4)
61-61: LGTM - Endpoint updated correctly.The endpoint constant correctly reflects the production path without the dev flag.
207-207: LGTM - Formatting improvements.The object spread syntax adjustments maintain consistency without affecting test behavior.
Also applies to: 222-222
339-493: LGTM - Skipped test endpoints updated consistently.Although these tests are currently skipped, the endpoint URLs have been correctly updated to remove the dev flag, maintaining consistency with the production behavior.
569-569: LGTM - GET endpoint updated correctly.The endpoint correctly reflects the production path for retrieving all requests.
controllers/oooRequests.ts (2)
41-90: LGTM - OOO creation controller enabled for production use.The feature flag gating has been successfully removed, enabling the OOO creation endpoint by default. All critical safeguards remain in place:
- Discord membership authorization check (line 50-52)
- User status validation (line 55-65)
- Duplicate pending request check (line 67-79)
- Comprehensive error handling and logging
The formatting improvements enhance readability without affecting the control flow.
159-182: LGTM - Formatting improvements in acknowledge controller.The try-catch block formatting adjustments improve code structure while preserving all error handling logic.
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.
- please try to improve PR title
- please use HTTP client like postman to test and provide demo recording as proof
- please include test run and coverage as well
- build is failing for one test please fix that
1) /requests OOO
GET /requests
should return all requests:
Uncaught AssertionError: expected { id: 'jvYQTCnv4hOdHT6aksK8', …(10) } to have property 'state'
at fn (test/integration/requests.test.ts:576:44)
at Test.Request.callback (node_modules/superagent/src/node/index.js:913:12)
at Test.callback (node_modules/newrelic/lib/instrumentation/superagent.js:55:21)
at fn (node_modules/superagent/src/node/index.js:1166:18)
at IncomingMessage.<anonymous> (node_modules/superagent/src/node/parsers/json.js:19:7)
at IncomingMessage.emit (node:events:530:35)
at IncomingMessage.emit (node:domain:489:12)
at endReadableNT (node:internal/streams/readable:1698:12)
at processTicksAndRejections (node:internal/process/task_queues:90:21)
9861ce2 to
b1d83b5
Compare
| .string() | ||
| .valid(REQUEST_STATE.APPROVED, REQUEST_STATE.PENDING, REQUEST_STATE.REJECTED) | ||
| .optional(), | ||
| dev: joi.boolean().optional(), |
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 adding dev in schema again, if we want to remove this feature from dev flag.
Date: 28 Dec 2025
Developer Name: Dhirender Choudhary
Issue Ticket Number
#2525
Tech Doc Link
https://docs.google.com/document/d/11ydUBODXS1FB_ebxgRxOD6-1wL3lTTBc/edit
Business Doc Link
https://docs.google.com/document/d/1q_AabPkKPpXMbSlp5rR4it5sJBQvI1kI/edit
Description
Removes the
dev=truefeature flag requirement from the Out-Of-Office (OOO) request creation endpoint (POST /requests), enabling the feature for production use.Changes Made
req.query.dev === "true"check fromcontrollers/oooRequests.ts.test/integration/requests.test.tsto use the production endpoint/requestsinstead of/requests?dev=true.501 Feature not implementedresponse.Documentation Updated?
Under Feature Flag
Feature flag has been completely removed.
Database Changes
Breaking Changes
Backward compatible — existing clients sending
?dev=truewill continue to work, as the parameter is now ignored.Development Tested?
Verified locally using integration tests.
Screenshots
Screenshot 1
Screen.Recording.2025-12-29.at.8.mp4
Test Coverage
Integration Tests
Ran yarn test-integration test/integration/requests.test.ts All OOO request tests passed without dev flScreen.Recording.2025-12-30.at.1.03.11.AM.mov
Additional Notes
Authorization logic, Discord role checks, and user status validation remain unchanged and fully active.