-
Notifications
You must be signed in to change notification settings - Fork 2
feat: QuickLab ecosystem integration with cross-navigation, patient appointments, and lab admin dashboard #110
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
Conversation
…igned staff and in-lab search filter
…d import Manage Staff page
…ts/appointments/settings
…arer error messages
…ing by patient city
- Convert sampleType to lowercase in addTest and updateTest functions - Remove unnecessary dynamic Lab model import in addTest - Fixes validation error when saving test data with capital letters
- Fix LabAdmin import path from '../models/Users/' to '../../models/Lab/' - Fix LabStaff import path from '../models/Users/' to '../../models/Lab/' - Resolves 500 error in getLabAppointments endpoint
- Replace single textarea with structured address fields - Add fields: street, city, state, zipCode, country - Default country to 'India' - Fixes address not being saved to database issue
- Create comprehensive appointment management interface - Features: search, filters (status/type/date), summary cards - Staff assignment modal for home collection appointments - Status update modal with all available statuses - Display home collection addresses with proper formatting - Full dark mode support with transition-colors
- Create CRUD interface for managing lab tests - Features: search, add/edit modal, activate/deactivate tests - Comprehensive form with all test fields (pricing, categories, sample types) - Test categories: blood, urine, stool, imaging, ECG, other - Grid view with test cards showing key details - Full dark mode support
- Add /quick-lab/appointments route with LabAdminSetupGuard - Add /quick-lab/tests route with LabAdminSetupGuard - Both routes require profile and lab setup - Import LabAdminAppointments and LabAdminManageTests components
- Add Appointments button with Calendar icon - Add Tests button with TestTube icon - Add Staff button (shortened from 'Manage Staff') - All buttons visible only for lab_admin role - Import Calendar and TestTube icons from lucide-react
- Add Appointments button (Appts) with Calendar icon - Add Tests button with TestTube icon - Add Staff button with Users icon - All buttons in bottom navigation for lab_admin role - Import Calendar and TestTube icons from lucide-react
- Redirect lab_admin and lab_staff to their dashboards automatically - Use getDashboardPath utility for proper role routing - Other roles (patient, doctor, admin) remain on homepage - Import useAuth, useNavigate, and getDashboardPath
…d Quick Lab (public + patient, desktop + mobile)
…(desktop & mobile)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces QuickLab, a comprehensive lab management and appointment booking feature spanning patient-facing search and booking, lab administrator dashboard and staff management, lab staff onboarding and assignment workflows. Changes include new UI components, role-based routing guards, API service wrappers, backend controllers with multiple endpoints, and database schema extensions to support lab operations across three user roles. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📋 PR Auto-Check📝 Files Changed✨ Code QualityPrettier Check: ✅ Passed Auto-generated on 2025-12-28T06:27:14.916Z |
📋 PR Auto-Check📝 Files Changed✨ Code QualityPrettier Check: ✅ Passed Auto-generated on 2025-12-28T06:29:00.495Z |
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: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/components/quicklab/MobileNavbar.jsx (1)
232-241: Home and Labs buttons lack onClick handlers.These buttons in the bottom navigation are not wired to any navigation action.
🔎 Proposed fix
{/* Home/Labs */} - <button className="flex flex-col items-center justify-center w-full h-full space-y-1 text-slate-500 dark:text-slate-400 hover:text-yellow-600 dark:hover:text-yellow-400 transition-colors active:scale-95"> + <button + onClick={() => navigate('/quick-lab')} + className="flex flex-col items-center justify-center w-full h-full space-y-1 text-slate-500 dark:text-slate-400 hover:text-yellow-600 dark:hover:text-yellow-400 transition-colors active:scale-95" + > <Home className="h-6 w-6" /> <span className="text-xs font-medium">Home</span> </button> {/* Labs */} - <button className="flex flex-col items-center justify-center w-full h-full space-y-1 text-slate-500 dark:text-slate-400 hover:text-yellow-600 dark:hover:text-yellow-400 transition-colors active:scale-95"> + <button + onClick={() => navigate('/quick-lab/search')} + className="flex flex-col items-center justify-center w-full h-full space-y-1 text-slate-500 dark:text-slate-400 hover:text-yellow-600 dark:hover:text-yellow-400 transition-colors active:scale-95" + > <BookOpen className="h-6 w-6" /> <span className="text-xs font-medium">Labs</span> </button>client/src/pages/doctor/AppointmentDetails.jsx (1)
289-295: MissinglabIdfield when loading existing prescription tests for editing.When editing an existing prescription, the
testsarray fallback on line 292 only includes{ name: '', instructions: '' }but omitslabId. This inconsistency with the initial state (line 46) andaddTest(line 227) means existing tests won't have their associated lab preserved when editing.🔎 Proposed fix
tests: prescription.tests && prescription.tests.length > 0 ? prescription.tests - : [{ name: '', instructions: '' }], + : [{ name: '', instructions: '', labId: '' }],
♻️ Duplicate comments (1)
client/src/components/Patient/PatientDesktopNavbar.jsx (1)
88-118: Code duplication with public DesktopNavbar.This Explore dropdown implementation duplicates the same logic from
client/src/components/public/DesktopNavbar.jsx(lines 11-19 and 53-83). See the review comment on that file for a suggested approach to extract this into a reusable component.
🟡 Minor comments (18)
client/src/service/prescriptionApiSevice.js-54-61 (1)
54-61: Add getNearbyLabs to the default export object.The function is defined and appears functional, but it's missing from the default export object at the bottom of the file (lines 62-70). This inconsistency means consumers importing the default export won't have access to this new function.
🔎 Proposed fix
export default { createPrescription, getPatientPrescriptions, getDoctorPrescriptions, getPrescription, getAppointmentPrescription, updatePrescription, getPatientAppointmentPrescription, + getNearbyLabs, };client/src/components/auth/ErrorMessage.jsx-10-10 (1)
10-10: Define theanimate-slideDownanimation.The component uses the
animate-slideDownclass, but this animation is not defined anywhere in the codebase. Add the animation definition to your CSS or Tailwind configuration. For example, add to your CSS:@keyframes slideDown { from { opacity: 0; transform: translateY(-10px); } to { opacity: 1; transform: translateY(0); } } .animate-slideDown { animation: slideDown 0.3s ease-out; }Alternatively, extend your Tailwind configuration with a custom animation.
client/src/pages/quicklab/LabStaffProfileComplete.jsx-27-32 (1)
27-32: Memory leak: Object URL not revoked when changing or clearing profile picture.Similar to the other component, revoke the previous URL before creating a new one.
🔎 Proposed fix
const handleChange = (e) => { const { name, value, type } = e.target; if (type === 'file') { const file = e.target.files[0]; if (file) { + if (previewUrl) URL.revokeObjectURL(previewUrl); setProfilePicture(file); setPreviewUrl(URL.createObjectURL(file)); } } else if (name === 'experience') {Also update the clear button handler (lines 108-111):
<button type="button" onClick={() => { + if (previewUrl) URL.revokeObjectURL(previewUrl); setProfilePicture(null); setPreviewUrl(null); }}Committable suggestion skipped: line range outside the PR's diff.
client/src/pages/quicklab/LabAdminAddLab.jsx-51-61 (1)
51-61: Memory leak: Object URLs are not revoked.
URL.createObjectURL()allocates browser memory that persists until explicitly released. Revoke previous URLs when replacing them or on unmount.🔎 Proposed fix
+import { useState, useEffect, useRef } from 'react'; ... export default function LabAdminAddLab() { const navigate = useNavigate(); const [form, setForm] = useState(initialState); const [logoFile, setLogoFile] = useState(null); const [photoFiles, setPhotoFiles] = useState([]); const [logoPreview, setLogoPreview] = useState(null); const [photoPreviews, setPhotoPreviews] = useState([]); const [loading, setLoading] = useState(false); const [error, setError] = useState(''); + // Cleanup object URLs on unmount or when previews change + useEffect(() => { + return () => { + if (logoPreview) URL.revokeObjectURL(logoPreview); + photoPreviews.forEach((url) => URL.revokeObjectURL(url)); + }; + }, []); const handleLogoChange = (e) => { const file = e.target.files?.[0]; + if (logoPreview) URL.revokeObjectURL(logoPreview); setLogoFile(file || null); setLogoPreview(file ? URL.createObjectURL(file) : null); }; const handlePhotosChange = (e) => { const files = Array.from(e.target.files || []); + photoPreviews.forEach((url) => URL.revokeObjectURL(url)); setPhotoFiles(files); setPhotoPreviews(files.map((file) => URL.createObjectURL(file))); };Committable suggestion skipped: line range outside the PR's diff.
server/Controllers/QuickLab/labAdminController.js-82-86 (1)
82-86: Number conversion may produce NaN without validation.If
generalHomeCollectionFeecontains non-numeric input,Number()returnsNaNwhich would be stored in the database.🔎 Proposed fix
// Normalize numeric fee - const generalHomeCollectionFee = - labData.generalHomeCollectionFee !== undefined && labData.generalHomeCollectionFee !== '' - ? Number(labData.generalHomeCollectionFee) - : undefined; + let generalHomeCollectionFee; + if (labData.generalHomeCollectionFee !== undefined && labData.generalHomeCollectionFee !== '') { + const parsed = Number(labData.generalHomeCollectionFee); + if (!isNaN(parsed)) { + generalHomeCollectionFee = parsed; + } + }server/Controllers/QuickLab/labAdminController.js-413-416 (1)
413-416: Same NaN concern in updateLabInfo.The
generalHomeCollectionFeeconversion here has the same issue as increateLab.🔎 Proposed fix
let generalHomeCollectionFee = lab.generalHomeCollectionFee; if (body.generalHomeCollectionFee !== undefined && body.generalHomeCollectionFee !== '') { - generalHomeCollectionFee = Number(body.generalHomeCollectionFee); + const parsed = Number(body.generalHomeCollectionFee); + if (!isNaN(parsed)) { + generalHomeCollectionFee = parsed; + } }client/src/routes/guards/LabAdminPreventGuard.jsx-21-24 (1)
21-24: Guard should only allow lab_admin role, not lab_staff.The guard allows both
lab_staffandlab_adminon line 21, but whenpreventProfile=true, it always callscheckLabAdminProfileExists()which queries the/lab-admin/profile/statusendpoint—a lab admin-specific resource. Lab staff users would not have a lab admin profile, making this combination inconsistent. SinceLabAdminPreventGuardis exclusively for admin-specific flows (profile completion, lab setup), line 21 should restrict tolab_adminonly:user?.role !== 'lab_admin'.client/src/components/auth/PasswordInput.jsx-39-45 (1)
39-45: Missing accessible label on visibility toggle button.The toggle button removed
aria-label, leaving it without an accessible name for screen reader users. Add anaria-labelto communicate the button's purpose.🔎 Proposed fix
<button type="button" onClick={() => setShowPassword(!showPassword)} className="absolute inset-y-0 right-0 pr-3 flex items-center text-gray-400 hover:text-gray-600 dark:hover:text-gray-300" + aria-label={showPassword ? 'Hide password' : 'Show password'} > {showPassword ? <EyeOff className="h-5 w-5" /> : <Eye className="h-5 w-5" />} </button>client/src/pages/public/RegisterPage.jsx-255-267 (1)
255-267:compactprop is not used by PasswordInput.The
compactprop is passed toPasswordInputbut the component doesn't accept or use this prop. Either remove the prop or updatePasswordInputto support compact styling.🔎 Proposed fix - remove unused prop
<PasswordInput id="password" name="password" value={formData.password} onChange={handleInputChange} showPassword={showPassword} setShowPassword={setShowPassword} required error={fieldErrors.password} placeholder="Min. 8 characters" - compact />client/src/pages/quicklab/LabAdminProfileComplete.jsx-220-228 (1)
220-228: Loading component may not render correctly inline.The
Loadingcomponent (from relevant snippets) is designed as a full-page loader withmin-h-screen. Using it inline within a button will likely cause layout issues. Consider using a simple spinner instead.🔎 Proposed fix
{loading ? ( <span className="flex items-center justify-center gap-2"> - <Loading /> + <svg className="animate-spin h-5 w-5" viewBox="0 0 24 24"> + <circle className="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="4" fill="none" /> + <path className="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4z" /> + </svg> Saving Profile... </span> ) : (client/src/pages/patient/PatientAppointments.jsx-311-311 (1)
311-311:replace('_', ' ')only replaces the first underscore.
String.replace()with a string argument only replaces the first occurrence. UsereplaceAll()or a regex with the global flag to replace all underscores (e.g.,sample_collected→ "sample collected").🔎 Proposed fix
- {labAppt.status.replace('_', ' ')} + {labAppt.status.replaceAll('_', ' ')}Apply the same fix at line 363.
client/src/pages/quicklab/LabAdminProfileComplete.jsx-28-28 (1)
28-28: Memory leak: Object URL is never revoked.
URL.createObjectURL()allocates memory that persists until explicitly revoked. Add cleanup to prevent memory leaks, especially if the user changes the image multiple times.🔎 Proposed fix
if (name === 'profilePicture' && files && files[0]) { + // Revoke previous URL to prevent memory leak + if (profilePreview) { + URL.revokeObjectURL(profilePreview); + } setFields((f) => ({ ...f, profilePicture: files[0], })); setProfilePreview(URL.createObjectURL(files[0]));Also add cleanup on unmount:
useEffect(() => { return () => { if (profilePreview) { URL.revokeObjectURL(profilePreview); } }; }, [profilePreview]);Committable suggestion skipped: line range outside the PR's diff.
client/src/service/geolocationService.js-1-2 (1)
1-2: Remove unused import.
apiServiceis imported but never used in this file.🔎 Proposed fix
// Geolocation service to detect user's city -import apiService from './apiservice';client/src/service/geolocationService.js-36-47 (1)
36-47: Fix typo and add required User-Agent header for Nominatim API.
- Function name
reversGeocodehas a typo (should bereverseGeocode)- Nominatim's usage policy requires a valid User-Agent header identifying your application
🔎 Proposed fix
-const reversGeocode = async (lat, lon) => { +const reverseGeocode = async (lat, lon) => { try { const response = await fetch( - `https://nominatim.openstreetmap.org/reverse?format=json&lat=${lat}&lon=${lon}&addressdetails=1` + `https://nominatim.openstreetmap.org/reverse?format=json&lat=${lat}&lon=${lon}&addressdetails=1`, + { + headers: { + 'User-Agent': 'QuickLab/1.0 (contact@example.com)', + }, + } );Also update line 19:
- const city = await reversGeocode(latitude, longitude); + const city = await reverseGeocode(latitude, longitude);client/src/pages/quicklab/LabSearchResults.jsx-235-235 (1)
235-235: Replace deprecatedonKeyPresswithonKeyDown.
onKeyPressis deprecated. UseonKeyDowninstead for keyboard event handling.🔎 Proposed fix
onChange={(e) => setNewCity(e.target.value)} - onKeyPress={(e) => e.key === 'Enter' && handleSaveCity()} + onKeyDown={(e) => e.key === 'Enter' && handleSaveCity()} placeholder="e.g., Mumbai, Delhi, Bangalore"client/src/pages/quicklab/LabAdminAppointments.jsx-147-152 (1)
147-152: Remove debug console.log statements.Debug logging for home collection appointments should be removed before merging to production.
🔎 Proposed fix
- // Debug: Log first appointment with home collection - const homeCollectionApt = filtered.find((a) => a.collectionType === 'home_collection'); - if (homeCollectionApt) { - console.log('Home Collection Appointment:', homeCollectionApt); - console.log('Collection Address:', homeCollectionApt.collectionAddress); - } - setFilteredAppointments(filtered);client/src/pages/quicklab/LabAdminManageTests.jsx-93-130 (1)
93-130:testLabelfield missing in reset for new test creation.The
formDatastate includestestLabel(line 29), and editing mode correctly sets it (implicitly via spread), but the reset for new test creation (lines 113-127) doesn't includetestLabel. This could cause stale data if a user edits a test with a label, then creates a new test.🔎 Add testLabel to form reset
} else { setEditingTest(null); setFormData({ testName: '', + testLabel: '', testCode: '', description: '', category: '',client/src/pages/quicklab/LabAdminManageTests.jsx-318-322 (1)
318-322: Price display logic appears inverted for discounted price.The strikethrough is applied to
discountedPricewhen it's less thanprice, but typically the original (higher) price should be struck through, not the discounted one. The current logic shows the discounted price with a strikethrough, which is counterintuitive.🔎 Fix price display logic
<span className="text-xl font-bold text-lab-black-900 dark:text-lab-black-50"> - ₹{test.price} + ₹{test.discountedPrice && test.discountedPrice < test.price ? test.discountedPrice : test.price} </span> {test.discountedPrice && test.discountedPrice < test.price && ( <span className="text-sm text-lab-black-500 dark:text-lab-black-400 line-through"> - ₹{test.discountedPrice} + ₹{test.price} </span> )}Committable suggestion skipped: line range outside the PR's diff.
🧹 Nitpick comments (41)
client/src/components/Patient/PatientMobileNavigation.jsx (1)
21-21: Consider adding outside-click handling for the Explore dropdown.The isExploreOpen state is introduced to manage the Explore dropdown visibility. However, unlike the Navbar.jsx implementation in this PR (which includes an exploreRef and outside-click detection), this component lacks a mechanism to close the dropdown when clicking outside of it. This may lead to inconsistent UX where the dropdown remains open until the user explicitly toggles it again.
🔎 Proposed fix to add outside-click handling
+import { useState, useEffect, useRef } from 'react'; -import { useState } from 'react'; // ... other imports const PatientMobileNavigation = () => { const navigate = useNavigate(); const location = useLocation(); const [isMenuOpen, setIsMenuOpen] = useState(false); const [isExploreOpen, setIsExploreOpen] = useState(false); + const exploreRef = useRef(null); const [searchQuery, setSearchQuery] = useState(''); const { logout } = useAuth(); + useEffect(() => { + const handleClickOutside = (event) => { + if (exploreRef.current && !exploreRef.current.contains(event.target)) { + setIsExploreOpen(false); + } + }; + document.addEventListener('mousedown', handleClickOutside); + return () => document.removeEventListener('mousedown', handleClickOutside); + }, []); // ... rest of componentThen wrap the Explore dropdown in a div with the ref:
- {/* Explore toggle */} + {/* Explore toggle */} + <div ref={exploreRef} className="flex flex-col items-center justify-center flex-1 h-full"> <button onClick={() => setIsExploreOpen((v) => !v)} - className="flex flex-col items-center justify-center flex-1 h-full text-gray-600" + className="w-full h-full flex flex-col items-center justify-center text-gray-600" > <span className="text-xs mt-1">Explore</span> </button> {isExploreOpen && ( <div className="absolute bottom-16 right-4 bg-white border border-gray-200 rounded-lg shadow-lg py-2 w-40"> {/* ... dropdown content ... */} </div> )} + </div> - </button> - {isExploreOpen && ( - <div className="absolute bottom-16 right-4 bg-white border border-gray-200 rounded-lg shadow-lg py-2 w-40"> - {/* ... dropdown content ... */} - </div> - )}client/src/service/labAppointmentService.js (1)
3-11: Consider adding input validation for API parameters.While the API wrapper functions are correctly structured, they lack defensive checks for required parameters like
appointmentId. If these functions are called withnullorundefined, the resulting API paths would be malformed (e.g.,/lab-appointment/undefined/assign-staff).🔎 Proposed enhancement with parameter validation
export const assignStaffForCollection = (appointmentId, staffId) => { + if (!appointmentId || !staffId) { + throw new Error('appointmentId and staffId are required'); + } apiService.put(`/lab-appointment/${appointmentId}/assign-staff`, { staffId }); }; export const updateLabAppointmentStatus = (appointmentId, status) => { + if (!appointmentId || !status) { + throw new Error('appointmentId and status are required'); + } apiService.put(`/lab-appointment/${appointmentId}/status`, { status }); };client/src/components/public/DesktopNavbar.jsx (1)
53-83: Extract the Explore dropdown to a reusable component.This exact Explore dropdown pattern (state management, outside-click handling, and UI) is duplicated in
client/src/components/Patient/PatientDesktopNavbar.jsx(lines 16-24 and 88-118). Consider extracting it to a shared component like<ExploreDropdown />to reduce duplication and improve maintainability.🔎 Example structure for a reusable ExploreDropdown component
Create
client/src/components/common/ExploreDropdown.jsx:import { useState, useRef, useEffect } from 'react'; import { useNavigate } from 'react-router-dom'; export default function ExploreDropdown() { const navigate = useNavigate(); const [isOpen, setIsOpen] = useState(false); const ref = useRef(null); useEffect(() => { const onDocClick = (e) => { if (ref.current && !ref.current.contains(e.target)) { setIsOpen(false); } }; document.addEventListener('mousedown', onDocClick); return () => document.removeEventListener('mousedown', onDocClick); }, []); return ( <div className="relative" ref={ref}> <button onClick={() => setIsOpen((v) => !v)} className="flex items-center text-gray-700 hover:text-blue-600 transition-colors px-3 py-2 rounded-md hover:bg-gray-50" > <span className="whitespace-nowrap">Explore</span> </button> {isOpen && ( <div className="absolute right-0 mt-2 w-40 bg-white border border-gray-200 rounded-lg shadow-lg py-2"> <button onClick={() => { setIsOpen(false); navigate('/quick-med'); }} className="block w-full text-left px-4 py-2 text-gray-700 hover:bg-gray-50" > Quick Med </button> <button onClick={() => { setIsOpen(false); navigate('/quick-lab'); }} className="block w-full text-left px-4 py-2 text-gray-700 hover:bg-gray-50" > Quick Lab </button> </div> )} </div> ); }Then replace lines 53-83 with
<ExploreDropdown />in both navbar files.server/Controllers/QuickLab/labAppointmentController.js (2)
268-276: Redundant dynamic import of LabStaff; it's already statically imported at line 6.
LabStaffis statically imported at line 6, but lines 275 and 347 dynamically re-import it, shadowing the module-level binding. Use the existing static import for consistency, or remove the static import if lazy loading is intentional.🔎 Proposed fix
if (role === 'lab_admin') { const LabAdmin = (await import('../../models/Lab/LabAdmin.js')).default; const labAdmin = await LabAdmin.findById(profileId); if (!labAdmin || !labAdmin.labId) { return res.status(400).json({ message: 'Lab admin not associated with any lab' }); } labId = labAdmin.labId; } else if (role === 'lab_staff') { - const LabStaff = (await import('../../models/Lab/LabStaff.js')).default; const staff = await LabStaff.findById(profileId); if (!staff) { return res.status(404).json({ message: 'Staff not found' }); } labId = staff.labId; }
347-348: Same issue: redundant dynamic import of LabStaff.Remove the dynamic import at line 347 and use the static import from line 6.
🔎 Proposed fix
- const LabStaff = (await import('../../models/Lab/LabStaff.js')).default; const staff = await LabStaff.findById(staffId);client/src/components/public/MobileBottomBar.jsx (2)
34-39: Missing icon for the Explore button.All other navigation items have icons (FiMapPin, FiUser, FiSearch, FiLogIn), but the Explore button only shows text. Consider adding an icon for visual consistency.
🔎 Proposed fix
+import { FiLogIn, FiMapPin, FiSearch, FiUser, FiCompass } from 'react-icons/fi'; ... <button onClick={() => setIsExploreOpen((v) => !v)} className="flex flex-col items-center text-gray-600 p-2" > + <FiCompass size={20} /> <span className="text-xs mt-1">Explore</span> </button>
40-61: Dropdown doesn't close when clicking outside.Users commonly expect dropdowns to close when clicking elsewhere. Consider adding a click-outside handler or an overlay.
🔎 Proposed fix using an overlay
{isExploreOpen && ( + <> + <div + className="fixed inset-0 z-40" + onClick={() => setIsExploreOpen(false)} + /> - <div className="absolute bottom-16 right-4 bg-white border border-gray-200 rounded-lg shadow-lg py-2 w-40"> + <div className="absolute bottom-16 right-4 bg-white border border-gray-200 rounded-lg shadow-lg py-2 w-40 z-50"> <button onClick={() => { setIsExploreOpen(false); navigate('/quick-med'); }} className="block w-full text-left px-4 py-2 text-gray-700 hover:bg-gray-50" > Quick Med </button> <button onClick={() => { setIsExploreOpen(false); navigate('/quick-lab'); }} className="block w-full text-left px-4 py-2 text-gray-700 hover:bg-gray-50" > Quick Lab </button> </div> + </> )}client/src/pages/quicklab/LabStaffWaitingForAssignment.jsx (1)
8-8: Unused import:userfrom useAuth is never used.Remove the unused destructured variable.
- const { user } = useAuth(); + useAuth(); // Or remove entirely if not neededclient/src/pages/quicklab/LabStaffProfileComplete.jsx (2)
252-257:onKeyPressis deprecated; useonKeyDowninstead.
onKeyPressis deprecated and may not fire for all keys in some browsers. Replace withonKeyDown.- onKeyPress={(e) => { + onKeyDown={(e) => { if (e.key === 'Enter') { e.preventDefault(); handleAddQualification(); } }}
72-74: Error message extraction may miss axios error details.Axios errors typically have the message in
error.response?.data?.message. The current fallback toerror.messagemay show generic network errors instead of server-provided messages.} catch (error) { console.error('Error creating staff profile:', error); - toast.error(error.message || 'Failed to create profile'); + toast.error(error.response?.data?.message || error.message || 'Failed to create profile'); } finally {server/Controllers/QuickLab/labStaffController.js (2)
249-275: Status progression logic is correct but cancellation checks are unreachable.The validation at lines 268-275 checks if
status === 'cancelled', butcancelledis not invalidStatuses(line 231), so the request would already be rejected at line 232. This dead code can be removed or the validation restructured if cancellation should be allowed for staff.
367-388: No transaction: LabReport creation and appointment update can become inconsistent.If
labReport.save()succeeds butassignment.save()fails, you'll have an orphaned report. Consider using a MongoDB transaction for atomicity.🔎 Proposed fix using transactions
+ const session = await mongoose.startSession(); + session.startTransaction(); + + try { // Create lab report const labReport = new LabReport({ appointmentId: assignment._id, patientId: assignment.patientId._id, labId: assignment.labId._id, reportFile: { url: uploadResult.url, fileName: req.file.originalname, uploadedAt: new Date(), }, reportDate: new Date(), }); - await labReport.save(); + await labReport.save({ session }); // Update appointment with report and mark as completed assignment.reportId = labReport._id; assignment.status = 'completed'; if (notes) { assignment.notes = notes; } - await assignment.save(); + await assignment.save({ session }); + + await session.commitTransaction(); + } catch (err) { + await session.abortTransaction(); + throw err; + } finally { + session.endSession(); + }client/src/components/quicklab/MobileNavbar.jsx (4)
24-24: State type mismatch: initialized as array but used as object.
suggestionsis initialized as an empty array[]but later assigned an object{ labs, tests }on line 82-85. This works in JavaScript but is confusing and could cause issues if code checkssuggestions.length.🔎 Proposed fix
- const [suggestions, setSuggestions] = useState([]); + const [suggestions, setSuggestions] = useState({ labs: [], tests: [] });
216-221: Loading indicator only visible when suggestions already exist.The loading state is rendered inside the suggestions dropdown, which only shows when
suggestions.labs?.length > 0 || suggestions.tests?.length > 0. This means the "Searching..." indicator won't appear during the initial search when no suggestions exist yet.🔎 Proposed fix
- {showSuggestions && (suggestions.labs?.length > 0 || suggestions.tests?.length > 0) && ( + {showSuggestions && (loading || suggestions.labs?.length > 0 || suggestions.tests?.length > 0) && ( <div className="absolute top-full left-0 right-0 mt-2 bg-white dark:bg-slate-900 border border-slate-200 dark:border-slate-700 rounded-lg shadow-2xl max-h-80 overflow-y-auto z-50"> + {/* Loading State */} + {loading && ( + <div className="p-4 text-center text-sm text-slate-500 dark:text-slate-400"> + Searching... + </div> + )} + {/* Labs Section */} - {suggestions.labs?.length > 0 && ( + {!loading && suggestions.labs?.length > 0 && (And remove the duplicate loading block at lines 216-221.
251-284: Explore popover lacks outside-click handling.Unlike the search suggestions dropdown which has outside-click handling (lines 37-45), the Explore popover remains open until explicitly closed. Users may expect it to close when clicking elsewhere.
287-328: Consider consolidating repeated lab_admin conditionals.Four separate
{isAuthenticated && user?.role === 'lab_admin' && (...)}blocks could be combined into one conditional block rendering all four buttons, reducing repetition.server/Controllers/QuickLab/labAdminController.js (1)
58-80: Duplicated field reconstruction logic between createLab and updateLabInfo.The contact and address object reconstruction is nearly identical in both functions. Consider extracting a helper function.
Also applies to: 390-411
client/src/pages/quicklab/LabStaffDashboard.jsx (2)
1-7: Unused imports and variables.
useEffectis imported but never used.useris destructured fromuseAuth()but never referenced in the component.🔎 Proposed fix
-import { useEffect, useState } from 'react'; -import { useAuth } from '../../context/authContext'; +import { useState } from 'react'; import '../../quicklab.css'; import { CheckSquare, Clock, AlertCircle, Zap, Home, Phone, MoreVertical } from 'lucide-react'; export default function LabStaffDashboard() { - const { user } = useAuth(); const [staffInfo] = useState({
8-54: Dashboard uses hardcoded demo data instead of API calls.The staffInfo, stats, upcomingAssignments, and quickActions are all static. Unlike
LabAdminDashboardwhich fetches real data, this component has no backend integration. The quick actions usealert()callbacks.Is this intentional as a placeholder for future implementation? The
labStaffService.jsprovidesgetMyAssignmentsandgetStaffProfilefunctions that could populate real data here. Would you like me to generate an implementation that integrates with those services?client/src/service/labStaffService.js (1)
4-12: Simplify unused file options in createFormDataFromObject call.Both
createStaffProfile()andupdateStaffProfile()pass{ photos: [], logo: null }tocreateFormDataFromObject(), which are unnecessary. This appears copied fromlabAdminServicewherelogoandphotosare actual file objects. In staff functions, onlyprofilePictureis used and appended separately. Either pass an empty object{}or omit the second argument to use the default.client/src/components/auth/PasswordInput.jsx (1)
11-11: Unusedlabelprop.The
labelprop is destructured from props but is never used in the component since the label rendering was removed. Consider removing it from the props interface to avoid confusion.🔎 Proposed fix
export const PasswordInput = ({ id, name, value, onChange, placeholder = 'Enter password', showPassword, setShowPassword, - label, required = false, error = null, }) => {client/src/pages/public/RegisterPage.jsx (1)
172-206: Role selection grid may be cramped on medium screens.A 5-column grid (
grid-cols-5) displaying all role options may be too narrow for readable labels on medium-width screens. Consider using a responsive approach or reducing columns for better UX.🔎 Suggested responsive approach
- <div className="grid grid-cols-5 gap-2"> + <div className="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-5 gap-2">client/src/pages/quicklab/LabAdminManageStaff.jsx (2)
23-27: Consider adjusting staffId validation length.MongoDB ObjectIds are 24 characters. The current threshold of 8 characters is quite permissive and may allow many invalid searches. Consider increasing it or validating the format more strictly.
🔎 Suggested adjustment
const canSearch = useMemo(() => { return ( - (query.email && query.email.includes('@')) || (!!query.staffId && query.staffId.length >= 8) + (query.email && query.email.includes('@')) || (!!query.staffId && query.staffId.length === 24) ); }, [query]);
286-292: Consider adding confirmation before removing staff.Removing staff is a destructive action performed with a single click. Consider adding a confirmation dialog to prevent accidental removals.
client/src/pages/patient/PatientAppointments.jsx (1)
147-153: Loading state only checks doctor appointments.The initial loading check only considers
loading(doctor appointments). Users will see the page shell while lab appointments are still loading, which may be confusing. Consider either showing a unified loading state or clearly indicating lab data is still loading in the left panel.🔎 Option: Combined loading state
- if (loading) { + if (loading && labLoading) { return <Loading />; }Or show a partial loading indicator in the lab section count while
labLoadingis true.client/src/pages/doctor/AppointmentDetails.jsx (1)
53-65: Consider extractinguseDebouncehook outside the component.Defining
useDebounceinside the component body means it's recreated on every render. While theuseCallbackinside helps, this pattern is unconventional and could be confusing. Consider moving it to a separate hooks file for reusability and clarity.client/src/pages/quicklab/LabStaffMyAppointments.jsx (2)
112-121: PDF validation relies solely on file extension.The validation only checks if the filename ends with
🔎 Proposed enhancement
const handleUploadReport = async (appointmentId) => { if (!reportFile) { toast.error('Please select a PDF file'); return; } - if (!reportFile.name.toLowerCase().endsWith('.pdf')) { + if (!reportFile.name.toLowerCase().endsWith('.pdf') || reportFile.type !== 'application/pdf') { toast.error('Only PDF files are allowed'); return; }
38-60: Consider extracting shared status configuration to a common module.The
statusColorsandstatusLabelsobjects are duplicated acrossLabStaffMyAppointments,PatientLabAppointments, andLabAdminAppointments. Extract these to a shared constants file to ensure consistency and reduce maintenance burden.client/src/service/geolocationService.js (1)
67-98: Add try-catch for localStorage access.
localStorageoperations can throw exceptions in private browsing mode or when storage is full. Wrap the access in try-catch to prevent runtime errors.🔎 Proposed fix
export const detectUserCity = async () => { try { // Try localStorage first for cached city - const cachedCity = localStorage.getItem('userCity'); - if (cachedCity) { - return cachedCity; + try { + const cachedCity = localStorage.getItem('userCity'); + if (cachedCity) { + return cachedCity; + } + } catch (e) { + // localStorage not available } // Try browser geolocation try { const city = await getCityFromGeolocation(); if (city) { - localStorage.setItem('userCity', city); + try { localStorage.setItem('userCity', city); } catch (e) { /* ignore */ } return city; }client/src/pages/quicklab/LabAdminAppointments.jsx (1)
79-86: Potential race condition between filter effects.The first
useEffect(lines 79-82) fetches appointments when filters change, while the second (lines 84-86) applies client-side search when appointments or searchQuery changes. If filters and searchQuery change simultaneously,applyFiltersmight run beforefetchAppointmentscompletes, operating on stale data.Consider combining the logic or ensuring
applyFiltersis called afterfetchAppointmentscompletes.🔎 Suggested approach
Call
applyFilters()at the end offetchAppointmentsafter setting appointments, rather than relying on the separate effect:const fetchAppointments = async () => { try { setLoading(true); // ... fetch logic ... setAppointments(data.appointments || []); calculateSummary(data.appointments || []); + // Apply search filter to new data + applyFiltersToData(data.appointments || []); } catch (error) { // ... } };client/src/pages/quicklab/LabSearchResults.jsx (2)
38-50: MemoizeavailableTestCategoriesto avoid recalculation on every render.This function is called during render and iterates through all labs and their tests. Consider using
useMemoto cache the result.🔎 Proposed fix
+import { useEffect, useState, useMemo } from 'react'; -import { useEffect, useState } from 'react'; - // Extract unique test categories from current lab results - const availableTestCategories = () => { + // Extract unique test categories from current lab results (memoized) + const availableTestCategories = useMemo(() => { const categoriesSet = new Set(); labs.forEach((lab) => { if (lab.tests && Array.isArray(lab.tests)) { lab.tests.forEach((test) => { if (test.category) { categoriesSet.add(test.category); } }); } }); return Array.from(categoriesSet).sort(); - }; + }, [labs]); // Update usage from availableTestCategories() to availableTestCategories
56-60: Effect may trigger unnecessary API calls on page changes.The effect depends on
filters,searchQuery, andpagination.page, butperformSearchis only called whenfilters.cityis truthy. However, changing pagination without a city set won't trigger a search but will still run the effect. Consider restructuring to be more explicit.client/src/components/quicklab/DesktopNavbar.jsx (2)
44-97: Debounce cleanup and effect dependency are correct.The debounce implementation properly cleans up timeouts on both re-render and unmount. The effect correctly depends on
searchQuery.One minor observation: the
suggestionsstate is initialized as an empty array ([]) on line 22 but later set as an object{ labs: [], tests: [] }. This type inconsistency could cause issues if code accessessuggestions.labsbefore the first search.🔎 Initialize suggestions with consistent shape
- const [suggestions, setSuggestions] = useState([]); + const [suggestions, setSuggestions] = useState({ labs: [], tests: [] });
280-326: Consider consolidating repeated role checks for lab_admin.Four separate conditional blocks check
isAuthenticated && user?.role === 'lab_admin'. This repetition can be consolidated for better maintainability.🔎 Consolidate lab_admin navigation items
- {/* Lab Admin: Manage Appointments */} - {isAuthenticated && user?.role === 'lab_admin' && ( - <button - onClick={() => navigate('/quick-lab/appointments')} - ... - </button> - )} - - {/* Lab Admin: Manage Staff shortcut */} - {isAuthenticated && user?.role === 'lab_admin' && ( - ... - )} - ... + {/* Lab Admin Navigation */} + {isAuthenticated && user?.role === 'lab_admin' && ( + <> + <button onClick={() => navigate('/quick-lab/appointments')} ...> + <Calendar className="h-5 w-5" /> Appointments + </button> + <button onClick={() => navigate('/quick-lab/staff')} ...> + <Users className="h-5 w-5" /> Staff + </button> + <button onClick={() => navigate('/quick-lab/tests')} ...> + <TestTube className="h-5 w-5" /> Tests + </button> + <button onClick={() => navigate('/quick-lab/lab-settings')} ...> + <Settings className="h-5 w-5" /> Settings + </button> + </> + )}client/src/pages/quicklab/LabAdminLabSettings.jsx (2)
88-137: Form submission bypasses native form validation.The "Save Changes" button (line 170-177) has
onClick={handleSubmit}but is outside the<form>element (which starts at line 216). This means the button doesn't trigger native form validation. Additionally, the button inside the form area would be more appropriate.However, since validation is handled manually in
handleSubmit, this works but could be confusing. Consider moving the button inside the form or usingform.requestSubmit().
139-142: Date formatting usestoLocaleString()without locale specification.The output will vary based on user's browser locale, which may lead to inconsistent display across users. Consider specifying locale options for consistent formatting.
🔎 Use consistent date formatting
const formatDate = (value) => { if (!value) return '—'; - return new Date(value).toLocaleString(); + return new Date(value).toLocaleString('en-IN', { + dateStyle: 'medium', + timeStyle: 'short', + }); };client/src/pages/quicklab/LabAdminManageTests.jsx (1)
59-91: Consider usinguseMemoinstead ofuseEffectfor filtering.The current approach uses
useEffectto computefilteredTestsand store it in state. This pattern is less efficient than usinguseMemo, which would avoid the extra render cycle and state update. The related fileLabAdminManageStaff.jsx(line 28-37) usesuseMemofor similar filtering.🔎 Refactor to use useMemo for filtering
- const [filteredTests, setFilteredTests] = useState([]); + // Remove filteredTests state - useEffect(() => { - applyFilters(); - }, [tests, searchQuery]); - const applyFilters = () => { - let filtered = [...tests]; - - if (searchQuery.trim()) { - const query = searchQuery.toLowerCase(); - filtered = filtered.filter( - (test) => - test.testName?.toLowerCase().includes(query) || - test.testCode?.toLowerCase().includes(query) || - test.category?.toLowerCase().includes(query) - ); - } - - setFilteredTests(filtered); - }; + const filteredTests = useMemo(() => { + if (!searchQuery.trim()) return tests; + const query = searchQuery.toLowerCase(); + return tests.filter( + (test) => + test.testName?.toLowerCase().includes(query) || + test.testCode?.toLowerCase().includes(query) || + test.category?.toLowerCase().includes(query) + ); + }, [tests, searchQuery]);Don't forget to add
useMemoto the import from React.client/src/service/labAdminService.js (1)
33-42: Redundantasync/awaitin simple pass-through functions.
checkLabExistsandcheckLabAdminProfileExistsuseasync/awaitunnecessarily since they just return the promise fromapiService.get(). Other functions in this file correctly return the promise directly.🔎 Remove unnecessary async/await
-export const checkLabExists = async () => { - return await apiService.get('/lab-admin/lab/status'); -}; +export const checkLabExists = () => apiService.get('/lab-admin/lab/status'); -export const checkLabAdminProfileExists = async () => { - return await apiService.get('/lab-admin/profile/status'); -}; +export const checkLabAdminProfileExists = () => apiService.get('/lab-admin/profile/status');client/src/pages/quicklab/LabDetails.jsx (3)
48-53: useEffect may cause unnecessary API calls on auth state changes.The effect depends on
[labId, isAuthenticated, user], butfetchLabDetails()only needslabId. When authentication state changes (login/logout),fetchLabDetailswill be called again unnecessarily. Consider splitting effects.🔎 Split effects to prevent unnecessary fetches
useEffect(() => { fetchLabDetails(); - if (isAuthenticated && user?.role === 'patient') { - fetchRecommendedTests(); - } -}, [labId, isAuthenticated, user]); +}, [labId]); +useEffect(() => { + if (isAuthenticated && user?.role === 'patient') { + fetchRecommendedTests(); + } +}, [isAuthenticated, user]);
127-172: Missing address validation for home collection booking.When
collectionTypeis'home_collection', the address fields (street, city, state, zipCode) are marked as required in the UI but not validated insubmitBooking. If the user submits with empty required fields, the HTML5 validation will catch it, but explicit validation would be more robust.🔎 Add address validation for home collection
const submitBooking = async (e) => { e.preventDefault(); if (!bookingData.appointmentDate || !bookingData.appointmentTime) { toast.error('Please select date and time'); return; } + if (bookingData.collectionType === 'home_collection') { + const { street, city, state, zipCode } = bookingData.collectionAddress; + if (!street || !city || !state || !zipCode) { + toast.error('Please fill in all address fields'); + return; + } + } try {
166-171: Error message extraction could be improved.The catch block uses
err.message, but axios errors typically have the message inerr.response?.data?.message. This may result in generic error messages being shown to users.🔎 Improve error message extraction
} catch (err) { console.error('Booking failed:', err); - toast.error(err.message || 'Failed to book appointment'); + toast.error(err.response?.data?.message || err.message || 'Failed to book appointment'); } finally {
| {/* Explore Dropdown (patients & public) */} | ||
| {(!isAuthenticated || user?.role === 'patient') && ( | ||
| <div className="relative" ref={exploreRef}> | ||
| <button | ||
| onClick={() => setIsExploreOpen((v) => !v)} | ||
| className="px-3 py-2 text-[var(--color-med-text)] hover:text-[var(--color-med-green-600)] font-medium" | ||
| > | ||
| Explore | ||
| </button> | ||
| {isExploreOpen && ( | ||
| <div className="absolute right-0 mt-2 w-40 bg-[var(--color-med-surface)] border border-[var(--color-med-border)] rounded-lg shadow-lg py-2"> | ||
| <button | ||
| onClick={() => { | ||
| setIsExploreOpen(false); | ||
| navigate('/'); | ||
| }} | ||
| className="block w-full text-left px-4 py-2 text-[var(--color-med-text)] hover:bg-[var(--color-med-green-50)] dark:hover:bg-[var(--color-med-green-900)]" | ||
| > | ||
| Quick Clinic | ||
| </button> | ||
| <button | ||
| onClick={() => { | ||
| setIsExploreOpen(false); | ||
| navigate('/quick-lab'); | ||
| }} | ||
| className="block w-full text-left px-4 py-2 text-[var(--color-med-text)] hover:bg-[var(--color-med-green-50)] dark:hover:bg-[var(--color-med-green-900)]" | ||
| > | ||
| Quick Lab | ||
| </button> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that isAuthenticated and user are defined in this component
# Search for useAuth or similar auth hook usage in the Navbar component
rg -n "useAuth|isAuthenticated|user" client/src/components/quickmed/Navbar.jsx -A 2 -B 2Repository: genzz-dev/QuickClinic
Length of output: 354
🏁 Script executed:
#!/bin/bash
# Get the first 60 lines of Navbar.jsx to see imports and hook declarations
head -60 client/src/components/quickmed/Navbar.jsxRepository: genzz-dev/QuickClinic
Length of output: 2152
🏁 Script executed:
#!/bin/bash
# Search for all lines containing isAuthenticated or user variable across the entire file
rg -n "^import.*isAuthenticated|^import.*user|const.*isAuthenticated|const.*user|useAuth" client/src/components/quickmed/Navbar.jsxRepository: genzz-dev/QuickClinic
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check file size first
wc -l client/src/components/quickmed/Navbar.jsx
# Get the entire file
cat -n client/src/components/quickmed/Navbar.jsxRepository: genzz-dev/QuickClinic
Length of output: 12653
Add missing authentication imports to the Navbar component.
The conditional at line 126 uses isAuthenticated and user variables that are not imported, defined, or passed to the component. This will cause a ReferenceError at runtime. Import these from your authentication context or hook (e.g., useAuth, useAuthContext, etc.).
🤖 Prompt for AI Agents
In client/src/components/quickmed/Navbar.jsx around lines 125 to 157 the JSX
references isAuthenticated and user but those values are not defined in the
component scope; import or obtain them from your auth provider (for example
import useAuth or useAuthContext at the top of the file) and then inside the
Navbar component call the hook and destructure const { isAuthenticated, user } =
useAuth(); so the conditional has defined values (or alternatively accept them
as props and update the component signature accordingly).
| const profileData = { firstName, lastName, phoneNumber }; | ||
| await createLabAdminProfile(profileData); |
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.
Profile picture is collected but never uploaded.
The form collects profilePicture (lines 23-28) and shows a preview, but the submission only sends firstName, lastName, and phoneNumber. Either remove the profile picture UI or implement the upload logic.
🔎 Option 1: Remove unused profile picture UI
Remove lines 159-212 (profile picture section) and related state (lines 15, 17, 23-28, 202-205).
Option 2: Include profile picture in submission
Use FormData to upload the image:
const profileData = new FormData();
profileData.append('firstName', firstName);
profileData.append('lastName', lastName);
profileData.append('phoneNumber', phoneNumber);
if (fields.profilePicture) {
profileData.append('profilePicture', fields.profilePicture);
}
await createLabAdminProfile(profileData);Note: This requires backend changes to handle multipart form data.
🤖 Prompt for AI Agents
In client/src/pages/quicklab/LabAdminProfileComplete.jsx around lines 49-50, the
form collects a profilePicture (see state and preview code) but the submission
only builds profileData with firstName, lastName, and phoneNumber, so the image
never gets uploaded; either remove the profile picture state/UI or modify the
submit to send multipart data: build a FormData, append
firstName/lastName/phoneNumber and, if present, append profilePicture, then pass
that FormData to createLabAdminProfile and ensure the API call sets the correct
headers for multipart/form-data (backend must accept multipart).
| const getNextAllowedStatuses = (currentStatus) => { | ||
| const statusOrder = ['pending', 'confirmed', 'sample_collected', 'processing', 'completed']; | ||
| const currentIndex = statusOrder.indexOf(currentStatus); | ||
|
|
||
| if (currentStatus === 'cancelled' || currentStatus === 'completed') { | ||
| return []; | ||
| } | ||
|
|
||
| // Staff can move forward or stay in same status | ||
| return statusOrder.slice(currentIndex + 1, currentIndex + 3).filter((s) => s !== 'completed'); | ||
| }; |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check backend status validation logic
rg -n "validStatuses|statusOrder|status.*progression|updateAssignmentStatus|updateMyAssignmentStatus" server/Repository: genzz-dev/QuickClinic
Length of output: 1126
🏁 Script executed:
#!/bin/bash
# Check the file size and read the status update handler in LabStaffMyAppointments.jsx
wc -l client/src/pages/quicklab/LabStaffMyAppointments.jsx
echo "---"
# Find the status update handler that uses getNextAllowedStatuses
rg -n "getNextAllowedStatuses\|handleStatusUpdate\|updateStatus" client/src/pages/quicklab/LabStaffMyAppointments.jsx -A 5Repository: genzz-dev/QuickClinic
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Check the actual line context around 80-90 in the file
head -n 120 client/src/pages/quicklab/LabStaffMyAppointments.jsx | tail -n 60Repository: genzz-dev/QuickClinic
Length of output: 1883
🏁 Script executed:
#!/bin/bash
# Read the backend status update validation logic
sed -n '225,270p' server/Controllers/QuickLab/labStaffController.jsRepository: genzz-dev/QuickClinic
Length of output: 1768
🏁 Script executed:
#!/bin/bash
# Check the exact comparison in backend to see if equality is allowed
sed -n '250,260p' server/Controllers/QuickLab/labStaffController.js | grep -A 2 "newStatusIndex"Repository: genzz-dev/QuickClinic
Length of output: 269
🏁 Script executed:
#!/bin/bash
# Check if there's any test or documentation about intended status flow
rg -n "sequential|immediate|next.*status|skipping" --type=js server/ client/Repository: genzz-dev/QuickClinic
Length of output: 225
Backend and frontend both allow skipping intermediate statuses; enforce sequential progression if required.
The getNextAllowedStatuses function returns up to 2 statuses ahead, allowing transitions like pending → sample_collected (skipping confirmed). More critically, the backend validation at labStaffController.js:250-252 only prevents backward movement (newStatusIndex < currentStatusIndex) but does not enforce sequential progression. To guarantee sequential status flow, limit the frontend to the immediate next status only and add corresponding backend validation to enforce newStatusIndex === currentStatusIndex + 1.
Note: The frontend comment "stay in same status" is misleading—the code does not support same-status transitions.
🤖 Prompt for AI Agents
In client/src/pages/quicklab/LabStaffMyAppointments.jsx around lines 80 to 90,
the getNextAllowedStatuses function currently returns up to two future statuses
(allowing skips) and contains a misleading comment about "stay in same status";
change it to only return the immediate next status (or [] for
'cancelled'/'completed' or when none), remove/replace the misleading comment,
and ensure edge cases (no currentIndex found) return []; additionally update the
backend validation in labStaffController.js (around lines 250-252) to enforce
sequential progression by requiring newStatusIndex === currentStatusIndex + 1
(instead of only preventing backwards moves) and return a clear error when the
transition is not the immediate next status.
| useEffect(() => { | ||
| const checkStatus = async () => { | ||
| try { | ||
| const res = await checkStaffProfileExists(); | ||
| setStaffId(res.staffId); | ||
| setIsAssigned(res.isAssignedToLab); | ||
| setLoading(false); | ||
|
|
||
| // Poll every 5 seconds to check if assigned | ||
| if (!res.isAssignedToLab) { | ||
| const interval = setInterval(async () => { | ||
| const updated = await checkStaffProfileExists(); | ||
| if (updated.isAssignedToLab) { | ||
| setIsAssigned(true); | ||
| window.location.href = '/quick-lab/staff-dashboard'; | ||
| } | ||
| }, 5000); | ||
|
|
||
| return () => clearInterval(interval); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error checking staff status:', error); | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| checkStatus(); | ||
| }, []); |
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.
Critical: Interval cleanup is broken and will cause memory leaks.
The return () => clearInterval(interval) at line 32 is inside the async checkStatus function, so it never reaches the useEffect cleanup. The interval will keep running even after the component unmounts.
🔎 Proposed fix
useEffect(() => {
+ let intervalId = null;
+ let isMounted = true;
+
const checkStatus = async () => {
try {
const res = await checkStaffProfileExists();
+ if (!isMounted) return;
setStaffId(res.staffId);
setIsAssigned(res.isAssignedToLab);
setLoading(false);
- // Poll every 5 seconds to check if assigned
- if (!res.isAssignedToLab) {
- const interval = setInterval(async () => {
- const updated = await checkStaffProfileExists();
- if (updated.isAssignedToLab) {
- setIsAssigned(true);
- window.location.href = '/quick-lab/staff-dashboard';
- }
- }, 5000);
-
- return () => clearInterval(interval);
+ if (!res.isAssignedToLab && !intervalId) {
+ intervalId = setInterval(async () => {
+ try {
+ const updated = await checkStaffProfileExists();
+ if (!isMounted) return;
+ if (updated.isAssignedToLab) {
+ setIsAssigned(true);
+ // Use navigate instead of window.location.href
+ }
+ } catch (err) {
+ console.error('Polling error:', err);
+ }
+ }, 5000);
}
} catch (error) {
+ if (!isMounted) return;
console.error('Error checking staff status:', error);
setLoading(false);
}
};
checkStatus();
+
+ return () => {
+ isMounted = false;
+ if (intervalId) clearInterval(intervalId);
+ };
}, []);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In client/src/pages/quicklab/LabStaffWaitingForAssignment.jsx around lines 14 to
41, the interval cleanup is returned from inside the async checkStatus function
so the useEffect cleanup never runs and the interval leaks; fix by declaring the
interval ID in the outer useEffect scope, assign it when starting setInterval,
and move the cleanup into the useEffect return callback so
clearInterval(interval) runs on unmount; also clear the interval immediately
when updated.isAssignedToLab is true (before redirect) to avoid a race.
| const updated = await checkStaffProfileExists(); | ||
| if (updated.isAssignedToLab) { | ||
| setIsAssigned(true); | ||
| window.location.href = '/quick-lab/staff-dashboard'; |
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.
Avoid window.location.href; use React Router's navigate instead.
window.location.href causes a full page reload, losing React state and degrading UX. Since navigate is available via react-router-dom, use it for client-side navigation.
+import { useNavigate } from 'react-router-dom';
...
export default function LabStaffWaitingForAssignment() {
const { user } = useAuth();
+ const navigate = useNavigate();
...
- window.location.href = '/quick-lab/staff-dashboard';
+ navigate('/quick-lab/staff-dashboard', { replace: true });Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In client/src/pages/quicklab/LabStaffWaitingForAssignment.jsx around line 28,
replace the direct window.location.href assignment with React Router navigation:
import or obtain the navigate function from react-router-dom (useNavigate hook
or receive it via props/history depending on component type) and call
navigate('/quick-lab/staff-dashboard') instead of setting window.location.href;
ensure the useNavigate import is added and any event handlers are updated to use
navigate so navigation is client-side without a full page reload.
| const runChecks = async () => { | ||
| if (!isAuthenticated || (user?.role !== 'lab_admin' && user?.role !== 'lab_staff')) { | ||
| if (mounted) navigate('/', { replace: true }); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const [profileRes, labRes] = await Promise.all([ | ||
| requireProfile ? checkLabAdminProfileExists() : Promise.resolve({}), | ||
| requireLab && user?.role === 'lab_admin' ? checkLabExists() : Promise.resolve({}), | ||
| ]); | ||
|
|
||
| if (!mounted) return; | ||
|
|
||
| if (requireProfile && !profileRes.exists) { | ||
| toast.warning('Please complete your profile first.'); | ||
| navigate('/quick-lab/complete-profile', { replace: true }); | ||
| return; | ||
| } | ||
|
|
||
| if (requireLab && user?.role === 'lab_admin' && !labRes.exists) { | ||
| toast.warning('Please add your lab details first.'); | ||
| navigate('/quick-lab/add-lab', { replace: true }); | ||
| return; | ||
| } |
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.
Potential role/service mismatch for lab_staff users.
The guard allows lab_staff role (line 21) but calls checkLabAdminProfileExists (line 28) which checks for lab admin profiles. If a lab_staff user passes this guard, they'll be checking/redirecting based on lab admin profile status rather than their own staff profile.
Either:
- Remove
lab_stafffrom the role check if this guard is only for lab admins - Use role-specific profile checks if both roles should use this guard
🔎 Option 1: Restrict to lab_admin only
const runChecks = async () => {
- if (!isAuthenticated || (user?.role !== 'lab_admin' && user?.role !== 'lab_staff')) {
+ if (!isAuthenticated || user?.role !== 'lab_admin') {
if (mounted) navigate('/', { replace: true });
return;
}📝 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.
| const runChecks = async () => { | |
| if (!isAuthenticated || (user?.role !== 'lab_admin' && user?.role !== 'lab_staff')) { | |
| if (mounted) navigate('/', { replace: true }); | |
| return; | |
| } | |
| try { | |
| const [profileRes, labRes] = await Promise.all([ | |
| requireProfile ? checkLabAdminProfileExists() : Promise.resolve({}), | |
| requireLab && user?.role === 'lab_admin' ? checkLabExists() : Promise.resolve({}), | |
| ]); | |
| if (!mounted) return; | |
| if (requireProfile && !profileRes.exists) { | |
| toast.warning('Please complete your profile first.'); | |
| navigate('/quick-lab/complete-profile', { replace: true }); | |
| return; | |
| } | |
| if (requireLab && user?.role === 'lab_admin' && !labRes.exists) { | |
| toast.warning('Please add your lab details first.'); | |
| navigate('/quick-lab/add-lab', { replace: true }); | |
| return; | |
| } | |
| const runChecks = async () => { | |
| if (!isAuthenticated || user?.role !== 'lab_admin') { | |
| if (mounted) navigate('/', { replace: true }); | |
| return; | |
| } | |
| try { | |
| const [profileRes, labRes] = await Promise.all([ | |
| requireProfile ? checkLabAdminProfileExists() : Promise.resolve({}), | |
| requireLab && user?.role === 'lab_admin' ? checkLabExists() : Promise.resolve({}), | |
| ]); | |
| if (!mounted) return; | |
| if (requireProfile && !profileRes.exists) { | |
| toast.warning('Please complete your profile first.'); | |
| navigate('/quick-lab/complete-profile', { replace: true }); | |
| return; | |
| } | |
| if (requireLab && user?.role === 'lab_admin' && !labRes.exists) { | |
| toast.warning('Please add your lab details first.'); | |
| navigate('/quick-lab/add-lab', { replace: true }); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In client/src/routes/guards/LabAdminSetupGuard.jsx around lines 20 to 44, the
guard currently allows 'lab_staff' but always calls checkLabAdminProfileExists
and checkLabExists which mismatches staff users; either restrict the allowed
roles to only 'lab_admin' by removing 'lab_staff' from the initial role check,
or implement role-specific checks: when user.role === 'lab_admin' call
checkLabAdminProfileExists (and checkLabExists when requireLab) and redirect to
admin profile/lab routes, and when user.role === 'lab_staff' call the
staff-specific profile checker (e.g. checkLabStaffProfileExists) and use
staff-appropriate redirect paths; update the Promise.all construction to choose
the correct check functions based on role, and ensure subsequent conditional
checks and navigation use the corresponding profile existence fields and routes.
| const labs = await Lab.find({ | ||
| 'address.city': { $regex: city, $options: 'i' }, | ||
| isActive: { $ne: false }, // Exclude inactive labs if the field exists | ||
| }) | ||
| .select('name address contact tests generalHomeCollectionFee') | ||
| .lean(); |
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.
Sanitize regex input to prevent ReDoS attacks.
User-supplied city parameter is directly interpolated into a regex pattern. Malicious input with special regex characters could cause performance issues or unexpected matching behavior. Consider escaping special characters.
🔎 Proposed fix
+// Helper to escape regex special characters
+const escapeRegex = (str) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+
export const getNearbyLabs = async (req, res) => {
try {
const { city } = req.query;
if (!city) {
return res.status(400).json({ success: false, message: 'City parameter is required' });
}
// Find labs in the same city
const labs = await Lab.find({
- 'address.city': { $regex: city, $options: 'i' },
+ 'address.city': { $regex: escapeRegex(city), $options: 'i' },
isActive: { $ne: false },
})📝 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.
| const labs = await Lab.find({ | |
| 'address.city': { $regex: city, $options: 'i' }, | |
| isActive: { $ne: false }, // Exclude inactive labs if the field exists | |
| }) | |
| .select('name address contact tests generalHomeCollectionFee') | |
| .lean(); | |
| // Helper to escape regex special characters | |
| const escapeRegex = (str) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | |
| export const getNearbyLabs = async (req, res) => { | |
| try { | |
| const { city } = req.query; | |
| if (!city) { | |
| return res.status(400).json({ success: false, message: 'City parameter is required' }); | |
| } | |
| // Find labs in the same city | |
| const labs = await Lab.find({ | |
| 'address.city': { $regex: escapeRegex(city), $options: 'i' }, | |
| isActive: { $ne: false }, // Exclude inactive labs if the field exists | |
| }) | |
| .select('name address contact tests generalHomeCollectionFee') | |
| .lean(); |
🤖 Prompt for AI Agents
In server/Controllers/publicController.js around lines 509 to 514, the
user-supplied city string is used directly in a MongoDB regex which risks ReDoS
and unexpected matching; escape all regex-special characters from the city input
before building the regex (e.g., use a small escape function or a utility like
lodash.escapeRegExp), guard against null/undefined by defaulting to an empty
string or returning no results, then construct the regex from the escaped value
with the 'i' flag and use that in the query so only literal matching is
performed.
| if (query && query.trim()) { | ||
| searchQuery.$or = [ | ||
| { name: { $regex: query, $options: 'i' } }, | ||
| { description: { $regex: query, $options: 'i' } }, | ||
| { 'tests.testName': { $regex: query, $options: 'i' } }, | ||
| ]; |
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.
Apply regex escaping consistently across all search filters.
Similar to getNearbyLabs, the searchLabs function uses unescaped user input in regex patterns for query, city, and test name searches. Apply the same escaping helper here.
🔎 Proposed fix
// Search by lab name or test name
if (query && query.trim()) {
+ const escapedQuery = escapeRegex(query);
searchQuery.$or = [
- { name: { $regex: query, $options: 'i' } },
- { description: { $regex: query, $options: 'i' } },
- { 'tests.testName': { $regex: query, $options: 'i' } },
+ { name: { $regex: escapedQuery, $options: 'i' } },
+ { description: { $regex: escapedQuery, $options: 'i' } },
+ { 'tests.testName': { $regex: escapedQuery, $options: 'i' } },
];
}📝 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.
| if (query && query.trim()) { | |
| searchQuery.$or = [ | |
| { name: { $regex: query, $options: 'i' } }, | |
| { description: { $regex: query, $options: 'i' } }, | |
| { 'tests.testName': { $regex: query, $options: 'i' } }, | |
| ]; | |
| if (query && query.trim()) { | |
| const escapedQuery = escapeRegex(query); | |
| searchQuery.$or = [ | |
| { name: { $regex: escapedQuery, $options: 'i' } }, | |
| { description: { $regex: escapedQuery, $options: 'i' } }, | |
| { 'tests.testName': { $regex: escapedQuery, $options: 'i' } }, | |
| ]; |
🤖 Prompt for AI Agents
In server/Controllers/publicController.js around lines 554 to 559, the code
builds MongoDB $regex queries directly from user input (query, city, and
tests.testName) which is unsafe and inconsistent with getNearbyLabs; use the
same regex-escaping helper function (e.g., escapeRegex or escapeRegExp) before
constructing each $regex pattern. Concretely, validate existence/trim of each
input, call the escape helper on query, city, and test name values, then use the
escaped string in the { $regex: escapedValue, $options: 'i' } clauses so all
search filters consistently escape user input.
| const lab = await Lab.findById(labId) | ||
| .populate('staff', 'firstName lastName role phoneNumber') | ||
| .lean(); |
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.
🧩 Analysis chain
🏁 Script executed:
# Check how this endpoint is routed and if it's truly public
rg -n "getLabById|labs/:labId" server/Routes/Repository: genzz-dev/QuickClinic
Length of output: 184
🏁 Script executed:
# Find all route files to understand routing structure
find server/Routes -type f -name "*.js" | head -20Repository: genzz-dev/QuickClinic
Length of output: 637
🏁 Script executed:
# Check the actual route definition and any middleware
rg -n -B 5 -A 5 "getLabById" server/Routes/Repository: genzz-dev/QuickClinic
Length of output: 1116
🏁 Script executed:
# Look for authentication middleware on the route
rg -n "labs/:labId|/labs/" server/Routes/ -A 3Repository: genzz-dev/QuickClinic
Length of output: 376
🏁 Script executed:
# Find the main app file and check how publicRoutes is used
fd -e js server | xargs rg -l "publicRoutes" | head -5Repository: genzz-dev/QuickClinic
Length of output: 110
🏁 Script executed:
# Check the main app file (likely index.js, server.js, or app.js)
rg -n "publicRoutes|import.*publicRoutes" server/Repository: genzz-dev/QuickClinic
Length of output: 244
🏁 Script executed:
# Look at the full getLabById implementation to understand the data being returned
rg -n -A 30 "const getLabById|async.*getLabById|export.*getLabById" server/Controllers/publicController.jsRepository: genzz-dev/QuickClinic
Length of output: 678
🏁 Script executed:
# Check if there are other public endpoints that expose staff/sensitive data
rg -n "\.populate\(" server/Controllers/publicController.jsRepository: genzz-dev/QuickClinic
Length of output: 666
Exclude staff phoneNumber from public lab endpoint or require authentication.
The getLabById endpoint at /api/public/labs/:labId exposes staff phoneNumber along with firstName, lastName, and role. Staff phoneNumber is personally identifiable information that should not be publicly accessible. Either remove phoneNumber from the populate query (line 641) or add authentication to this endpoint.
🤖 Prompt for AI Agents
In server/Controllers/publicController.js around lines 640-642, the public
getLabById endpoint is populating staff.phoneNumber which exposes PII; either
remove phoneNumber from the populate call or gate the endpoint with
authentication. To fix: remove "phoneNumber" from the populate projection string
so staff is populated with only firstName, lastName, role; OR add the
appropriate authentication middleware to the route and conditionally include
phoneNumber only when req.user/authenticated (e.g., switch populate fields based
on auth and return phoneNumber only for authenticated requests).
| // Update appointment with report and mark as completed | ||
| assignment.reportId = labReport._id; | ||
| assignment.status = 'completed'; | ||
| if (notes) { | ||
| assignment.notes = notes; | ||
| } | ||
| await assignment.save(); |
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.
Missing staff assignment list update when completing via report upload.
updateMyAssignmentStatus (lines 284-288) moves the appointment from currentAssignments to completedAssignments when status becomes completed. The uploadReportAndComplete function also sets status to completed but doesn't perform this update, causing inconsistency.
🔎 Proposed fix
assignment.reportId = labReport._id;
assignment.status = 'completed';
if (notes) {
assignment.notes = notes;
}
await assignment.save();
+ // Move from current to completed assignments
+ await LabStaff.findByIdAndUpdate(profileId, {
+ $pull: { currentAssignments: appointmentId },
+ $push: { completedAssignments: appointmentId },
+ });
res.json({Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/Controllers/QuickLab/labStaffController.js around lines 382 to 388,
uploadReportAndComplete sets assignment.status = 'completed' but does not update
the staff member's assignment lists; replicate the same behavior as
updateMyAssignmentStatus by removing the assignment id from the staff's
currentAssignments and pushing it onto completedAssignments (or invoke
updateMyAssignmentStatus with the same parameters) and save the staff document
so the staff's assignment lists stay consistent when a report completes the
appointment.
Project
Change Type
Stack
Page Type
Route/API Endpoint Status
What Changed
Major Features Added
1. Patient Lab Appointments Experience
2. Cross-Navigation Between Modules
3. QuickLab Infrastructure (Backend)
4. Lab Admin Dashboard & Management
5. Lab Staff Pages & Flows
6. Appointment Management
7. Backend Fixes & Improvements
Files Modified
Frontend:
client/src/pages/patient/PatientAppointments.jsx- Added lab appointments section with filters and modalclient/src/components/public/DesktopNavbar.jsx- Added Explore dropdownclient/src/components/public/MobileBottomBar.jsx- Added Explore popoverclient/src/components/Patient/PatientDesktopNavbar.jsx- Added Explore dropdownclient/src/components/Patient/PatientMobileNavigation.jsx- Added Explore popoverclient/src/components/quicklab/DesktopNavbar.jsx- Added Explore dropdown with gatingclient/src/components/quicklab/MobileNavbar.jsx- Added Explore popover with gatingclient/src/components/quickmed/Navbar.jsx- Added Explore dropdown with gatingBackend:
Routes Added
Frontend:
/quick-lab/appointments- Lab admin appointments management/quick-lab/tests- Lab admin test management/quick-lab/settings- Lab settings page/quick-lab/staff- Lab staff management/quick-lab/search- Lab search results/quick-lab/{labId}- Lab details with test selectionBackend:
GET /api/labs/search- Search labs with filtersGET /api/labs/:id- Get lab detailsPOST /api/appointments/book-lab- Book lab appointmentGET /api/location/nearby-labs- Get nearby labs by cityCode Quality
npx prettier --write .)Related Issues
Part of broader QuickLab ecosystem integration
Summary by CodeRabbit
New Features
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.