-
Notifications
You must be signed in to change notification settings - Fork 524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Scheduling and Appointments; support for delete schedule, cancel booking, improved date filter in appointments page, etc. #9799
Conversation
Warning Rate limit exceeded@rithviknishad has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 4 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. 📒 Files selected for processing (3)
WalkthroughThis pull request introduces a comprehensive refactoring of the scheduling and appointment management system across multiple components and modules. The changes involve restructuring type definitions, updating API interactions, enhancing localization, and improving user interface components for scheduling, appointments, and exceptions. Key areas of focus include updating type structures, consolidating API calls, and introducing more flexible scheduling template and exception management. Changes
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
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. |
CARE Run #4240
Run Properties:
|
Project |
CARE
|
Branch Review |
rithviknishad/feat/schedule-validations
|
Run status |
Failed #4240
|
Run duration | 02m 13s |
Commit |
2a12e60b7b: Refactor Scheduling and Appointments; support for delete schedule, cancel bookin...
|
Committer | Rithvik Nishad |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
1
|
Flaky |
0
|
Pending |
0
|
Skipped |
1
|
Passing |
4
|
View all changes introduced in this branch ↗︎ |
Tests for review
cypress/e2e/facility_spec/facility_creation.cy.ts • 1 failed test
Test | Artifacts | |
---|---|---|
Facility Management > Create a new facility using the admin role |
Test Replay
Screenshots
|
Deploying care-fe with Cloudflare Pages
|
👋 Hi, @rithviknishad, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
👋 Hi, @rithviknishad, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
…ist of shedulable users and no practitioner was selected on first load.
👋 Hi, @rithviknishad, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this 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
🧹 Nitpick comments (1)
public/locale/en.json (1)
195-195
: Fix typo in closed session description"Inidcates" should be "Indicates".
- "SCHEDULE_AVAILABILITY_TYPE_DESCRIPTION__closed": "Inidcates the practitioner is not available for in this session's time", + "SCHEDULE_AVAILABILITY_TYPE_DESCRIPTION__closed": "Indicates the practitioner is not available for in this session's time"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json
(27 hunks)
🔇 Additional comments (3)
public/locale/en.json (3)
319-319
: LGTM! Well-structured template and session keysThe schedule template and session related keys are clear, consistent, and provide good user guidance.
Also applies to: 658-659, 901-901
1604-1604
: LGTM! Clear and consistent action keysThe schedule status and action keys are well-structured with clear, concise messages.
Also applies to: 1193-1195
1665-1665
:⚠️ Potential issueEnsure HTML tags are properly escaped
The slot allocation message contains HTML tags. Ensure these are properly escaped when rendered in the UI to prevent XSS attacks.
✅ Verification successful
HTML content is properly handled using react-i18next
The translation key is used with react-i18next's
Trans
component, which safely handles HTML content by converting the tags to React elements. This is the recommended approach and is secure against XSS attacks.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components that render this translation key to verify HTML escaping rg -l "schedule_slots_allocation_callout" --type ts --type tsxLength of output: 90
Script:
#!/bin/bash # Search for the translation key in js/jsx/ts files rg "schedule_slots_allocation_callout" -t js -t jsx -t ts # Also search for potential i18n/translation usage patterns rg -A 3 -B 3 "useTranslation|t\([\'\"]schedule_slots_allocation_callout" -t js -t jsx -t tsLength of output: 323
Script:
#!/bin/bash # Simple search without file type filters rg "schedule_slots_allocation_callout" # Search for common translation patterns rg "useTranslation"Length of output: 20870
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json
(27 hunks)src/Utils/request/errorHandler.ts
(2 hunks)src/components/Users/UserHome.tsx
(1 hunks)src/pages/Appoinments/PatientRegistration.tsx
(3 hunks)src/pages/Appoinments/PatientSelect.tsx
(3 hunks)src/pages/Appoinments/Schedule.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/Users/UserHome.tsx
- src/pages/Appoinments/PatientRegistration.tsx
- src/pages/Appoinments/PatientSelect.tsx
- src/Utils/request/errorHandler.ts
- src/pages/Appoinments/Schedule.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (4)
public/locale/en.json (4)
664-665
: LGTM!The schedule template management keys follow consistent naming conventions and have clear, descriptive values.
Also applies to: 787-787
1685-1699
: LGTM!The session configuration keys are well-structured with:
- Clear, descriptive values
- Consistent naming conventions
- Proper HTML formatting in the callout message
908-908
: LGTM!The time-related keys follow consistent naming conventions and have clear values.
Also applies to: 1816-1816
1689-1691
: LGTM!The slot management keys are well-structured with:
- Clear, descriptive values
- Consistent naming conventions
- Proper HTML formatting in the callout message
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this 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
🧹 Nitpick comments (2)
public/locale/en.json (2)
194-196
: Review availability type descriptions for clarity and consistency.The descriptions are clear and consistent, but there's a minor improvement opportunity in wording.
- "SCHEDULE_AVAILABILITY_TYPE_DESCRIPTION__appointment": "Patients can be booked for slots in this session", + "SCHEDULE_AVAILABILITY_TYPE_DESCRIPTION__appointment": "Patients can book appointments in this session",
1685-1691
: Enhance session configuration keys for better UX.The session configuration keys are functional but could be more user-friendly.
- "schedule_slot_size": "Slot Size", + "schedule_slot_size": "Appointment Duration", - "schedule_slot_size_label": "Slot size (mins.)", + "schedule_slot_size_label": "Duration per appointment (mins.)",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json
(27 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
- GitHub Check: OSSAR-Scan
🔇 Additional comments (4)
public/locale/en.json (4)
197-199
: LGTM! Availability type labels are concise and clear.The labels effectively represent the three states of availability.
664-665
: LGTM! Template creation keys are clear and concise.The keys follow proper naming conventions and are self-explanatory.
1693-1699
: LGTM! Template configuration keys are well structured.The keys provide clear guidance for template setup with appropriate placeholders and descriptions.
908-908
: Ensure consistency in time-related labels.The time-related keys are scattered but maintain consistency in terminology.
Also applies to: 1680-1681, 1816-1816
There was a problem hiding this 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
🧹 Nitpick comments (3)
public/locale/en.json (3)
1685-1690
: Consider adding validation hints in session-related messages.While the session-related messages are clear, consider adding validation hints (e.g., "Session time must be within facility working hours") to improve user experience.
+"SCHEDULE_SESSION_TIME_HINT": "Session time must be within facility working hours", +"SCHEDULE_SESSION_TYPE_HINT": "Select the type of session (appointment, open, or closed)",
908-908
: Ensure consistent time-related terminology.The time-related keys use both "Start Time" and "End Time" consistently. However, consider adding time format hints to improve user input accuracy.
+"schedule_time_format_hint": "Use 24-hour format (HH:mm)",
Also applies to: 1680-1681, 1691-1691, 1815-1815
664-665
: Consider adding error messages for template operations.While success messages exist, consider adding specific error messages for template operations to improve error handling.
+"create_schedule_template_error": "Failed to create schedule template", +"edit_schedule_template_error": "Failed to edit schedule template",Also applies to: 1843-1843
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json
(27 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (5)
public/locale/en.json (5)
194-199
: LGTM! Clear and consistent schedule availability type descriptions.The schedule availability type keys and descriptions are well-defined and follow a consistent pattern:
- "appointment": For bookable slots
- "closed": For unavailable time
- "open": For available time
1692-1698
: LGTM! Comprehensive template configuration messages.The schedule template configuration messages are well-structured and include helpful descriptions for weekday selection.
1683-1684
: LGTM! Clear remarks field labels and placeholders.The remarks field labels and placeholders provide clear guidance for users.
1463-1463
: LGTM! Clear slot allocation and capacity messages.The slot allocation message effectively communicates the relationship between total slots and appointment duration using dynamic values.
Also applies to: 1689-1690, 1864-1864
1677-1678
: LGTM! Clear success/failure messages for schedule operations.The success messages for schedule creation and deletion are clear and consistent.
@@ -69,10 +69,10 @@ function isNotFound(error: HTTPError) { | |||
|
|||
type PydanticError = { | |||
type: string; | |||
loc: string[]; | |||
loc?: string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's create different error types for the varoetu of errors the backend throws.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will rewrite this piece from scratch post #9708 and this PR is merged.
Plan: discriminated union based on the type
of error. Maybe use a switch case to match the type
and accordingly handle the toast message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And to also create a util fn. that catches field errors and sets it to the react-hook-form form's error state and if any non field errors encountered, rethrow it for global error handler to do its thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rithviknishad Is there an issue for it? Otherwise can you file? Just don't wanna miss out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, already filed: #9882
Reminder: To add the "needs testing" label, comment "ready for testing" on this PR. |
@@ -45,6 +40,11 @@ import { | |||
AppointmentPatientRegister, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spellcheck for the Appointments Folder
👋 Hi, @rithviknishad, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
src/pages/Scheduling/components/CreateScheduleExceptionSheet.tsx (1)
Line range hint
264-275
: Internationalize all labels and placeholdersSome
FormLabel
components andInput
placeholders use hardcoded English strings. To ensure proper localization and internationalization, wrap these strings with thet
function fromreact-i18next
.Apply this diff to wrap labels and placeholders:
- <FormLabel required>Reason</FormLabel> + <FormLabel required>{t("reason")}</FormLabel> - <Input - placeholder="e.g. Holiday Leave, Conference, etc." - {...field} - /> + <Input + placeholder={t("reason_placeholder")} + {...field} + /> - <FormLabel required>Valid From</FormLabel> + <FormLabel required>{t("valid_from")}</FormLabel> - <FormLabel required>Valid Till</FormLabel> + <FormLabel required>{t("valid_to")}</FormLabel> - <FormLabel>Full Day Unavailable</FormLabel> + <FormLabel>{t("full_day_unavailable")}</FormLabel> - <FormLabel required>From</FormLabel> + <FormLabel required>{t("from")}</FormLabel> - <FormLabel required>To</FormLabel> + <FormLabel required>{t("to")}</FormLabel>Also applies to: 284-291, 299-306, 323-333, 372-385
src/pages/Appointments/AppointmentsPage.tsx (1)
Line range hint
1114-1139
: Add uniquekey
props to fragments in list renderingWhen rendering a list of elements, each child should have a unique
key
prop to help React identify which items have changed. In theslotsByAvailability.map
function, you are using a fragment<>
without akey
. This can lead to rendering issues.Apply this diff to add a
key
prop to the fragment:- {slotsByAvailability.map(({ availability, slots }) => ( - <> + {slotsByAvailability.map(({ availability, slots }) => ( + <React.Fragment key={availability.name}> <CommandSeparator /> <CommandGroup - key={availability.name} heading={availability.name} > {slots.map((slot) => ( <CommandItem key={slot.id} value={formatSlotTimeRange(slot)} onSelect={() => onSelect(slot.id)} className="cursor-pointer" > <span>{formatSlotTimeRange(slot)}</span> <span className="text-xs text-gray-500 font-medium"> {slot.allocated} / {availability.tokens_per_slot} </span> {selectedSlot?.id === slot.id && ( <CheckIcon className="ml-auto" /> )} </CommandItem> ))} </CommandGroup> - </> + </React.Fragment> ))}
🧹 Nitpick comments (14)
src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx (2)
55-62
: Consider improving type safety by avoidingunknown
casting.The current type casting through
unknown
could be made more type-safe.Consider this alternative approach:
- value: [appointment] as unknown as ResponseValue["value"], + value: [appointment] satisfies ResponseValue["value"],This ensures type checking at compile time rather than bypassing it with type assertions.
Line range hint
40-196
: Consider architectural improvements for better maintainability.While the component is well-structured, consider these improvements:
- Extract slot availability check logic into a custom hook to avoid duplication
- Add error handling for the
resourcesQuery
andslotsQuery
Example structure for the custom hook:
function useSlotAvailability(slot: Slot, availability: Availability) { return { isFullyBooked: slot.allocated >= availability.tokens_per_slot, remainingSlots: availability.tokens_per_slot - slot.allocated }; }Example error handling:
{resourcesQuery.error && ( <div className="text-destructive">{t("error_loading_practitioners")}</div> )}src/CAREUI/interactive/WeekdayCheckbox.tsx (3)
16-24
: Consider deriving day keys from the enum.Instead of maintaining a separate array that mirrors the enum, consider deriving the keys directly from the enum to avoid duplication and potential maintenance issues.
-const dayOfWeekKeys = [ - "MONDAY", - "TUESDAY", - "WEDNESDAY", - "THURSDAY", - "FRIDAY", - "SATURDAY", - "SUNDAY", -] as const; +const dayOfWeekKeys = Object.keys(DayOfWeek).filter((key) => isNaN(Number(key))) as (keyof typeof DayOfWeek)[];
Line range hint
39-47
: Remove redundant null check.Since
onChange
is a required prop, the null check is unnecessary.const handleDayToggle = (day: DayOfWeek) => { - if (!onChange) return; - if (value.includes(day)) { onChange(value.filter((d) => d !== day)); } else { onChange([...value, day]); } };
62-63
: Remove redundant ARIA attribute.Using both
aria-pressed
andaria-checked
is redundant. Since this is a button element,aria-pressed
is more appropriate.aria-pressed={isSelected} - aria-checked={isSelected}
src/pages/Scheduling/components/CreateScheduleTemplateSheet.tsx (2)
170-183
: Handle potential asynchrony inonSuccess
callbackIn the
useMutation
hook forcreateTemplate
, you are invalidating queries in theonSuccess
callback:onSuccess: () => { toast.success("Schedule template created successfully"); setOpen(false); form.reset(); queryClient.invalidateQueries({ queryKey: ["user-schedule-templates", { facilityId, userId }], }); },Ensure that any state updates dependent on the successful invalidation occur after the query has been invalidated. Consider using the
onSettled
option or awaiting theinvalidateQueries
promise if necessary.
191-206
: Simplify the mapping inonSubmit
In the
onSubmit
function, the mapping ofvalues.availabilities
can be simplified for better readability.Consider refactoring as follows:
availabilities: values.availabilities.map( (availability) => ({ name: availability.name, slot_type: availability.slot_type, slot_size_in_minutes: availability.slot_size_in_minutes, tokens_per_slot: availability.tokens_per_slot, reason: availability.reason, availability: values.weekdays.map((day) => ({ day_of_week: day, start_time: availability.start_time, end_time: availability.end_time, })), }) as ScheduleAvailabilityCreateRequest, ),src/pages/Appointments/components/AppointmentTokenCard.tsx (1)
Line range hint
67-68
: Replace fake token number implementation with actual backend integration.The TODO comment indicates that the token number is currently using a temporary implementation.
Would you like me to help create a GitHub issue to track the implementation of proper token number retrieval from the backend?
src/pages/Scheduling/ScheduleExceptions.tsx (1)
Line range hint
124-131
: Consider implementing the cancelled appointments information.The TODO comment indicates that information about cancelled appointments needs to be added.
Would you like me to help create a GitHub issue to track the implementation of showing cancelled appointments information when exceptions are created?
src/pages/Scheduling/ScheduleTemplates.tsx (1)
128-136
: Consider simplifying the slots calculation.The slots calculation could be simplified and made more readable.
Consider this refactoring:
- {Math.floor( - getSlotsPerSession( - slot.availability[0].start_time, - slot.availability[0].end_time, - slot.slot_size_in_minutes, - ) ?? 0, - )}{" "} + {Math.floor( + getSlotsPerSession( + slot.availability[0].start_time, + slot.availability[0].end_time, + slot.slot_size_in_minutes, + ) || 0 + )}{" "}This change:
- Replaces the nullish coalescing operator with a more appropriate OR operator
- Improves readability by removing unnecessary parentheses
src/pages/Appointments/utils.ts (1)
79-79
: Consider caching for availability data.The availability stats query and heatmap data could benefit from caching to improve performance.
Consider implementing React Query's caching capabilities:
return useQuery({ queryKey: ["availabilityHeatmap", userId, fromDate, toDate], queryFn, enabled: !!userId, + staleTime: 5 * 60 * 1000, // 5 minutes + cacheTime: 30 * 60 * 1000, // 30 minutes });Also applies to: 108-108
src/pages/Appointments/AppointmentDetail.tsx (2)
383-395
: Add error handling for the cancel mutation.The
cancelAppointment
mutation is missing error handling. Consider adding anonError
callback to show error messages to users when cancellation fails.const { mutate: cancelAppointment } = useMutation({ mutationFn: mutate(scheduleApis.appointments.cancel, { pathParams: { facility_id: facilityId, id: appointment.id, }, }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["appointment", appointment.id], }); }, + onError: (error) => { + toast.error(error?.message || t("failed_to_cancel_appointment")); + }, });
222-224
: Consider using a type-safe approach for status-to-variant mapping.The variant mapping could be more type-safe using a const assertion.
-} as Partial<Record<Appointment["status"], BadgeProps["variant"]>> +const STATUS_VARIANT_MAP = { + booked: "secondary", + checked_in: "primary", + in_consultation: "primary", + pending: "secondary", + arrived: "primary", + fulfilled: "primary", + entered_in_error: "destructive", + cancelled: "destructive", + noshow: "destructive", +} as const satisfies Record<Appointment["status"], BadgeProps["variant"]>;src/components/Users/UserAvailabilityTab.tsx (1)
198-209
: Consider memoizing the slot calculation.The slot calculation logic could be expensive for large numbers. Consider memoizing it to prevent unnecessary recalculations.
+const calculateSlotsPerSession = useMemo(() => + Math.floor( + getSlotsPerSession( + availability[0].start_time, + availability[0].end_time, + slot_size_in_minutes, + ) ?? 0, + ), + [availability, slot_size_in_minutes] +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
public/locale/en.json
(33 hunks)src/CAREUI/interactive/WeekdayCheckbox.tsx
(2 hunks)src/Routers/AppRouter.tsx
(1 hunks)src/Routers/PatientRouter.tsx
(1 hunks)src/Routers/PublicRouter.tsx
(1 hunks)src/Routers/routes/ScheduleRoutes.tsx
(2 hunks)src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx
(5 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx
(2 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx
(1 hunks)src/components/Schedule/ScheduleTemplatesList.tsx
(0 hunks)src/components/Users/UserAvailabilityTab.tsx
(9 hunks)src/pages/Appointments/AppointmentDetail.tsx
(8 hunks)src/pages/Appointments/AppointmentsPage.tsx
(18 hunks)src/pages/Appointments/BookAppointment.tsx
(4 hunks)src/pages/Appointments/components/AppointmentTokenCard.tsx
(1 hunks)src/pages/Appointments/utils.ts
(3 hunks)src/pages/Patient/index.tsx
(4 hunks)src/pages/PublicAppointments/PatientRegistration.tsx
(3 hunks)src/pages/PublicAppointments/PatientSelect.tsx
(3 hunks)src/pages/PublicAppointments/Schedule.tsx
(5 hunks)src/pages/Scheduling/ScheduleExceptions.tsx
(2 hunks)src/pages/Scheduling/ScheduleTemplates.tsx
(1 hunks)src/pages/Scheduling/components/CreateScheduleExceptionSheet.tsx
(5 hunks)src/pages/Scheduling/components/CreateScheduleTemplateSheet.tsx
(1 hunks)src/pages/Scheduling/components/EditScheduleTemplateSheet.tsx
(1 hunks)src/pages/Scheduling/utils.ts
(3 hunks)
💤 Files with no reviewable changes (1)
- src/components/Schedule/ScheduleTemplatesList.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/Questionnaire/QuestionnaireEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/Patient/index.tsx
- src/components/Questionnaire/QuestionTypes/QuestionInput.tsx
🧰 Additional context used
📓 Learnings (1)
src/CAREUI/interactive/WeekdayCheckbox.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9747
File: src/CAREUI/interactive/WeekdayCheckbox.tsx:49-49
Timestamp: 2025-01-09T04:11:30.690Z
Learning: When using the `cn()` utility for composing class strings in React components, ensure that each item in the array (including ternary expressions) has a trailing comma according to the Prettier configuration.
🔇 Additional comments (26)
src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx (3)
23-23
: LGTM! Clean import updates for the appointment refactoring.The new imports align well with the PR objectives, supporting the transition from follow-up appointments to a more general appointment system.
Also applies to: 29-30
40-40
: LGTM! Component rename and type updates are consistent.The transition from
FollowUpAppointmentQuestion
toAppointmentQuestion
aligns with the PR objectives of generalizing the appointment system.Also applies to: 51-51
72-72
: LGTM! API integration updates are well-aligned.The transition to
scheduleApis
is consistent with the backend changes referenced in the linked PRs (#2711, #2721, #2725).Also applies to: 84-84
src/Routers/PublicRouter.tsx (1)
12-12
: LGTM! Import path update improves organization.The change from
Appoinments
toPublicAppointments
fixes a typo and better reflects the public nature of these components.Let's verify the consistency of the new path across the codebase:
✅ Verification successful
Import path change is consistent with codebase structure
The move to
@/pages/PublicAppointments/auth/PatientLogin
is part of a well-structured separation between public and private appointment components. The codebase consistently uses this new path, and no old paths remain.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of appointment-related imports and ensure no old paths remain # Check if any files still use the old path with typo echo "Checking for any remaining usage of old path with typo..." rg -l "Appoinments/auth/PatientLogin" # Check if the new directory structure exists and is used consistently echo "Verifying the new directory structure..." fd -t d "PublicAppointments" src/pages/ # Check for any mixed usage of old and new paths echo "Checking for any appointment-related imports..." rg -l "import.*from.*[Aa]ppointments/"Length of output: 807
Script:
#!/bin/bash # Check specific imports of PatientLogin component echo "Checking PatientLogin imports..." rg "import.*PatientLogin.*from.*Appointments" -A 1 # Examine the context of appointment-related imports echo -e "\nExamining appointment-related import contexts..." rg "import.*from.*(PublicAppointments|Appointments).*" -A 1Length of output: 2698
src/Routers/AppRouter.tsx (2)
20-20
: LGTM! Import follows project conventions.The import statement for
ScheduleRoutes
is properly placed and follows the project's module aliasing conventions.
Line range hint
47-57
: Verify the imported schedule routes match requirements.The integration of
ScheduleRoutes
looks correct. However, since this PR involves significant changes to scheduling functionality (delete schedule, cancel booking, etc.), we should verify that all required routes are properly defined.Let's check the route definitions:
src/pages/Scheduling/components/CreateScheduleExceptionSheet.tsx (1)
139-139
: Verify the API expectsuser
instead ofuser_id
In the
onSubmit
function, you're passinguser: userId
to thecreateException
mutation:createException({ // ... user: userId, });Please verify whether the backend API expects the key
user
oruser_id
. If the API requiresuser_id
, update the key accordingly to prevent potential issues.✅ Verification successful
The current implementation using
user
is correctThe
ScheduleExceptionCreateRequest
interface insrc/types/scheduling/schedule.ts
confirms that the API expectsuser
as the parameter name:interface ScheduleExceptionCreateRequest { user: string; // user's id // ... }🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for createException definition and usage ast-grep --pattern 'createException' # Search for any type definitions or interfaces related to schedule exceptions rg -g '*.ts' -g '*.tsx' 'interface.*Exception|type.*Exception' -A 5 # Search for API related files that might contain the schema fd -e ts -e tsx api schemaLength of output: 1199
src/pages/Scheduling/components/EditScheduleTemplateSheet.tsx (1)
582-582
: Handle potential null values inslot_size_in_minutes
andtokens_per_slot
In the
onSubmit
function, ensure thatslot_size_in_minutes
andtokens_per_slot
are correctly handled when they might benull
. This prevents potential runtime errors.Consider adding validation or default values to handle
null
cases safely.src/Routers/routes/ScheduleRoutes.tsx (1)
6-8
: Code changes are correctThe import statements and route component updates correctly reflect the refactored components. The routing logic is updated appropriately to use
BookAppointment
,AppointmentsPage
, andAppointmentDetail
components.Also applies to: 27-33
src/pages/Scheduling/utils.ts (1)
4-4
: Updates to types and parameter definitions are appropriateThe changes to import statements and function parameter types align with the updated scheduling types. The functions
getDaysOfWeekFromAvailabilities
andfilterAvailabilitiesByDayOfWeek
now correctly useScheduleAvailability[]
, and the logic within these functions remains valid.Also applies to: 53-63, 65-74
src/pages/Appointments/components/AppointmentTokenCard.tsx (1)
10-12
: LGTM: Import path updates align with the scheduling refactor.The import paths have been updated to reflect the new organization of scheduling-related components, improving code organization.
src/Routers/PatientRouter.tsx (1)
15-18
: LGTM: Import paths updated consistently.The import paths have been updated to reflect the new organization of public appointment components under the
/PublicAppointments
directory.src/pages/Scheduling/ScheduleExceptions.tsx (2)
21-23
: LGTM: Props addition improves component context.The addition of
facilityId
anduserId
props provides better context and aligns with the scheduling refactor.Also applies to: 26-30
68-71
: LGTM: API usage updated consistently.The mutation has been updated to use the new
scheduleApis
and includes proper query invalidation with facility and user context.Also applies to: 77-80
src/pages/Scheduling/ScheduleTemplates.tsx (1)
27-59
: LGTM: Well-structured component with proper error handling.The component implements proper loading and empty states, with clear error boundaries and user feedback.
src/pages/PublicAppointments/PatientSelect.tsx (2)
22-26
: LGTM! Type imports are properly organized.The imports are well-structured and align with the refactoring from
SlotAvailability
toTokenSlot
.
Line range hint
74-93
: Verify error handling in appointment creation.The mutation function looks good, but let's verify the error handling for appointment creation, especially since this is a public-facing component.
Run this script to check for consistent error handling across appointment creation:
✅ Verification successful
Error handling for appointment creation is properly implemented
The code includes comprehensive error handling with:
- Validation checks for staff, phone number, and selected slot
- Clear error messages using the translation system
- Proper error handling for API failures
- Appropriate navigation on errors
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check error handling patterns in appointment creation # Look for similar mutation error handling patterns rg -A 5 "onError.*appointment"Length of output: 32
Script:
#!/bin/bash # Search for error handling patterns in multiple ways echo "=== Searching for onError in mutation patterns ===" rg "onError.*mutation" -A 5 echo -e "\n=== Searching for toast.error usage ===" rg "toast\.error" -A 2 echo -e "\n=== Searching for error-related translations ===" rg "t\([\"'].*error.*[\"']\)" -A 1 echo -e "\n=== Checking for error boundary components ===" fd -e tsx -e ts "ErrorBoundary"Length of output: 23174
src/pages/PublicAppointments/Schedule.tsx (2)
2-2
: LGTM! Consistent date formatting library usage.The switch from dayjs to date-fns is applied consistently, improving library consistency.
Also applies to: 249-249
Line range hint
84-95
: Verify token-based authentication handling.The slots query includes token-based authentication. Let's verify the token handling across the application.
Run this script to check token usage patterns:
✅ Verification successful
Token-based authentication implementation verified and secure.
The application implements a robust token-based authentication system with:
- Centralized token management
- Consistent authentication patterns
- Proper token validation and expiration
- Protected routes and API calls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check token usage in API calls # Look for consistent token handling rg "Authorization.*Bearer.*token"Length of output: 889
Script:
#!/bin/bash # Check token data imports and provider implementation rg "tokenData" -B 2 -A 2 # Check PatientUserProvider implementation ast-grep --pattern 'const PatientUserProvider = $_'Length of output: 17616
src/pages/Appointments/BookAppointment.tsx (2)
33-37
: LGTM! Improved module organization.The imports are well-organized and follow a consistent pattern.
44-44
: Verify API endpoint consistency.The migration from
ScheduleAPIs
toscheduleApis
needs verification across all endpoints.Run this script to check API usage consistency:
Also applies to: 57-57, 78-78, 89-89
✅ Verification successful
API migration verification completed successfully
The codebase shows consistent usage of the new
scheduleApis
format across all files, with no remaining instances of the oldScheduleAPIs
format.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining old API usage # Ensure complete migration to new API structure rg -A 2 "ScheduleAPIs"Length of output: 22
Script:
#!/bin/bash # Search for various API patterns case-insensitively echo "=== Searching for schedule API patterns ===" rg -i "scheduleapis?" -A 2 echo -e "\n=== Checking import statements for APIs ===" rg -i "import.*schedule.*api" -A 1 echo -e "\n=== Looking for API endpoint usage patterns ===" ast-grep --pattern 'const $_ = $$$Api.$_($$$)'Length of output: 11747
src/pages/Appointments/utils.ts (1)
30-33
: Verify type compatibility in slot grouping.The type changes in
groupSlotsByAvailability
look good, but let's verify type compatibility across the application.Run this script to check type usage:
✅ Verification successful
Type changes are compatible across the codebase
The type changes in
groupSlotsByAvailability
maintain compatibility with theTokenSlot
interface and all its usages across components. The availability property structure withname
is preserved and correctly used in all consuming components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent type usage # Look for any remaining SlotAvailability usage ast-grep --pattern 'type $_ = SlotAvailability'Length of output: 49
Script:
#!/bin/bash # Search for TokenSlot type definition and usage ast-grep --pattern 'type TokenSlot = $_' ast-grep --pattern 'interface TokenSlot $_' # Find references to availability property ast-grep --pattern 'availability: $_' # Look for groupSlotsByAvailability usage rg "groupSlotsByAvailability" -A 2Length of output: 2725
Script:
#!/bin/bash # Find TokenSlot type definition and related imports rg "TokenSlot" -A 3 "src/types/scheduling/schedule.ts" # Check imports in the utils file rg "import.*TokenSlot" "src/pages/Appointments/utils.ts"Length of output: 309
src/pages/Appointments/AppointmentDetail.tsx (1)
15-15
: LGTM! Clean import organization.The imports are well-organized and properly typed with the new scheduling types and APIs.
Also applies to: 47-57
src/pages/PublicAppointments/PatientRegistration.tsx (1)
43-47
: LGTM! Type safety improvements.The changes properly update the types for appointment creation and slot handling, improving type safety.
Also applies to: 74-74, 144-144
src/components/Users/UserAvailabilityTab.tsx (1)
41-43
: LGTM! Type-safe query params.Good use of TypeScript discriminated union for view state management.
public/locale/en.json (1)
195-195
: Fix typo in closed availability description.The description has a typo and grammatical error.
- "SCHEDULE_AVAILABILITY_TYPE_DESCRIPTION__closed": "Indicates the practitioner is not available for this session's time", + "SCHEDULE_AVAILABILITY_TYPE_DESCRIPTION__closed": "Indicates the practitioner is not available for this session's time",
@rithviknishad 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! 🙌 |
Required Backend Changes
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the release notes:
New Features
Improvements
Bug Fixes
Deprecations
Refactoring