Skip to content

Conversation

@Dhirenderchoudhary
Copy link
Contributor

@Dhirenderchoudhary Dhirenderchoudhary commented Dec 31, 2025

Date: 1-01-2026

Developer Name: Dhirender


Issue Ticket Number:-

#1109

Description:

  • This PR fixes a bug caused by an incorrect date format sent in the request payload.

The from and until fields were unnecessarily converted to UTC midnight values, which resulted in a mismatch with the backend API contract.
Changes:
• Removed unnecessary UTC date conversion.
• Ensured from and until are sent as numeric timestamps.
• Updated tests to validate correct data types and prevent regressions.

Is Under Feature Flag

  • Yes
  • No

Database changes

  • Yes
  • No

Breaking changes

  • Yes
  • No

Is Development Tested?

  • Yes
  • No

Tested in staging?

  • Yes
  • No

Screenshot below

Screen.Recording.2026-01-01.at.4.20.49.AM.mov

@coderabbitai
Copy link

coderabbitai bot commented Dec 31, 2025

Walkthrough

The change modifies how OOO (Out of Office) status parameters are passed to createOOORequest in the user status modal, shifting from UTC midnight timestamps to raw date values, with corresponding test assertions updated to expect date strings instead of numeric timestamps.

Changes

Cohort / File(s) Summary
OOO Request Parameter Type Update
app/components/user-status-modal.js, tests/integration/components/user-status-modal-test.js
Changed createOOORequest invocation to pass raw fromDate and untilDate values instead of UTC midnight timestamps; bypasses prior timestamp conversion logic. Test assertions updated to expect from/until as date strings rather than numeric timestamps.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • Hariom01010
  • iamitprakash

Poem

🐰 Timestamps were once our midnight friend,
But now raw dates shall reign supreme!
From numeric values, strings ascend,
Making dates flow like a cleaner dream.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: OOO migration date issues' directly relates to the changeset which modifies how dates are passed to the OOO request creation, addressing the date parameter conversion issue in the migration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description is related to the changeset, describing a bug fix for incorrect date format in the OOO feature.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 (3)
app/components/user-status-modal.js (3)

49-57: Remove dead code: newStateObj is unused.

The newStateObj construction (lines 49-56) performs timestamp conversions and builds an object that is never used in updateStatusBasedOnMode. This creates unnecessary overhead and confusion.

🔎 Proposed refactor to remove dead code
  @action
  async getCurrentStatusObj() {
    if (!this.isValidStatusChange()) return;

-    const updatedAt = Date.now();
-    const newStateObj = {
-      updatedAt,
-      from: this.getFromTimestamp(),
-      until: this.getUntilTimestamp(),
-      message: this.reason,
-      state: this.args.newStatus,
-    };
-    await this.updateStatusBasedOnMode(newStateObj);
+    await this.updateStatusBasedOnMode();
    this.resetInputFields();
    this.disableSubmitButton = true;
  }

85-100: Remove unused timestamp conversion from status object.

The from and until fields computed in getCurrentStatusObj (lines 53-54) are never used. The updateStatusBasedOnMode method (line 107) doesn't accept parameters and instead uses this.fromDate and this.untilDate directly, making the getFromTimestamp() and getUntilTimestamp() calls dead code.

Additionally, updateStatusBasedOnMode only handles the OOO status and has no logic for IDLE, even though isValidStatusChange() validates IDLE status input.

Remove the unused from and until fields from the newStateObj or refactor updateStatusBasedOnMode to accept and use the status object parameter consistently. If IDLE status no longer needs timestamp handling, remove those branches from both getFromTimestamp() and getUntilTimestamp().


107-115: Critical: Unused parameter and dead code.

This method has two issues:

  1. Parameter mismatch: The method is defined without parameters (line 107) but called with newStateObj at line 57. The passed parameter is silently ignored.

  2. Dead code: The code builds newStateObj with timestamp values from getFromTimestamp() and getUntilTimestamp() (lines 50-56), but then never uses them. Instead, it passes the raw date strings from this.fromDate and this.untilDate directly to createOOORequest, which is the correct approach since the API expects date strings (the conversion to timestamps happens in the controller).

Remove the unused parameter and dead code:

Proposed fix

At line 57 in getCurrentStatusObj:

-    await this.updateStatusBasedOnMode(newStateObj);
+    await this.updateStatusBasedOnMode();

Remove the unused timestamp methods or refactor if needed elsewhere. Delete or simplify lines 50-56 in getCurrentStatusObj since only this.reason is ultimately needed.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9108de8 and ce992e1.

📒 Files selected for processing (2)
  • app/components/user-status-modal.js
  • tests/integration/components/user-status-modal-test.js
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integration/components/user-status-modal-test.js (1)
tests/unit/controllers/status-test.js (2)
  • until (135-137)
  • until (180-182)
⏰ 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 (18.x)
🔇 Additional comments (1)
tests/integration/components/user-status-modal-test.js (1)

64-70: Test assertions correctly updated to expect date strings.

The type assertions now correctly expect from and until to be strings instead of numeric timestamps, aligning with the production change. The YYYY-MM-DD date format used in the test is consistent with the backend utility getUTCMidnightTimestampFromDate, which explicitly parses dates in this format before converting them to UTC midnight timestamps.

@MayankBansal12 MayankBansal12 mentioned this pull request Dec 31, 2025
10 tasks
@iamitprakash iamitprakash merged commit c013f54 into RealDevSquad:develop Jan 1, 2026
3 checks passed
iamitprakash added a commit that referenced this pull request Jan 1, 2026
* Feature flag removed : user OOO approve reject (#1110)

* code changes OOO flow Migrated from users page to request flow

* removed console log

* changed assert.expect

* fix: pass date strings instead of timestamps to createOOORequest

* fixed errors

* wrong commit fix

* removed else

* removed stateobj

---------

Co-authored-by: Dhirenderchoudhary <dhirenderchoudhary0001@gmailcom>

* fix: OOO migration date issues (#1112)

* fixed date issues

* fix lint

---------

Co-authored-by: Dhirenderchoudhary <dhirenderchoudhary0001@gmailcom>

* fix: Update OOO request button text (#1116)

* message changed

* message fix

* lint fix

---------

Co-authored-by: Dhirenderchoudhary <dhirenderchoudhary0001@gmailcom>

---------

Co-authored-by: Dhirender Choudhary <[email protected]>
Co-authored-by: Dhirenderchoudhary <dhirenderchoudhary0001@gmailcom>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants