User Availabilities Exception related fixes #9977
Conversation
…te; wire add exception button; fix same day exception validation;
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
rithviknishad
changed the title
WalkthroughThis pull request introduces enhancements to the scheduling and exception handling functionality across multiple components. The changes focus on improving internationalization, query parameter management, and slot computation. Key modifications include updating localization files, refactoring components to use query parameters for state management, and implementing a new function to compute appointment slots while considering exceptions. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
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 (
|
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
rithviknishad/fix/9959-issues-in-exceptions
|
| Run status |
|
| Run duration | 01m 45s |
| Commit |
|
| Committer | Rithvik Nishad |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
4
|
| View all changes introduced in this branch ↗︎ | |
Deploying care-fe with
|
| Latest commit: |
6cffa9c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e25b7435.care-fe.pages.dev |
| Branch Preview URL: | https://rithviknishad-fix-9959-issue.care-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/pages/Scheduling/components/CreateScheduleExceptionSheet.tsx (1)
109-119: Consider combining the useEffect hooks.The two useEffect hooks for form value updates can be combined to reduce the number of re-renders.
Apply this diff to combine the hooks:
- useEffect(() => { - if (qParams.valid_from) { - form.setValue("valid_from", new Date(qParams.valid_from)); - } - }, [qParams.valid_from, form]); - - useEffect(() => { - if (qParams.valid_to) { - form.setValue("valid_to", new Date(qParams.valid_to)); - } - }, [qParams.valid_to, form]); + useEffect(() => { + if (qParams.valid_from) { + form.setValue("valid_from", new Date(qParams.valid_from)); + } + if (qParams.valid_to) { + form.setValue("valid_to", new Date(qParams.valid_to)); + } + }, [qParams.valid_from, qParams.valid_to, form]);src/components/Users/UserAvailabilityTab.tsx (1)
51-53: Consider using qParams.tab directly.Instead of creating a new variable
view, consider usingqParams.tabdirectly to reduce potential synchronization issues.Apply this diff to remove the redundant variable:
- const view = qParams.tab || "schedule"; + const currentTab = qParams.tab ?? "schedule";Then update all occurrences of
viewtocurrentTab.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
public/locale/en.json(2 hunks)src/components/Users/UserAvailabilityTab.tsx(6 hunks)src/pages/Scheduling/components/CreateScheduleExceptionSheet.tsx(6 hunks)src/pages/Scheduling/components/CreateScheduleTemplateSheet.tsx(6 hunks)src/pages/Scheduling/utils.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (15)
src/pages/Scheduling/utils.ts (1)
45-48: LGTM!The type definition is clear and correctly structured with the required fields.
src/pages/Scheduling/components/CreateScheduleExceptionSheet.tsx (4)
45-49: LGTM!The QueryParams type is well-defined with appropriate optional parameters.
59-61: LGTM!Good use of
replace: falseto merge with existing query params, and the comment clearly explains the reasoning.
92-95: LGTM!The date validation is more intuitive using
isAfter, and the error is correctly mapped to thevalid_fromfield.
159-167: LGTM!The sheet state is correctly managed through URL parameters, with proper cleanup on close.
src/components/Users/UserAvailabilityTab.tsx (3)
44-47: LGTM!The AvailabilityTabQueryParams type is well-defined with appropriate optional parameters.
162-174: LGTM!The "Add Exception" button correctly sets query parameters and uses dateQueryString for consistent date formatting.
261-284: LGTM!The exception display is well-implemented with clear visual indication using diagonal stripes and proper time formatting.
src/pages/Scheduling/components/CreateScheduleTemplateSheet.tsx (4)
62-64: LGTM!The QueryParams type is well-defined with appropriate optional parameters.
74-76: LGTM!Good use of
replace: falseto merge with existing query params, and the comment clearly explains the reasoning.
147-150: LGTM!The date validation is more intuitive using
isAfter, and the error is correctly mapped to thevalid_fromfield.
246-251: LGTM!The sheet state is correctly managed through URL parameters, with proper cleanup on close.
public/locale/en.json (3)
982-982: LGTM! Clear and consistent translation key.The translation follows the established naming and capitalization patterns in the file.
1882-1882: LGTM! Clear slot information format.The translation uses proper interpolation syntax and follows consistent terminology.
1883-1883: Consider HTML sanitization for the strike-through tag.While the translation is well-formatted and consistent, ensure that the HTML
<s>tag is properly sanitized when rendered to prevent XSS vulnerabilities.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Users/UserAvailabilityTab.tsx (2)
357-357: Address the TODO comment for handling multiple days of the week.The comment indicates that multiple days of the week handling is pending implementation.
Would you like me to help implement the multiple days of week handling or create a GitHub issue to track this task?
336-436: Consider extracting appointment-specific logic into a separate component.The component handles both regular and appointment slot types. Consider extracting the appointment-specific logic (lines 365-432) into a separate
AppointmentAvailabilityDetailscomponent for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Users/UserAvailabilityTab.tsx(6 hunks)src/pages/Scheduling/utils.ts(2 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/Users/UserAvailabilityTab.tsx (1)
Learnt from: rithviknishad
PR: ohcnetwork/care_fe#9977
File: src/components/Users/UserAvailabilityTab.tsx:193-251
Timestamp: 2025-01-15T08:13:18.414Z
Learning: The `computeAppointmentSlots` function in scheduling utils is efficient enough for real-world scenarios without memoization, as it has linear complexity based on the number of slots in a day, not the number of exceptions.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (7)
src/pages/Scheduling/utils.ts (4)
1-14: LGTM!The imports are well-organized and all imported functions are utilized effectively in the implementation.
45-50: LGTM!The
VirtualSlottype is well-designed, providing a clear structure for appointment slots with proper tracking of availability and associated exceptions.
126-126: LGTM!Good addition of
Math.floorto ensure slot counts are always whole numbers.
52-117: 🛠️ Refactor suggestionAdd input validation to prevent runtime errors.
The function should validate its inputs before processing:
- Check if
availability.availability[0]exists- Validate
slot_size_in_minutesis positiveApply this diff to add the necessary validations:
export function computeAppointmentSlots( availability: ScheduleAvailability & { slot_type: "appointment" }, exceptions: ScheduleException[], referenceDate: Date = new Date(), ) { + if (!availability.availability[0]) { + throw new Error("Availability must have at least one time slot"); + } + + if (!availability.slot_size_in_minutes || availability.slot_size_in_minutes <= 0) { + throw new Error("Invalid slot size"); + } + const startTime = parse( availability.availability[0].start_time, "HH:mm:ss", referenceDate, );⛔ Skipped due to learnings
Learnt from: rithviknishad PR: ohcnetwork/care_fe#9977 File: src/components/Users/UserAvailabilityTab.tsx:193-251 Timestamp: 2025-01-15T08:13:18.414Z Learning: The `computeAppointmentSlots` function in scheduling utils is efficient enough for real-world scenarios without memoization, as it has linear complexity based on the number of slots in a day, not the number of exceptions.src/components/Users/UserAvailabilityTab.tsx (3)
Line range hint
1-60: LGTM!The imports are well-organized and the query parameter types are properly defined with clear type constraints.
64-66: LGTM!Good implementation of internationalization and consistent naming with the tab parameter.
246-334: LGTM!The
DayDetailsPopovercomponent is well-implemented with proper separation of concerns and good handling of overflow content.
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
Localization
New Features
Improvements