OTP schedule timezone fix #9676
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces changes across multiple files related to date handling and API endpoint definitions. In the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
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 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![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
🧹 Nitpick comments (2)
src/pages/Patient/index.tsx (1)
78-88: Consider making timezone adjustments more configurable.While this workaround subtracts 5 hours and 30 minutes using dayjs, it may be safer and more maintainable to draw the offset from a configuration or environment variable. That way, time calculations are more explicit, remain consistent across the codebase, and can be easily updated or removed once the backend handles timezones natively.
If you’d like, I can help propose a patch that extracts the offset logic into a single source of truth so it’s easier to modify in the future.
src/pages/Appoinments/Schedule.tsx (1)
252-256: Keep the temporary timezone offset uncluttered.The subtraction of 5 hours and 30 minutes is explicitly described as a temporary fix. Like in the
PatientIndexcomponent, storing this offset in a centralized config or environment variable can avert confusion and reduce rework once the backend handles timezones.Would you like me to propose a consolidated approach for handling these offsets so it’s easier to remove once the backend is fixed?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/Appoinments/Schedule.tsx(2 hunks)src/pages/Patient/index.tsx(1 hunks)src/types/organization/organizationApi.ts(1 hunks)
🔇 Additional comments (2)
src/types/organization/organizationApi.ts (1)
55-59: Looks consistent; ensure usage and testing.The new
getPublicOrganizationendpoint neatly mirrors the existing API structure. Just confirm that it's properly integrated and tested, particularly for edge cases and IDs that may not exist. Consider verifying if authentication is needed for this public endpoint and if additional security checks are required.src/pages/Appoinments/Schedule.tsx (1)
2-2: Upgrade from date-fns to dayjs approved.Importing dayjs aligns with your overall shift away from date-fns and helps maintain date consistency across the project.
Co-authored-by: Rithvik Nishad <rithvikn2001@gmail.com>
Co-authored-by: Rithvik Nishad <rithvikn2001@gmail.com>
|
@Jacobjeevan 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
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates
date-fnstodayjsfor time-related operations