Appointment detail flicker problem resolved #9987
Conversation
|
Warning Rate limit exceeded@NikhilA8606 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 3 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 (2)
WalkthroughThe pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant User
participant AppointmentDialog
participant QueryClient
participant Backend
User->>AppointmentDialog: Open Dialog
AppointmentDialog->>AppointmentDialog: Render Appointment Details
User->>AppointmentDialog: Click Cancel Appointment
AppointmentDialog->>Backend: Send Cancellation Request
Backend-->>AppointmentDialog: Cancellation Response
AppointmentDialog->>QueryClient: Invalidate Queries
AppointmentDialog->>User: Close Dialog
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 (
|
NikhilA8606
changed the title
✅ 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: 3
🧹 Nitpick comments (3)
src/pages/Patient/AppointmentDialog.tsx (2)
50-52: Remove debug console.log statement.The console.log statement for debugging the open state should be removed before merging.
- useEffect(() => { - console.log("IsOpen", open); - }, [open]);
42-42: Remove debug console.log statement.Remove the console.log statement from the mutation success callback.
- console.log("Mutation Success!");src/pages/Patient/index.tsx (1)
Line range hint
63-71: Optimize appointment filtering.The appointments are filtered twice, once for past and once for scheduled appointments. Consider filtering once and storing the results.
+ const now = dayjs(); + const [pastAppointments, scheduledAppointments] = appointments?.reduce( + (acc, appointment) => { + const isPast = now.isAfter(dayjs(appointment.token_slot.start_datetime)); + acc[isPast ? 0 : 1].push(appointment); + return acc; + }, + [[], []] as [Appointment[], Appointment[]] + ) ?? [[], []]; - const pastAppointments = appointments?.filter((appointment) => - dayjs().isAfter(dayjs(appointment.token_slot.start_datetime)), - ); - - const scheduledAppointments = appointments?.filter((appointment) => - dayjs().isBefore(dayjs(appointment.token_slot.start_datetime)), - );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.env(1 hunks)src/pages/Patient/AppointmentDialog.tsx(1 hunks)src/pages/Patient/index.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/pages/Patient/AppointmentDialog.tsx (1)
104-106: Add onClick handler for reschedule button.The reschedule button is currently non-functional. Either implement the reschedule functionality or remove the button if the feature is not ready.
src/pages/Patient/index.tsx (1)
Line range hint
165-173: LGTM! Dialog integration looks good.The AppointmentDialog integration is well implemented with proper state management for visibility and selected appointment.
| {open && ( | ||
| <DialogContent className="p-0"> | ||
| <DialogHeader className="p-3"> | ||
| <DialogTitle>{t("appointment_details")}</DialogTitle> | ||
| <DialogDescription className="mb-4"> | ||
| {t("appointment_details")} | ||
| </DialogDescription> | ||
| <div className="flex flex-row justify-between"> | ||
| <div className="space-y-1"> | ||
| <Label className="text-xs">{t("practitioner")}</Label> | ||
| <p className="text-base font-semibold"> | ||
| {formatName(appointment.user)} | ||
| </p> | ||
| <p className="text-sm font-semibold text-gray-600"> | ||
| {formatAppointmentSlotTime(appointment)} | ||
| </p> | ||
| </div> | ||
| <div className="space-y-1"> | ||
| <Label className="text-xs">{t("patient_name")}</Label> | ||
| <p className="font-semibold text-base"> | ||
| {appointment.patient.name} | ||
| </p> | ||
| <p className="text-sm text-gray-600 font-medium"> | ||
| {formatPatientAge(appointment.patient as any, true)},{" "} | ||
| {t(`GENDER__${appointment.patient.gender}`)} | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </DialogHeader> | ||
| <DialogFooter className="flex flex-row sm:justify-between items-center bg-blue-200 m-0 w-full p-3 rounded-b-lg"> | ||
| <span className="text-sm font-semibold text-blue-700"> | ||
| {t(appointment.status)} | ||
| </span> | ||
| <span className="flex flex-row gap-2"> | ||
| <Button | ||
| variant="destructive" | ||
| disabled={isPending} | ||
| onClick={() => { | ||
| cancelAppointment({ | ||
| appointment: appointment.id, | ||
| patient: appointment.patient.id, | ||
| }); | ||
| }} | ||
| > | ||
| <span>{t("cancelli")}</span> | ||
| </Button> | ||
| <Button variant="secondary"> | ||
| <span>{t("reschedule")}</span> | ||
| </Button> | ||
| </span> | ||
| </DialogFooter> | ||
| </DialogContent> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent dialog content flicker.
The current implementation may cause flickering as the dialog content is conditionally rendered based on the open state. To prevent this, move the condition to control visibility rather than mounting/unmounting.
<Dialog open={open} onOpenChange={onOpenChange}>
- {open && (
<DialogContent className="p-0">
{/* ... dialog content ... */}
</DialogContent>
- )}
</Dialog>Committable suggestion skipped: line range outside the PR's diff.
| {appointment.patient.name} | ||
| </p> | ||
| <p className="text-sm text-gray-600 font-medium"> | ||
| {formatPatientAge(appointment.patient as any, true)},{" "} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unsafe type casting.
The type casting to any should be avoided. Instead, ensure proper typing for the patient object.
- {formatPatientAge(appointment.patient as any, true)},{" "}
+ {formatPatientAge(appointment.patient, true)},{" "}Committable suggestion skipped: line range outside the PR's diff.
.env
Outdated
| @@ -7,7 +7,7 @@ REACT_APP_COVER_IMAGE_ALT=https://cdn.ohc.network/care_logo.svg | |||
| REACT_PUBLIC_URL=https://care.ohc.network | |||
|
|
|||
| # Care API URL without the /api prefix | |||
| REACT_CARE_API_URL=https://careapi.ohc.network | |||
| REACT_CARE_API_URL=https://care-api.do.ohc.network | |||
There was a problem hiding this comment.
💡 Codebase verification
Update API endpoint references in configuration files
The new Care API endpoint has been configured in .env, but there are still references to the old endpoint that need to be updated:
vercel.json: Update the destination URL fromcareapi.ohc.networktocare-api.do.ohc.networkREADME.md: Update the Swagger documentation link to use the new endpoint
🔗 Analysis chain
Verify API endpoint change impact.
The Care API endpoint has been updated. Please ensure:
- The new endpoint is properly configured and accessible
- All API calls in the application are tested with the new endpoint
- No hardcoded references to the old endpoint exist in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any hardcoded references to the old API endpoint
rg -i "careapi\.ohc\.network"
Length of output: 179
Script:
#!/bin/bash
# Search for variations of both endpoints and environment variable usage
rg -i "care.?api\..*ohc\.network"
rg "REACT_CARE_API_URL"
fd -e json -e yaml -e yml -e env | xargs cat
Length of output: 69284
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/pages/Patient/AppointmentDialog.tsx (4)
49-51: Remove development console.log statement.Remove the console.log statement used for debugging as it's not needed in production code.
- useEffect(() => { - console.log("IsOpen", open); - }, [open]);
53-53: Add loading state handling.Instead of returning an empty fragment when appointment is undefined, consider showing a loading state or skeleton UI to improve user experience.
- if (!appointment) return <></>; + if (!appointment) { + return ( + <Dialog open={open} onOpenChange={onOpenChange}> + <DialogContent className="p-0"> + <div className="p-3 animate-pulse"> + <div className="h-4 bg-gray-200 rounded w-1/4 mb-4"></div> + <div className="space-y-3"> + <div className="h-4 bg-gray-200 rounded w-3/4"></div> + <div className="h-4 bg-gray-200 rounded w-1/2"></div> + </div> + </div> + </DialogContent> + </Dialog> + ); + }
86-86: Validate appointment status translation key.Direct usage of status as translation key could lead to missing translations. Consider maintaining a mapping of status to translation keys.
- {t(appointment.status)} + {t(APPOINTMENT_STATUS_TRANSLATIONS[appointment.status] ?? 'unknown_status')}
24-109: Consider splitting the component for better maintainability.While the component successfully implements the appointment dialog functionality, it could benefit from being split into smaller, more focused components:
- A separate component for patient information display
- A separate component for action buttons
- A custom hook for appointment cancellation logic
This would improve maintainability and make the code more testable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Patient/AppointmentDialog.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test
- GitHub Check: CodeQL-Build
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/pages/Patient/AppointmentDialog.tsx (1)
78-78: Remove unsafe type casting.The type casting to
anyshould be avoided. Instead, ensure proper typing for the patient object.- {formatPatientAge(appointment.patient as any, true)},{" "} + {formatPatientAge(appointment.patient, true)},{" "}
| <Button variant="secondary"> | ||
| <span>{t("reschedule")}</span> | ||
| </Button> |
There was a problem hiding this comment.
Add onClick handler for reschedule button.
The reschedule button is missing an onClick handler, making it non-functional.
- <Button variant="secondary">
+ <Button
+ variant="secondary"
+ onClick={() => {
+ // Add reschedule logic here
+ // Consider navigating to reschedule page or opening reschedule dialog
+ }}
+ >
<span>{t("reschedule")}</span>
</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button variant="secondary"> | |
| <span>{t("reschedule")}</span> | |
| </Button> | |
| <Button | |
| variant="secondary" | |
| onClick={() => { | |
| // Add reschedule logic here | |
| // Consider navigating to reschedule page or opening reschedule dialog | |
| }} | |
| > | |
| <span>{t("reschedule")}</span> | |
| </Button> |
| const { mutate: cancelAppointment, isPending } = useMutation({ | ||
| mutationFn: mutate(PublicAppointmentApi.cancelAppointment, { | ||
| headers: { | ||
| Authorization: `Bearer ${tokenData?.token}`, | ||
| }, | ||
| }), | ||
| onSuccess: () => { | ||
| console.log("Mutation Success!"); | ||
| queryClient.invalidateQueries({ | ||
| queryKey: ["appointment", tokenData?.phoneNumber], | ||
| }); | ||
| props.setAppointmentDialogOpen(false); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance mutation handling.
The mutation setup needs improvements:
- Remove the console.log statement
- Add error handling
- Show success/error feedback to the user
const { mutate: cancelAppointment, isPending } = useMutation({
mutationFn: mutate(PublicAppointmentApi.cancelAppointment, {
headers: {
Authorization: `Bearer ${tokenData?.token}`,
},
}),
onSuccess: () => {
- console.log("Mutation Success!");
queryClient.invalidateQueries({
queryKey: ["appointment", tokenData?.phoneNumber],
});
props.setAppointmentDialogOpen(false);
+ // Show success toast/notification
+ toast.success(t("appointment_cancelled_successfully"));
},
+ onError: (error) => {
+ // Show error toast/notification
+ toast.error(t("error_cancelling_appointment"));
+ console.error("Error cancelling appointment:", error);
+ },
});Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
We have a pattern that we follow for placing components.
src/pages/<module_name>/<page_name> are where the pages are kept
src/pages/<module_name>/components/<component_name> are where the components of that module are kept
let's move this file to components folder of this module.
|
@NikhilA8606 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! 🙌 |
Appointment page flicker issue
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
AppointmentDialogcomponent for managing patient appointment detailsImprovements