modularize book appointment#71
Conversation
- breaking down the book appointment page into components closes#68
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughExtracts booking UI and logic from the BookAppointment page into five new React components (BookingForm, DateSelection, TimeSlotSelection, BookingSummary, DoctorInfoCard) and updates the parent page to import and delegate state/handlers to these components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BookAppointmentPage as Page
participant BookingForm
participant DateSelection
participant TimeSlotSelection
participant BookingSummary
participant API
Note over User,BookingForm: high-level booking interaction
User->>Page: open BookAppointment
Page->>BookingForm: render (props: schedule, doctor, handlers, state)
BookingForm->>DateSelection: render dates (schedule, vacations)
User->>DateSelection: select date
DateSelection-->>BookingForm: selectedDate
BookingForm->>TimeSlotSelection: render slots for selectedDate
User->>TimeSlotSelection: select slot
TimeSlotSelection-->>BookingForm: selectedSlot
BookingForm->>BookingSummary: render summary (date, slot, teleconsult flag, fee)
User->>BookingForm: submit (reason, teleconsult flag)
BookingForm->>API: onBooking(payload)
API-->>BookingForm: success / error
BookingForm-->>User: display success or error alert
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/pages/patient/BookAppointment.jsx (2)
56-83: Reset invalid/stale selectedSlot when date or slots change.Switching dates keeps an old slot selected, allowing submission with a slot not offered for the new date.
// Check availability when date is selected useEffect(() => { const checkDateAvailability = async () => { if (selectedDate && doctor) { try { - const response = await getDoctorAvailability(doctorId, selectedDate); + const dateAtCall = selectedDate; + const response = await getDoctorAvailability(doctorId, dateAtCall); if (response.success) { - setDateAvailability(response.data); - if (response.data.available) { - setAvailableSlots(response.data.slots || []); - } else { - setAvailableSlots([]); - } + // Ignore stale responses + if (dateAtCall !== selectedDate) return; + setDateAvailability(response.data); + const slots = response.data.available ? (response.data.slots || []) : []; + setAvailableSlots(slots); } } catch (err) { @@ } }; checkDateAvailability(); }, [selectedDate, doctorId, doctor]); + + // Clear or validate selectedSlot on date/slots change + useEffect(() => { + if (!selectedDate) { + setSelectedSlot(''); + return; + } + if (selectedSlot && !availableSlots.includes(selectedSlot)) { + setSelectedSlot(''); + } + }, [selectedDate, availableSlots, selectedSlot]);
97-105: Specify radix for parseInt to avoid subtle bugs.Add base 10 when parsing hours/minutes.
- startTime.setHours(parseInt(hours), parseInt(minutes), 0, 0); + startTime.setHours(parseInt(hours, 10), parseInt(minutes, 10), 0, 0);
🧹 Nitpick comments (6)
client/src/components/Patient/BookAppointment/BookingSummary.jsx (1)
27-29: Use Intl for INR currency formatting and fallback.Avoid concatenating the rupee sign; format properly and handle non-numeric input.
- <span>₹{consultationFee}</span> + <span> + {Number.isFinite(Number(consultationFee)) + ? new Intl.NumberFormat('en-IN', { style: 'currency', currency: 'INR' }) + .format(Number(consultationFee)) + : '—'} + </span>client/src/components/Patient/BookAppointment/TimeSlotSelection.jsx (1)
11-23: Improve accessibility for selectable slot buttons.Expose state to AT and add visible focus styles.
- <button + <button key={slot} type="button" onClick={() => setSelectedSlot(slot)} - className={`p-3 text-sm rounded-lg border transition-colors ${ + aria-pressed={selectedSlot === slot} + aria-label={`Select time ${slot}`} + title={`Select time ${slot}`} + className={`p-3 text-sm rounded-lg border transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 ${ selectedSlot === slot ? 'bg-blue-600 text-white border-blue-600' : 'bg-white text-gray-900 border-gray-300 hover:border-blue-300 hover:bg-blue-50' }`} > {slot} </button>client/src/pages/patient/BookAppointment.jsx (1)
26-54: Optional: abort/ignore setState on unmounted during initial fetch.Prevent setState after unmount or param change; improves resilience during rapid nav.
useEffect(() => { - const fetchDoctorData = async () => { + let active = true; + const fetchDoctorData = async () => { try { setLoading(true); const [doctorResponse, scheduleResponse] = await Promise.all([ getDoctorById(doctorId), getDoctorSchedule(doctorId), ]); - if (doctorResponse.success) { - setDoctor(doctorResponse.data); - } - if (scheduleResponse.success) { - setSchedule(scheduleResponse.data); - } + if (!active) return; + if (doctorResponse.success) setDoctor(doctorResponse.data); + if (scheduleResponse.success) setSchedule(scheduleResponse.data); } catch (err) { - setError('Failed to load doctor information'); + if (!active) return; + setError('Failed to load doctor information'); console.error('Error fetching doctor data:', err); } finally { - setLoading(false); + if (active) setLoading(false); } }; if (doctorId) { fetchDoctorData(); } - }, [doctorId]); + return () => { + active = false; + }; + }, [doctorId]);client/src/components/Patient/BookAppointment/DoctorInfoCard.jsx (1)
73-76: Format INR currency via Intl with fallback.Avoid manual rupee sign; handle thousands/decimals and non-numeric safely.
- <div className="flex items-center"> - <span className="font-medium">Consultation Fee:</span> - <span className="ml-2">₹{doctor.consultationFee}</span> - </div> + <div className="flex items-center"> + <span className="font-medium">Consultation Fee:</span> + <span className="ml-2"> + {Number.isFinite(Number(doctor.consultationFee)) + ? new Intl.NumberFormat('en-IN', { style: 'currency', currency: 'INR' }) + .format(Number(doctor.consultationFee)) + : '—'} + </span> + </div>client/src/components/Patient/BookAppointment/BookingForm.jsx (2)
6-23: Consider Context API for cleaner state management.The component receives 12+ props, indicating props drilling. While this works, if the component tree grows deeper or more state is added, consider using React Context or a state management library to reduce coupling and improve maintainability.
109-122: Consider adding doctor existence check to button disabled logic.Line 111 disables the button when loading, or when date/slot/reason are missing. For defensive coding, consider also checking
!doctorto prevent submission attempts before doctor data loads.Apply this diff:
- disabled={loading || !selectedDate || !selectedSlot || !reason.trim()} + disabled={loading || !doctor || !selectedDate || !selectedSlot || !reason.trim()}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
client/src/components/Patient/BookAppointment/BookingForm.jsx(1 hunks)client/src/components/Patient/BookAppointment/BookingSummary.jsx(1 hunks)client/src/components/Patient/BookAppointment/DateSelection.jsx(1 hunks)client/src/components/Patient/BookAppointment/DoctorInfoCard.jsx(1 hunks)client/src/components/Patient/BookAppointment/TimeSlotSelection.jsx(1 hunks)client/src/pages/patient/BookAppointment.jsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
client/src/components/Patient/BookAppointment/TimeSlotSelection.jsx (1)
client/src/pages/patient/BookAppointment.jsx (2)
availableSlots(17-17)selectedSlot(18-18)
client/src/components/Patient/BookAppointment/BookingSummary.jsx (1)
client/src/pages/patient/BookAppointment.jsx (3)
selectedDate(16-16)selectedSlot(18-18)isTeleconsultation(20-20)
client/src/components/Patient/BookAppointment/BookingForm.jsx (4)
client/src/pages/patient/BookAppointment.jsx (11)
error(22-22)success(23-23)selectedDate(16-16)schedule(15-15)dateAvailability(24-24)availableSlots(17-17)selectedSlot(18-18)reason(19-19)doctor(14-14)isTeleconsultation(20-20)loading(21-21)client/src/components/Patient/BookAppointment/DateSelection.jsx (1)
DateSelection(3-93)client/src/components/Patient/BookAppointment/TimeSlotSelection.jsx (1)
TimeSlotSelection(3-28)client/src/components/Patient/BookAppointment/BookingSummary.jsx (1)
BookingSummary(2-33)
client/src/components/Patient/BookAppointment/DateSelection.jsx (2)
client/src/components/public/Nearbyclinics/ClinicCard.jsx (1)
today(9-9)client/src/pages/patient/BookAppointment.jsx (3)
schedule(15-15)selectedDate(16-16)dateAvailability(24-24)
client/src/pages/patient/BookAppointment.jsx (2)
client/src/components/Patient/BookAppointment/DoctorInfoCard.jsx (1)
DoctorInfoCard(3-141)client/src/components/Patient/BookAppointment/BookingForm.jsx (1)
BookingForm(6-127)
client/src/components/Patient/BookAppointment/DoctorInfoCard.jsx (1)
client/src/pages/patient/BookAppointment.jsx (2)
doctor(14-14)schedule(15-15)
🔇 Additional comments (1)
client/src/components/Patient/BookAppointment/BookingForm.jsx (1)
1-4: LGTM!Clean imports with appropriate icons and child components.
| {new Date(selectedDate).toLocaleDateString('en-US', { | ||
| weekday: 'long', | ||
| year: 'numeric', | ||
| month: 'long', | ||
| day: 'numeric', | ||
| })} | ||
| </span> |
There was a problem hiding this comment.
Fix off-by-one date: parse YYYY-MM-DD as local, not UTC.
new Date('YYYY-MM-DD') parses as UTC and can display the wrong day in many timezones. Parse as local parts and format via Intl.
Apply:
+// Parse 'YYYY-MM-DD' as a local date (no timezone shift)
+const parseISODateLocal = (iso) => {
+ if (!iso) return null;
+ const [y, m, d] = iso.split('-').map(Number);
+ return new Date(y, m - 1, d);
+};
+
const BookingSummary = ({ selectedDate, selectedSlot, isTeleconsultation, consultationFee }) => {
- return (
+ const dateObj = parseISODateLocal(selectedDate);
+ const formattedDate = dateObj
+ ? new Intl.DateTimeFormat(undefined, {
+ weekday: 'long',
+ year: 'numeric',
+ month: 'long',
+ day: 'numeric',
+ }).format(dateObj)
+ : '';
+ return (
@@
- {new Date(selectedDate).toLocaleDateString('en-US', {
- weekday: 'long',
- year: 'numeric',
- month: 'long',
- day: 'numeric',
- })}
+ {formattedDate}
</span>📝 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.
| {new Date(selectedDate).toLocaleDateString('en-US', { | |
| weekday: 'long', | |
| year: 'numeric', | |
| month: 'long', | |
| day: 'numeric', | |
| })} | |
| </span> | |
| // Parse 'YYYY-MM-DD' as a local date (no timezone shift) | |
| const parseISODateLocal = (iso) => { | |
| if (!iso) return null; | |
| const [y, m, d] = iso.split('-').map(Number); | |
| return new Date(y, m - 1, d); | |
| }; | |
| const BookingSummary = ({ selectedDate, selectedSlot, isTeleconsultation, consultationFee }) => { | |
| const dateObj = parseISODateLocal(selectedDate); | |
| const formattedDate = dateObj | |
| ? new Intl.DateTimeFormat(undefined, { | |
| weekday: 'long', | |
| year: 'numeric', | |
| month: 'long', | |
| day: 'numeric', | |
| }).format(dateObj) | |
| : ''; | |
| return ( | |
| // ... rest of component | |
| {formattedDate} | |
| </span> | |
| // ... rest of component | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In client/src/components/Patient/BookAppointment/BookingSummary.jsx around lines
10 to 16, the code constructs a Date from a YYYY-MM-DD string which is
interpreted as UTC and can shift the displayed day in some timezones; instead,
parse the selectedDate string into year/month/day integers and build a local
Date via new Date(year, monthIndex, day) (monthIndex = month - 1), then format
it with toLocaleDateString or Intl.DateTimeFormat as before; replace the direct
new Date(selectedDate) usage with this local-date construction to eliminate the
off-by-one day issue.
| const today = new Date(); | ||
|
|
||
| for (let i = 1; i <= 30; i++) { | ||
| const date = new Date(today); | ||
| date.setDate(today.getDate() + i); | ||
| dates.push(date); | ||
| } | ||
|
|
||
| return dates; | ||
| }; |
There was a problem hiding this comment.
Blocker: timezone/date normalization bug can select the wrong day.
- generateAvailableDates keeps the current time-of-day.
- dateStr uses toISOString() (UTC), while UI uses local; they can disagree.
- Vacation checks parse 'YYYY‑MM‑DD' as UTC.
Normalize to local start-of-day and build/send local ISO date strings.
- const generateAvailableDates = () => {
+ // Helpers
+ const startOfDay = (d) => {
+ const x = new Date(d);
+ x.setHours(0, 0, 0, 0);
+ return x;
+ };
+ const toISODateLocal = (d) => {
+ const y = d.getFullYear();
+ const m = String(d.getMonth() + 1).padStart(2, '0');
+ const day = String(d.getDate()).padStart(2, '0');
+ return `${y}-${m}-${day}`;
+ };
+ const parseISODateLocal = (iso) => {
+ const [y, m, d] = iso.slice(0, 10).split('-').map(Number);
+ return new Date(y, m - 1, d);
+ };
+
+ const generateAvailableDates = () => {
const dates = [];
- const today = new Date();
+ const today = startOfDay(new Date());
for (let i = 1; i <= 30; i++) {
- const date = new Date(today);
- date.setDate(today.getDate() + i);
- dates.push(date);
+ const d = new Date(today);
+ d.setDate(today.getDate() + i);
+ dates.push(d);
}
return dates;
};
@@
- const dayName = date.toLocaleDateString('en-US', { weekday: 'long' }).toLowerCase();
+ const dayName = date.toLocaleDateString('en-US', { weekday: 'long' }).toLowerCase();
@@
- return schedule.vacations.some((vacation) => {
- const vacationStart = new Date(vacation.startDate);
- const vacationEnd = new Date(vacation.endDate);
- return date >= vacationStart && date <= vacationEnd;
- });
+ return schedule.vacations.some((vacation) => {
+ const vacationStart = parseISODateLocal(vacation.startDate);
+ const vacationEnd = parseISODateLocal(vacation.endDate);
+ return date >= vacationStart && date <= vacationEnd;
+ });
@@
- const dateStr = date.toISOString().split('T')[0];
+ const dateStr = toISODateLocal(date);Also applies to: 22-26, 32-37, 49-52, 60-66
🤖 Prompt for AI Agents
In client/src/components/Patient/BookAppointment/DateSelection.jsx around lines
7 to 16 (and likewise adjust ranges 22-26, 32-37, 49-52, 60-66): the code
creates Date objects that retain the current time-of-day and later uses
toISOString() / UTC parsing which can shift the day vs the UI local date; change
generation to normalize each Date to the local start-of-day (set
hours/minutes/seconds/ms to 0) when pushing available dates, produce and use
local date strings in YYYY-MM-DD (no timezone offset) for UI keys/values, and
when comparing against vacations parse incoming 'YYYY-MM-DD' as local dates (or
construct local Date at start-of-day) so comparisons are done on normalized
local dates consistently before sending/storing.
| {schedule?.vacations?.filter((vacation) => new Date(vacation.endDate) >= new Date()) | ||
| .length > 0 && ( | ||
| <div className="mt-6 pt-6 border-t"> | ||
| <h3 className="text-sm font-medium text-gray-900 mb-3">Upcoming Vacations</h3> | ||
| <div className="space-y-2"> | ||
| {schedule.vacations | ||
| .filter((vacation) => new Date(vacation.endDate) >= new Date()) | ||
| .map((vacation, index) => ( | ||
| <div key={index} className="bg-red-50 p-2 rounded text-xs"> | ||
| <div className="font-medium text-red-800"> | ||
| {new Date(vacation.startDate).toLocaleDateString()} -{' '} | ||
| {new Date(vacation.endDate).toLocaleDateString()} | ||
| </div> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Prevent vacation date off-by-one: parse ISO as local and format via Intl.
Avoid UTC parsing of 'YYYY-MM-DD'.
- {schedule?.vacations?.filter((vacation) => new Date(vacation.endDate) >= new Date())
+ {schedule?.vacations?.filter((vacation) => {
+ const end = new Date(...vacation.endDate.slice(0,10).split('-').map((v,i)=> i===1 ? Number(v)-1 : Number(v)));
+ return end >= new Date();
+ })
.length > 0 && (
@@
- {schedule.vacations
- .filter((vacation) => new Date(vacation.endDate) >= new Date())
+ {schedule.vacations
+ .filter((vacation) => {
+ const end = new Date(...vacation.endDate.slice(0,10).split('-').map((v,i)=> i===1 ? Number(v)-1 : Number(v)));
+ return end >= new Date();
+ })
.map((vacation, index) => (
<div key={index} className="bg-red-50 p-2 rounded text-xs">
<div className="font-medium text-red-800">
- {new Date(vacation.startDate).toLocaleDateString()} -{' '}
- {new Date(vacation.endDate).toLocaleDateString()}
+ {new Intl.DateTimeFormat(undefined, { dateStyle: 'medium' }).format(
+ new Date(...vacation.startDate.slice(0,10).split('-').map((v,i)=> i===1 ? Number(v)-1 : Number(v)))
+ )} -{' '}
+ {new Intl.DateTimeFormat(undefined, { dateStyle: 'medium' }).format(
+ new Date(...vacation.endDate.slice(0,10).split('-').map((v,i)=> i===1 ? Number(v)-1 : Number(v)))
+ )}
</div>
</div>
))}Alternatively, centralize this logic in a small util (e.g., parseISODateLocal, formatDate) used across components.
Also applies to: 129-132
🤖 Prompt for AI Agents
In client/src/components/Patient/BookAppointment/DoctorInfoCard.jsx around lines
120 to 136 (also addressing lines 129-132), the code constructs Date objects
directly from 'YYYY-MM-DD' strings which are parsed as UTC and can cause
off-by-one day errors; replace the inline new Date(...) usage with a local-ISO
parser and Intl-based formatter: implement or import a small util (e.g.,
parseISODateLocal(isoString) that splits YYYY-MM-DD and constructs a Date in
local time, and formatDate(date) that uses Intl.DateTimeFormat) and use those
helpers for both startDate and endDate rendering and for the filter comparison
so dates are compared and displayed in local time consistently across
components.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
client/src/components/Patient/BookAppointment/BookingForm.jsx (2)
83-96: The null safety issue flagged in the previous review remains unaddressed.Line 83 still accesses
doctor.availableForTeleconsultationwithout null checking. As noted in the previous review, this will cause a runtime error whendoctorisnullduring initial render.Apply the previously suggested fix:
- {doctor.availableForTeleconsultation && ( + {doctor?.availableForTeleconsultation && ( <div className="flex items-center">
99-106: The null safety issue fordoctor.consultationFeeremains unaddressed.Line 104 still accesses
doctor.consultationFeewithout null checking. As flagged previously, this will throw a runtime error ifdoctorisnullorundefinedwhen bothselectedDateandselectedSlotare set.Apply one of the previously suggested fixes:
Option 1: Use optional chaining:
- consultationFee={doctor.consultationFee} + consultationFee={doctor?.consultationFee ?? 0}Option 2: Add
doctorguard to the conditional:- {selectedDate && selectedSlot && ( + {selectedDate && selectedSlot && doctor && ( <BookingSummary
🧹 Nitpick comments (1)
client/src/components/Patient/BookAppointment/TimeSlotSelection.jsx (1)
3-28: Consider adding PropTypes for runtime validation.Adding PropTypes helps catch prop type mismatches during development and serves as inline documentation.
Add PropTypes at the end of the file:
import PropTypes from 'prop-types'; // ... existing code ... TimeSlotSelection.propTypes = { availableSlots: PropTypes.arrayOf(PropTypes.string).isRequired, selectedSlot: PropTypes.string.isRequired, setSelectedSlot: PropTypes.func.isRequired, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
client/src/components/Patient/BookAppointment/BookingForm.jsx(1 hunks)client/src/components/Patient/BookAppointment/BookingSummary.jsx(1 hunks)client/src/components/Patient/BookAppointment/DateSelection.jsx(1 hunks)client/src/components/Patient/BookAppointment/DoctorInfoCard.jsx(1 hunks)client/src/components/Patient/BookAppointment/TimeSlotSelection.jsx(1 hunks)client/src/pages/patient/BookAppointment.jsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- client/src/components/Patient/BookAppointment/DateSelection.jsx
- client/src/components/Patient/BookAppointment/DoctorInfoCard.jsx
- client/src/components/Patient/BookAppointment/BookingSummary.jsx
- client/src/pages/patient/BookAppointment.jsx
🧰 Additional context used
🧬 Code graph analysis (2)
client/src/components/Patient/BookAppointment/BookingForm.jsx (4)
client/src/pages/patient/BookAppointment.jsx (11)
error(22-22)success(23-23)selectedDate(16-16)schedule(15-15)dateAvailability(24-24)availableSlots(17-17)selectedSlot(18-18)reason(19-19)doctor(14-14)isTeleconsultation(20-20)loading(21-21)client/src/components/Patient/BookAppointment/DateSelection.jsx (1)
DateSelection(3-93)client/src/components/Patient/BookAppointment/TimeSlotSelection.jsx (1)
TimeSlotSelection(3-28)client/src/components/Patient/BookAppointment/BookingSummary.jsx (1)
BookingSummary(1-32)
client/src/components/Patient/BookAppointment/TimeSlotSelection.jsx (1)
client/src/pages/patient/BookAppointment.jsx (2)
availableSlots(17-17)selectedSlot(18-18)
🔇 Additional comments (2)
client/src/components/Patient/BookAppointment/BookingForm.jsx (2)
6-23: Modularization successfully reduces inline complexity.The component accepts 16 props which is on the higher side, but this is reasonable given the PR's objective to extract inline booking logic into reusable components. The clear prop naming and focused responsibility (orchestrating the booking form flow) make this acceptable.
48-48: Remove this review comment—no actual risk exists.The
onBookingprop is always provided by the parent componentBookAppointment.jsx(line 189:onBooking={handleBooking}), wherehandleBookingis always defined (line 85). The form submission will not fail due to a missing or undefinedonBookingprop. While adding PropTypes validation would be a code quality improvement, the current code has no undefined prop issue and works correctly as-is.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Refactor