Fix : Today's date allowed while creation OOO #2529
Conversation
This reverts commit 6685c0c.
WalkthroughModified the minimum validation for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
middlewares/validators/oooRequests.ts (1)
6-40: Critical: Middleware doesn't callnext()on successful validation.Unlike
acknowledgeOooRequestValidator(line 89), this validator doesn't callnext()when validation succeeds. This breaks the Express middleware chain and prevents the request from reaching the route handler. The test change on line 34 of the test file confirms this bug.🔎 Proposed fix
export const createOooStatusRequestValidator = async ( req: OooRequestCreateRequest, res: OooRequestResponse, next: NextFunction ) => { const schema = joi .object() .strict() .keys({ from: joi .number() .min(new Date().setHours(0, 0, 0, 0)) .messages({ "number.min": "from date must be greater than or equal to Today's date", }) .required(), until: joi .number() .min(joi.ref("from")) .messages({ "number.min": "until date must be greater than or equal to from date", }) .required(), reason: joi.string().required().messages({ "any.required": "reason is required", "string.empty": "reason cannot be empty", }), type: joi.string().valid(REQUEST_TYPE.OOO).required().messages({ "string.empty": "type cannot be empty", "any.required": "type is required", }), }); - await schema.validateAsync(req.body, { abortEarly: false }); + try { + await schema.validateAsync(req.body, { abortEarly: false }); + return next(); + } catch (error: unknown) { + if (error instanceof joi.ValidationError) { + const errorMessages = error.details.map((detail) => detail.message); + logger.error(`Error validating OOO request: ${errorMessages}`); + return res.boom.badRequest(errorMessages); + } + const errorMessage = error instanceof Error ? error.message : String(error); + logger.error(`Error validating OOO request: ${errorMessage}`); + return res.boom.badRequest(["Error validating OOO request"]); + } };test/unit/middlewares/oooRequests.test.ts (3)
27-35: Test now validates incorrect middleware behavior.This test change reflects the bug identified in the validator: it now asserts that
next()is NOT called on successful validation, which is incorrect middleware behavior. After fixing the validator to properly callnext(), this assertion should be reverted.🔎 Proposed fix (after validator is fixed)
it("should validate for a valid create request", async function () { req = { body: validOooStatusRequests, }; res = {}; await createOooStatusRequestValidator(req as any, res as any, nextSpy); - expect(nextSpy.notCalled).to.be.true; + expect(nextSpy.calledOnce).to.be.true; });
83-93: Timezone inconsistency between test and validator.The test uses
setUTCHours(0, 0, 0, 0)(UTC timezone) while the validator usessetHours(0, 0, 0, 0)(local timezone). This mismatch could cause the test to pass locally but fail in CI/CD environments with different timezone configurations, or vice versa.🔎 Proposed fix
Align with the validator by using local time:
it("should not validate for an invalid request if all until date is greater than from", async function () { req = { - body: { ...validOooStatusRequests, from: new Date().setUTCHours(0, 0, 0, 0) + 5000000, until: new Date().setUTCHours(0, 0, 0, 0) + 1000000 }, + body: { ...validOooStatusRequests, from: new Date().setHours(0, 0, 0, 0) + 5000000, until: new Date().setHours(0, 0, 0, 0) + 1000000 }, };Or better, use UTC consistently in both places for predictable behavior.
9-9: Update test fixture for consistency.The
validOooStatusRequestsfixture (imported fromtest/fixtures/oooRequest/oooRequest.ts) still usesDate.now() + 1 * 24 * 60 * 60 * 1000(tomorrow), which works but is inconsistent with the new validation logic that allows today's date. Consider updating the fixture to use the start-of-day pattern for clarity and consistency.#!/bin/bash # Description: Check all usages of validOooStatusRequests to assess impact of updating the fixture # Find all files using validOooStatusRequests rg -n "validOooStatusRequests" --type=ts -g '!node_modules' -C3
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
middlewares/validators/oooRequests.tstest/integration/requests.test.tstest/unit/middlewares/oooRequests.test.ts
💤 Files with no reviewable changes (1)
- test/integration/requests.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/unit/middlewares/oooRequests.test.ts (1)
test/fixtures/oooRequest/oooRequest.ts (1)
validOooStatusRequests(15-20)
⏰ 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: build (22.10.0)
2 participants
Date: 3 Jan 2026
Developer Name: Dhirender Choudhary
Issue Ticket Number
RealDevSquad/website-www#1125
Tech Doc Link
https://docs.google.com/document/d/1qqa3eBRCceZpiI7tigfy5SpYO2TzkxL6pFwMk8BF7-g/edit?tab=t.0
Description
This PR fixes a bug where creating an OOO request for "Today's Date" resulted in a 400 Bad Request error.
The Problem: The backend validation from >= Date.now() was comparing the selected date (which defaults to midnight 00:00) against the current execution time. This caused valid same-day requests to fail because midnight is technically in the past relative to the current time.
The Fix: I updated the validation logic to allow the current day to be a valid start date. Users can now successfully raise OOO requests starting from today without triggering the validation error.
Documentation Updated?
Under Feature Flag
Feature flag has been completely removed.
Database Changes
Breaking Changes
Development Tested?
Verified locally using integration tests.
Test Coverage
Details
Screen.Recording.2026-01-03.at.4.23.35.PM.mp4
Additional Notes
Authorization logic, Discord role checks, and user status validation remain unchanged and fully active.