Fix same-day discharge date selection in DateInputV2 component #9014
Conversation
|
Warning Rate limit exceeded@github-actions[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 45 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes in Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
❌ Deploy Preview for care-ohc failed.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/Common/DateInputV2.tsx (2)
193-202: Consider using dayjs for more robust date comparison.The current implementation uses granular date component comparisons. Consider using dayjs for more maintainable date equality checks:
- if ( - min && - max && - min.getDate() === max.getDate() && - day === min.getDate() && - month === min.getMonth() && - year === min.getFullYear() - ) { - return true; - } + if (min && max && dayjs(min).isSame(max, 'day') && dayjs(new Date(year, month, day)).isSame(min, 'day')) { + return true; + }This approach:
- Reduces code complexity
- Handles edge cases more reliably
- Aligns with the existing dayjs usage in the component
193-202: Consider documenting the same-day date handling behavior.The special case handling for same-day dates is a significant behavior that should be documented for future maintainers and users of the component.
Consider:
- Adding a JSDoc comment to the
isDateWithinConstraintsfunction explaining the same-day date handling- Updating the component's props documentation to clarify that same-day selections are allowed when min and max dates are equal
- Adding test cases to verify this behavior
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Common/DateInputV2.tsx(1 hunks)
🔇 Additional comments (1)
src/components/Common/DateInputV2.tsx (1)
193-202: LGTM! Verify the time component handling.
The new condition correctly enables same-day date selection, addressing the core issue.
Let's verify the time component handling:
✅ Verification successful
Time component is properly handled - no issues found
The implementation correctly handles the time component:
- When
allowTimeis false,getDayStart()is used which sets hours, minutes, seconds, and milliseconds to 0 - When
allowTimeis true, the full date object with time is preserved - The same-day validation in
isDateWithinConstraints()only checks date components (day, month, year) which is correct since time constraints are handled separately through thegetDayStart()function
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for time-related validations in the component
rg -A 5 "allowTime.*isDateWithinConstraints|isDateWithinConstraints.*allowTime" .
Length of output: 83
Script:
#!/bin/bash
# Let's first check the full implementation of isDateWithinConstraints
ast-grep --pattern 'isDateWithinConstraints($$$) {
$$$
}'
# Also check for any time-related props or state
rg "allowTime|timeFormat|time" src/components/Common/DateInputV2.tsx
Length of output: 728
Script:
#!/bin/bash
# Let's check how getDayStart is implemented since it's used with allowTime
ast-grep --pattern 'getDayStart($$$) {
$$$
}'
# Also check for any time-related utility functions
rg -A 5 "function.*Date|const.*Date" src/components/Common/DateInputV2.tsx
Length of output: 1939
|
LGTM @rithviknishad can you review the code |
|
@JavidSumra Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
DateInputV2.tsxcomponent to allow selection of same-day discharge date.isDateWithinConstraintsfunction checks if the selected date is the same as the admission date and falls within the allowed constraints.@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit