Feature/quicklabs backend #107
Conversation
…y for easy managing
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request introduces a comprehensive lab management system for QuickLab with new controllers for managing lab administration, staff, appointments, and reports. Five new Mongoose models define Lab, LabAdmin, LabAppointment, LabReport, and LabStaff data structures. Five new route modules expose RESTful APIs for these operations. Authentication middleware is extended to support new user roles (lab_admin and lab_staff), and existing User, Patient, and Doctor models are updated with new fields and role enumerations. Changes
Sequence DiagramssequenceDiagram
actor Patient
participant App as Client
participant Server
participant LabDB as Lab Service
participant FileStore as Cloud Storage
participant ReportDB as Report Service
Patient->>App: Book appointment (lab, tests, address)
App->>Server: POST /api/lab-appointment/book
Server->>LabDB: Verify lab & tests
alt Lab/Tests Invalid
Server-->>App: 404 Not Found
else Valid
LabDB-->>Server: Lab & test details
Server->>Server: Calculate fees & validate<br/>home collection constraints
Server->>LabDB: Create LabAppointment
LabDB-->>Server: Appointment created
Server->>LabDB: Link to Patient
Server-->>App: Appointment confirmed
end
Note over Patient,App: Staff Assignment Phase
app->>Server: PUT /api/lab-appointment/:id/assign-staff
Server->>LabDB: Verify staff & appointment
alt Staff/Appt Invalid
Server-->>App: 404 Not Found
else Valid
LabDB-->>Server: Staff & appointment data
Server->>LabDB: Assign staff to appointment<br/>Update status to confirmed
Server-->>App: Assignment successful
end
Note over Patient,App: Report Upload Phase
Patient->>App: Upload lab report (file)
App->>Server: POST /api/lab-report/upload/:appointmentId
Server->>LabDB: Verify appointment exists
alt Appointment Invalid
Server-->>App: 404 Not Found
else Valid
LabDB-->>Server: Appointment details
App->>FileStore: Upload report file
FileStore-->>App: File stored
Server->>ReportDB: Create LabReport document<br/>(file URL, test results)
ReportDB-->>Server: Report created
Server->>LabDB: Mark appointment completed
Server-->>App: Report uploaded & processed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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-12T04:37:21.976Z |
📋 PR Auto-Check📝 Files Changed✨ Code QualityPrettier Check: ✅ Passed Auto-generated on 2025-12-12T04:39:05.398Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (8)
server/index.js (1)
53-53: Minor typo in comment.Comment says "QuickLabs Route" but should be "QuickLab Route" for consistency (singular form).
Apply this diff:
-//QuickLabs Route +//QuickLab Routeserver/models/Lab/LabReport.js (1)
47-55: Consider adding default to doctorRemarks.addedAt.While the
doctorRemarksstructure is correct, theaddedAtfield lacks a default value. Although this may be set programmatically in the controller (as seen in labReportController.js), adding a default would provide a safety net.Apply this diff if you'd like to add the default:
doctorRemarks: { remarks: String, - addedAt: Date, + addedAt: { type: Date, default: Date.now }, addedBy: { type: mongoose.Schema.Types.ObjectId, ref: 'Doctor', }, },server/models/Lab/Lab.js (1)
4-114: Consider adding indexes for search performance.The
searchLabsendpoint inlabController.jsqueries byname,address.city,tests.testName, andisActive. Without indexes, these queries will perform full collection scans as data grows.Add indexes after the schema definition:
+labSchema.index({ isActive: 1 }); +labSchema.index({ name: 'text' }); +labSchema.index({ 'address.city': 1 }); +labSchema.index({ 'tests.testName': 1 }); +// If geospatial queries are planned: +// labSchema.index({ 'address.coordinates': '2dsphere' }); export default mongoose.model('Lab', labSchema);server/Controllers/QuickLab/labController.js (1)
5-36: Add pagination to prevent unbounded result sets.Without pagination, this endpoint could return all matching labs, causing performance issues and large payloads as the dataset grows.
export const searchLabs = async (req, res) => { try { - const { name, city, testName } = req.query; + const { name, city, testName, page = 1, limit = 20 } = req.query; + const skip = (parseInt(page) - 1) * parseInt(limit); const query = { isActive: true }; // ... filters ... const labs = await Lab.find(query) .select( 'name description address contact logo ratings tests generalHomeCollectionFee' ) + .skip(skip) + .limit(parseInt(limit)) .lean(); + const total = await Lab.countDocuments(query); + res.json({ count: labs.length, + total, + page: parseInt(page), + totalPages: Math.ceil(total / parseInt(limit)), labs, });server/Controllers/QuickLab/labReportController.js (1)
1-2: Remove unused import.
mongooseis imported but never used in this file.// controllers/labReportController.js -import mongoose from 'mongoose'; import LabReport from '../../models/Lab/LabReport.js';server/Controllers/QuickLab/labAppointmentController.js (1)
7-7: Remove unused import.
LabReportis imported but never used in this file.-import LabReport from '../../models/Lab/LabReport.js';server/Controllers/QuickLab/labStaffController.js (2)
123-148:checkStaffProfileExists: validateuserIdand keep response minimalIf
userIdis not a valid ObjectId,findOne({ userId })can still behave unexpectedly (string vs ObjectId). Consider validatinguserId(400) for consistency. Also double-check that returninglabIdhere is acceptable for your threat model (it can enable lab enumeration ifreq.useris compromised).
277-306:completeAssignmentduplicates status update + naming/semantics mismatchThis endpoint sets
assignment.status = 'sample_collected', which overlaps withupdateMyAssignmentStatus(already allowssample_collected). Either:
- fold this into
updateMyAssignmentStatus(and drop the extra route), or- make
completeAssignmentdo additional, unique work (e.g., enforce valid transition, set a completion timestamp, move assignment between “current/completed” if that’s your model).Also add
appointmentIdvalidation (400) beforefindById.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
server/Controllers/QuickLab/labAdminController.js(1 hunks)server/Controllers/QuickLab/labAppointmentController.js(1 hunks)server/Controllers/QuickLab/labController.js(1 hunks)server/Controllers/QuickLab/labReportController.js(1 hunks)server/Controllers/QuickLab/labStaffController.js(1 hunks)server/Controllers/authController.js(1 hunks)server/Controllers/doctorController.js(1 hunks)server/Middleware/authMiddleware.js(2 hunks)server/Middleware/authmiddleware demo 2.txt(0 hunks)server/Middleware/authmiddlewaredemo.txt(0 hunks)server/Routes/QuickLab/labAdminRoutes.js(1 hunks)server/Routes/QuickLab/labAppointmentRoutes.js(1 hunks)server/Routes/QuickLab/labReportRoutes.js(1 hunks)server/Routes/QuickLab/labRoutes.js(1 hunks)server/Routes/QuickLab/labStaffRoutes.js(1 hunks)server/index.js(2 hunks)server/models/Lab/Lab.js(1 hunks)server/models/Lab/LabAdmin.js(1 hunks)server/models/Lab/LabAppointment.js(1 hunks)server/models/Lab/LabReport.js(1 hunks)server/models/Lab/LabStaff.js(1 hunks)server/models/Users/Doctor.js(1 hunks)server/models/Users/Patient.js(1 hunks)server/models/Users/User.js(1 hunks)
💤 Files with no reviewable changes (2)
- server/Middleware/authmiddlewaredemo.txt
- server/Middleware/authmiddleware demo 2.txt
🧰 Additional context used
🧬 Code graph analysis (8)
server/Routes/QuickLab/labRoutes.js (1)
server/Controllers/QuickLab/labController.js (6)
searchLabs(5-37)searchLabs(5-37)getLabDetails(40-55)getLabDetails(40-55)getLabTests(58-79)getLabTests(58-79)
server/Routes/QuickLab/labReportRoutes.js (2)
server/Middleware/upload.js (1)
upload(23-23)server/Controllers/QuickLab/labReportController.js (10)
uploadLabReport(8-53)uploadLabReport(8-53)getPatientLabReports(56-75)getPatientLabReports(56-75)getDoctorPatientReports(78-100)getDoctorPatientReports(78-100)addDoctorRemarks(103-138)addDoctorRemarks(103-138)getReportDetails(141-174)getReportDetails(141-174)
server/Routes/QuickLab/labAppointmentRoutes.js (1)
server/Controllers/QuickLab/labAppointmentController.js (10)
bookLabAppointment(10-162)bookLabAppointment(10-162)getPatientLabAppointments(231-257)getPatientLabAppointments(231-257)getLabAppointments(260-323)getLabAppointments(260-323)assignStaffForCollection(326-374)assignStaffForCollection(326-374)updateLabAppointmentStatus(377-415)updateLabAppointmentStatus(377-415)
server/Middleware/authMiddleware.js (3)
server/Controllers/QuickLab/labAppointmentController.js (1)
LabAdmin(268-268)server/Controllers/authController.js (1)
user(25-25)server/services/authService.js (4)
user(12-12)user(19-19)user(54-54)user(78-78)
server/Controllers/QuickLab/labReportController.js (2)
server/Controllers/QuickLab/labAdminController.js (14)
req(12-12)req(13-13)req(55-55)req(112-112)req(172-172)req(173-173)req(217-217)req(218-218)req(254-254)req(276-276)req(277-277)req(308-308)req(332-332)req(333-346)server/Controllers/QuickLab/labAppointmentController.js (5)
req(12-12)req(13-21)appointment(131-144)appointment(335-335)appointment(394-401)
server/Controllers/QuickLab/labAdminController.js (2)
server/Controllers/QuickLab/labController.js (5)
req(7-7)req(42-42)req(60-60)lab(44-44)lab(62-62)server/Controllers/QuickLab/labStaffController.js (6)
staff(36-46)staff(80-83)staff(104-107)staff(127-127)staff(156-156)staff(196-196)
server/Controllers/QuickLab/labStaffController.js (1)
server/Controllers/QuickLab/labAppointmentController.js (5)
req(12-12)req(13-21)LabStaff(275-275)LabStaff(347-347)validStatuses(382-389)
server/Routes/QuickLab/labStaffRoutes.js (1)
server/Controllers/QuickLab/labStaffController.js (16)
createStaffProfile(9-62)createStaffProfile(9-62)updateStaffProfile(65-97)updateStaffProfile(65-97)getStaffProfile(100-120)getStaffProfile(100-120)checkStaffProfileExists(123-148)checkStaffProfileExists(123-148)getMyAssignments(151-188)getMyAssignments(151-188)getAssignmentDetails(191-222)getAssignmentDetails(191-222)updateMyAssignmentStatus(225-274)updateMyAssignmentStatus(225-274)completeAssignment(277-306)completeAssignment(277-306)
🔇 Additional comments (23)
server/Controllers/authController.js (1)
20-20: LGTM!Role validation correctly expanded to include new lab roles.
server/Routes/QuickLab/labRoutes.js (1)
1-16: LGTM!Route definitions are clear and properly protected with authentication middleware. The public nature of lab search and details endpoints is appropriate for allowing patients and doctors to discover and view lab information.
server/index.js (1)
22-27: LGTM!QuickLab route modules are properly imported and registered under appropriate API paths. The integration expands the API surface cleanly alongside existing QuickMed routes.
Also applies to: 54-58
server/Middleware/authMiddleware.js (1)
6-7: LGTM!Authentication middleware correctly extended to support new lab roles. The implementation follows the existing pattern for role-based profile lookups.
Also applies to: 36-41
server/models/Users/Doctor.js (1)
37-37: LGTM!The new field enables doctors to track lab appointments they suggest to patients, aligning with the QuickLab feature set.
server/models/Lab/LabAdmin.js (1)
1-32: LGTM!The LabAdmin model is well-structured with appropriate field types and constraints. The sparse index on labId correctly handles the optional lab association, and the permissions object provides granular access control.
server/Controllers/doctorController.js (1)
413-442: Verify the implementation completeness.The function has several gaps:
- The
patientIdparameter is validated but never used to verify the patient exists- The
testsandnotesparameters are accepted but not validated or persisted anywhere- Lines 427-428 contain placeholder comments indicating that the actual suggestion/notification logic is not implemented
- No record is created to track the suggestion
This appears to be a skeleton implementation. Confirm whether this is intentional (to be completed later) or if the implementation should be finalized before merging.
server/models/Users/Patient.js (1)
31-31: LGTM!The new field enables patients to track their lab appointments, following the same pattern as the existing appointments field.
server/models/Users/User.js (1)
6-11: LGTM! Role expansion supports new QuickLab roles.The addition of
lab_adminandlab_staffto the role enum and therefreshTokenfield properly support the new lab management functionality. The implementation is clean and follows Mongoose conventions.server/models/Lab/LabReport.js (1)
6-28: LGTM! Reference fields properly configured.The required references (appointmentId, patientId, labId) ensure data integrity, while the optional doctorId provides flexibility for reports that may not be doctor-initiated.
server/Routes/QuickLab/labReportRoutes.js (2)
15-24: LGTM! Routes properly secured with role-based authorization.The upload route correctly chains authentication, authorization, file upload middleware, and the controller. The patient, doctor, and remarks routes have appropriate role restrictions that align with the QuickLab workflow.
25-25: Authorization handled in controller for this endpoint.Unlike other routes, this endpoint uses only
authenticatewithout role-basedauthorizemiddleware. Based on the controller implementation, authorization is performed insidegetReportDetailsbased on the user's role and relationship to the report (patient viewing own reports, doctor viewing assigned reports). This approach is acceptable and provides more flexible access control.server/Routes/QuickLab/labAppointmentRoutes.js (1)
14-28: LGTM! Well-designed authorization hierarchy.The routes demonstrate a thoughtful authorization structure:
- Patients can book and view their own appointments
- Lab staff can view and update appointment status
- Staff assignment is restricted to lab admins, maintaining proper administrative control
This hierarchy aligns well with real-world lab operations.
server/models/Lab/LabStaff.js (2)
6-27: LGTM! Well-designed staff profile structure.The model properly supports the staff onboarding workflow with:
- Unique userId constraint preventing duplicate profiles
- Required personal information ensuring data completeness
- Optional labId allowing profile creation before lab assignment
- Appropriate role enumeration for lab operations
29-56: LGTM! Comprehensive staff tracking and assignment management.The model includes:
- Status flags for workflow tracking (profile completion, lab assignment, availability)
- Separate current and completed assignment arrays for proper task management
- Professional details supporting qualification verification
This structure supports the full lifecycle of lab staff operations.
server/Routes/QuickLab/labAdminRoutes.js (1)
19-42: LGTM! Well-organized and consistently secured admin routes.The routes demonstrate excellent organization:
- Logical grouping by function (profile, lab, staff, tests, info)
- Consistent authentication and authorization
- Proper file upload configuration for lab creation (1 logo, up to 10 photos)
- RESTful URL patterns with appropriate HTTP methods
The structure will be easy to maintain and extend.
server/models/Lab/LabAppointment.js (2)
6-22: LGTM! Core references properly configured.The required patient and lab references ensure data integrity, while the optional
suggestedByDoctorIdprovides flexibility for both doctor-initiated and patient-initiated appointments.
66-91: LGTM! Comprehensive appointment lifecycle and payment tracking.The model includes:
- Required appointment timing fields
- Well-defined status progression (pending → confirmed → sample_collected → processing → completed/cancelled)
- Proper payment tracking with support for refunds
- Optional notes field for additional context
This structure supports the complete appointment workflow.
server/Routes/QuickLab/labStaffRoutes.js (2)
19-34: LGTM! Well-designed profile management endpoints.The profile routes provide a complete CRUD interface with:
- Proper file upload handling for profile pictures
- Consistent authentication and authorization
- A helpful profile check endpoint supporting the onboarding flow
37-55: LGTM! Complete assignment workflow coverage.The assignment routes effectively support the staff workflow:
- List view with filtering capability
- Detail view for specific assignments
- Status update endpoint for progress tracking
- Dedicated completion endpoint for finalizing sample collection
The separation of status updates and completion provides clear action semantics.
server/Controllers/QuickLab/labController.js (1)
40-79: LGTM!
getLabDetailsandgetLabTestsare well-implemented with proper error handling, field selection for security (excluding sensitive fields), and filtering for active tests.server/Controllers/QuickLab/labAdminController.js (1)
10-50: LGTM!The profile creation, staff search, staff management, and lab info retrieval functions are well-implemented with proper validation, duplicate prevention, and error handling.
Also applies to: 110-167, 169-212, 252-271, 306-328
server/Controllers/QuickLab/labAppointmentController.js (1)
10-162: LGTM!
bookLabAppointmenthas thorough validation logic for home collection constraints, equipment requirements, and fee calculations.checkHomeCollectionAvailabilityandgetPatientLabAppointmentsare well-implemented with proper error handling.Also applies to: 165-228, 231-257
| return res.status(400).json({ message: 'Invalid ID format' }); | ||
| } | ||
|
|
||
| const lab = await Lab.findById(labId); |
There was a problem hiding this comment.
Missing import for Lab model.
The Lab model is referenced but not imported, which will cause a ReferenceError at runtime.
Add the import at the top of the file alongside other model imports:
import mongoose from 'mongoose';
import Clinic from '../models/Clinic/Clinic.js';
import Doctor from '../models/Users/Doctor.js';
+import Lab from '../models/Lab/Lab.js';
import { uploadToCloudinary } from '../services/uploadService.js';📝 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 lab = await Lab.findById(labId); | |
| import mongoose from 'mongoose'; | |
| import Clinic from '../models/Clinic/Clinic.js'; | |
| import Doctor from '../models/Users/Doctor.js'; | |
| import Lab from '../models/Lab/Lab.js'; | |
| import { uploadToCloudinary } from '../services/uploadService.js'; |
🤖 Prompt for AI Agents
In server/Controllers/doctorController.js around line 422 the Lab model is
referenced but not imported; add the Lab model import at the top of the file
alongside the other model imports (matching the project's import
style—require(...) or ES import) so Lab is defined before use.
| const staff = await LabStaff.findById(staffId); | ||
| if (!staff) { | ||
| return res.status(404).json({ message: 'Staff not found' }); | ||
| } | ||
|
|
||
| if (staff.labId.toString() !== labAdmin.labId.toString()) { | ||
| return res.status(403).json({ message: 'Staff does not belong to your lab' }); | ||
| } |
There was a problem hiding this comment.
Potential null pointer exception when staff is not assigned to any lab.
If staff.labId is null, calling .toString() will throw. Add a null check.
const staff = await LabStaff.findById(staffId);
if (!staff) {
return res.status(404).json({ message: 'Staff not found' });
}
- if (staff.labId.toString() !== labAdmin.labId.toString()) {
+ if (!staff.labId || staff.labId.toString() !== labAdmin.labId.toString()) {
return res.status(403).json({ message: 'Staff does not belong to your lab' });
}📝 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 staff = await LabStaff.findById(staffId); | |
| if (!staff) { | |
| return res.status(404).json({ message: 'Staff not found' }); | |
| } | |
| if (staff.labId.toString() !== labAdmin.labId.toString()) { | |
| return res.status(403).json({ message: 'Staff does not belong to your lab' }); | |
| } | |
| const staff = await LabStaff.findById(staffId); | |
| if (!staff) { | |
| return res.status(404).json({ message: 'Staff not found' }); | |
| } | |
| if (!staff.labId || staff.labId.toString() !== labAdmin.labId.toString()) { | |
| return res.status(403).json({ message: 'Staff does not belong to your lab' }); | |
| } |
🤖 Prompt for AI Agents
In server/Controllers/QuickLab/labAdminController.js around lines 225 to 232,
the code calls staff.labId.toString() which will throw if staff.labId is null;
add a defensive null check before comparing labIds so you first verify
staff.labId exists (e.g., if (!staff.labId) return res.status(400).json({
message: 'Staff is not assigned to any lab' })), then perform the toString()
comparison against labAdmin.labId.toString(); ensure you handle both
null/undefined and non-object cases to avoid the runtime exception.
| const lab = await Lab.findOneAndUpdate( | ||
| { _id: labAdmin.labId, 'tests._id': testId }, | ||
| { $set: { 'tests.$': { _id: testId, ...updates } } }, | ||
| { new: true, runValidators: true } | ||
| ); |
There was a problem hiding this comment.
updateTest may erase existing test fields not included in updates.
Using $set with 'tests.$': { _id: testId, ...updates } replaces the entire subdocument. Any fields not included in updates (e.g., isActive, homeCollectionAvailable) will be lost.
Use $set with dot notation for each field, or merge with existing data:
- const lab = await Lab.findOneAndUpdate(
- { _id: labAdmin.labId, 'tests._id': testId },
- { $set: { 'tests.$': { _id: testId, ...updates } } },
- { new: true, runValidators: true }
- );
+ // Build update object with dot notation for each field
+ const updateFields = {};
+ for (const [key, value] of Object.entries(updates)) {
+ updateFields[`tests.$.${key}`] = value;
+ }
+
+ const lab = await Lab.findOneAndUpdate(
+ { _id: labAdmin.labId, 'tests._id': testId },
+ { $set: updateFields },
+ { new: true, runValidators: true }
+ );📝 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 lab = await Lab.findOneAndUpdate( | |
| { _id: labAdmin.labId, 'tests._id': testId }, | |
| { $set: { 'tests.$': { _id: testId, ...updates } } }, | |
| { new: true, runValidators: true } | |
| ); | |
| // Build update object with dot notation for each field | |
| const updateFields = {}; | |
| for (const [key, value] of Object.entries(updates)) { | |
| updateFields[`tests.$.${key}`] = value; | |
| } | |
| const lab = await Lab.findOneAndUpdate( | |
| { _id: labAdmin.labId, 'tests._id': testId }, | |
| { $set: updateFields }, | |
| { new: true, runValidators: true } | |
| ); |
🤖 Prompt for AI Agents
In server/Controllers/QuickLab/labAdminController.js around lines 285 to 289,
the current update replaces the entire tests subdocument which can erase fields
not present in updates; instead retrieve the existing test subdocument (or
project it in the query), merge existing fields with the incoming updates, and
then perform findOneAndUpdate using $set with only the merged fields or
individual dot-notated fields (e.g., 'tests.$.fieldName') so you update only
provided properties and preserve unspecified ones; ensure runValidators and new
remain set and handle the case where the test isn't found.
| const Lab = (await import('../models/Lab/Lab.js')).default; | ||
| const lab = await Lab.findByIdAndUpdate( | ||
| labAdmin.labId, | ||
| { $push: { tests: testData } }, | ||
| { new: true, runValidators: true } | ||
| ); |
There was a problem hiding this comment.
Remove redundant dynamic import - Lab is already imported at the top of the file.
The dynamic import is unnecessary and uses an incorrect relative path (../models/ instead of ../../models/). Use the existing static import.
- const Lab = (await import('../models/Lab/Lab.js')).default;
const lab = await Lab.findByIdAndUpdate(
labAdmin.labId,
{ $push: { tests: testData } },
{ new: true, runValidators: true }
);🤖 Prompt for AI Agents
In server/Controllers/QuickLab/labAdminController.js around lines 401 to 406,
remove the redundant dynamic import and incorrect path; the file already has a
static Lab import at the top. Delete the "const Lab = (await
import(...)).default;" line and call the existing static Lab directly in the
findByIdAndUpdate call (i.e., replace usage of the dynamic-imported Lab by the
already-imported Lab), ensuring there are no leftover unused variables or
incorrect paths.
| if (role === 'lab_admin') { | ||
| const LabAdmin = (await import('../models/Users/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/Users/LabStaff.js')).default; | ||
| const staff = await LabStaff.findById(profileId); | ||
| if (!staff) { | ||
| return res.status(404).json({ message: 'Staff not found' }); | ||
| } | ||
| labId = staff.labId; | ||
| } |
There was a problem hiding this comment.
Incorrect import paths for LabAdmin and LabStaff models.
The paths reference ../models/Users/ but based on the static imports and AI summary, these models are located in ../../models/Lab/.
if (role === 'lab_admin') {
- const LabAdmin = (await import('../models/Users/LabAdmin.js')).default;
+ const LabAdmin = (await import('../../models/Lab/LabAdmin.js')).default;
const labAdmin = await LabAdmin.findById(profileId);
// ...
} else if (role === 'lab_staff') {
- const LabStaff = (await import('../models/Users/LabStaff.js')).default;
+ const LabStaff = (await import('../../models/Lab/LabStaff.js')).default;
const staff = await LabStaff.findById(profileId);Better yet, use static imports at the top of the file since LabStaff is already imported:
import LabStaff from '../../models/Lab/LabStaff.js';
+import LabAdmin from '../../models/Lab/LabAdmin.js';📝 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 (role === 'lab_admin') { | |
| const LabAdmin = (await import('../models/Users/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/Users/LabStaff.js')).default; | |
| const staff = await LabStaff.findById(profileId); | |
| if (!staff) { | |
| return res.status(404).json({ message: 'Staff not found' }); | |
| } | |
| labId = staff.labId; | |
| } | |
| 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; | |
| } |
🤖 Prompt for AI Agents
In server/Controllers/QuickLab/labAppointmentController.js around lines 267 to
281, the dynamic imports use incorrect paths ../models/Users/ and should
reference the lab models under ../../models/Lab/ and be statically imported;
replace the dynamic import of LabAdmin with a static import at the top from
../../models/Lab/LabAdmin.js (or update the path to ../../models/Lab/LabAdmin.js
if keeping dynamic), remove or reuse the already statically imported LabStaff
(ensure it imports from ../../models/Lab/LabStaff.js), and update the code to
use those static references (LabAdmin and LabStaff) instead of incorrect runtime
imports.
| export const getMyAssignments = async (req, res) => { | ||
| try { | ||
| const { profileId } = req.user; | ||
| const { status, type } = req.query; // type: current or completed | ||
|
|
||
| const staff = await LabStaff.findById(profileId); | ||
| if (!staff) { | ||
| return res.status(404).json({ message: 'Staff profile not found' }); | ||
| } | ||
|
|
||
| let query = { assignedStaffId: profileId }; | ||
|
|
||
| if (status) { | ||
| query.status = status; | ||
| } | ||
|
|
||
| // Filter by current or completed | ||
| if (type === 'completed') { | ||
| query.status = 'completed'; | ||
| } else if (type === 'current') { | ||
| query.status = { $in: ['confirmed', 'sample_collected', 'processing'] }; | ||
| } | ||
|
|
||
| const assignments = await LabAppointment.find(query) | ||
| .populate('patientId', 'firstName lastName phoneNumber address') | ||
| .populate('labId', 'name address') | ||
| .sort({ appointmentDate: 1, appointmentTime: 1 }) | ||
| .lean(); | ||
|
|
||
| res.json({ | ||
| count: assignments.length, | ||
| assignments, | ||
| }); | ||
| } catch (error) { | ||
| console.error('Error fetching staff assignments:', error); | ||
| res.status(500).json({ message: 'Failed to fetch assignments' }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Validate status/type query params (avoid operator injection + weird filtering)
req.query.status can be non-string (depending on Express query parser). Assigning it directly into a Mongo query can enable operator-style shapes (e.g., status[$ne]=...) and/or cause confusing behavior.
Suggested: enforce typeof status === 'string' and allowlist values; same for type.
const { profileId } = req.user;
- const { status, type } = req.query; // type: current or completed
+ const { status, type } = req.query; // type: current or completed
+ const allowedTypes = new Set(['current', 'completed']);
+ const allowedStatuses = new Set([
+ 'pending','confirmed','sample_collected','processing','completed','cancelled',
+ ]);
...
- if (status) {
- query.status = status;
- }
+ if (status !== undefined) {
+ if (typeof status !== 'string' || !allowedStatuses.has(status)) {
+ return res.status(400).json({ message: 'Invalid status filter' });
+ }
+ query.status = status;
+ }
...
- if (type === 'completed') {
+ if (type !== undefined) {
+ if (typeof type !== 'string' || !allowedTypes.has(type)) {
+ return res.status(400).json({ message: 'Invalid type filter' });
+ }
+ }
+ if (type === 'completed') {
query.status = 'completed';
} else if (type === 'current') {
query.status = { $in: ['confirmed', 'sample_collected', 'processing'] };
}🤖 Prompt for AI Agents
In server/Controllers/QuickLab/labStaffController.js around lines 151 to 188,
validate and sanitize req.query.status and req.query.type before using them in
the Mongo query to prevent operator injection and weird filtering: ensure each
is typeof 'string', match against an allowlist (define allowedStatuses and
allowedTypes arrays with permitted values), and only use the matched primitive
string value when setting query.status or branching on type; if a value is
missing or invalid either ignore it (don’t set query keys) or return a 400 with
a clear message — never assign req.query objects directly into the query and
always use the validated string values when building the query.
| export const getAssignmentDetails = async (req, res) => { | ||
| try { | ||
| const { profileId } = req.user; | ||
| const { appointmentId } = req.params; | ||
|
|
||
| const staff = await LabStaff.findById(profileId); | ||
| if (!staff) { | ||
| return res.status(404).json({ message: 'Staff profile not found' }); | ||
| } | ||
|
|
||
| const assignment = await LabAppointment.findById(appointmentId) | ||
| .populate('patientId', 'firstName lastName phoneNumber address emergencyContact') | ||
| .populate('labId', 'name address contact') | ||
| .lean(); | ||
|
|
||
| if (!assignment) { | ||
| return res.status(404).json({ message: 'Assignment not found' }); | ||
| } | ||
|
|
||
| // Check if this assignment belongs to this staff | ||
| if (assignment.assignedStaffId?.toString() !== profileId.toString()) { | ||
| return res.status(403).json({ message: 'Unauthorized to view this assignment' }); | ||
| } | ||
|
|
||
| res.json({ | ||
| assignment, | ||
| }); | ||
| } catch (error) { | ||
| console.error('Error fetching assignment details:', error); | ||
| res.status(500).json({ message: 'Failed to fetch assignment details' }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Validate appointmentId before querying + confirm patient PII exposure
- Add
ObjectId.isValid(appointmentId)guard to avoid CastError→500. - You populate patient
addressandemergencyContact. If lab staff doesn’t strictly need both, consider reducing fields (privacy minimization).
🤖 Prompt for AI Agents
In server/Controllers/QuickLab/labStaffController.js around lines 191 to 222,
add validation for appointmentId using mongoose.Types.ObjectId.isValid before
calling LabAppointment.findById to avoid CastError and early-return 400 when
invalid; import or reference ObjectId appropriately. Also review the populated
patient fields and remove or limit sensitive PII (e.g., drop address and
emergencyContact) unless required by this endpoint — either populate only
firstName/lastName/phoneNumber or make inclusion conditional based on staff
permissions, then return the reduced assignment object.
| export const updateMyAssignmentStatus = async (req, res) => { | ||
| try { | ||
| const { profileId } = req.user; | ||
| const { appointmentId } = req.params; | ||
| const { status, notes } = req.body; | ||
|
|
||
| const validStatuses = ['confirmed', 'sample_collected', 'processing']; | ||
| if (!validStatuses.includes(status)) { | ||
| return res.status(400).json({ | ||
| message: | ||
| 'Invalid status. Staff can only update to: confirmed, sample_collected, processing', | ||
| }); | ||
| } | ||
|
|
||
| const assignment = await LabAppointment.findById(appointmentId); | ||
| if (!assignment) { | ||
| return res.status(404).json({ message: 'Assignment not found' }); | ||
| } | ||
|
|
||
| // Check if this assignment belongs to this staff | ||
| if (assignment.assignedStaffId?.toString() !== profileId.toString()) { | ||
| return res.status(403).json({ message: 'Unauthorized to update this assignment' }); | ||
| } | ||
|
|
||
| assignment.status = status; | ||
| if (notes) { | ||
| assignment.notes = notes; | ||
| } | ||
| await assignment.save(); | ||
|
|
||
| // If completed, move to completed assignments | ||
| if (status === 'completed') { | ||
| await LabStaff.findByIdAndUpdate(profileId, { | ||
| $pull: { currentAssignments: appointmentId }, | ||
| $push: { completedAssignments: appointmentId }, | ||
| }); | ||
| } | ||
|
|
||
| res.json({ | ||
| message: 'Assignment status updated successfully', | ||
| assignment: await assignment.populate([ | ||
| { path: 'patientId', select: 'firstName lastName' }, | ||
| { path: 'labId', select: 'name' }, | ||
| ]), | ||
| }); | ||
| } catch (error) { | ||
| console.error('Error updating assignment status:', error); | ||
| res.status(500).json({ message: 'Failed to update assignment status' }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Bug: status === 'completed' branch is unreachable
You allow only ['confirmed', 'sample_collected', 'processing'], but later check if (status === 'completed') and try to move assignments—this never runs.
Decide one:
- If staff can set
completed: include it invalidStatusesand update the error message. - If staff cannot set
completed: delete the dead block (and ensure assignment list bookkeeping is handled elsewhere).
Also consider validating appointmentId format (400) up front.
🤖 Prompt for AI Agents
In server/Controllers/QuickLab/labStaffController.js around lines 225 to 274,
the branch checking if (status === 'completed') is unreachable because
validStatuses only allow ['confirmed','sample_collected','processing']; either
(A) allow staff to mark completed by adding 'completed' to validStatuses and
updating the error message text accordingly, or (B) remove the dead block that
moves the assignment between currentAssignments and completedAssignments (and
ensure that completed bookkeeping is handled elsewhere); additionally, validate
appointmentId early (e.g., check ObjectId format) and return 400 for invalid IDs
before querying the DB.
| collectionAddress: { | ||
| street: String, | ||
| city: String, | ||
| state: String, | ||
| zipCode: String, | ||
| country: String, | ||
| coordinates: { | ||
| lat: Number, | ||
| lng: Number, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add schema validation for collectionAddress based on collectionType.
The collectionAddress is not required at the schema level, but it should be mandatory when collectionType is 'home_collection'. While controller-level validation may exist, schema-level validation provides a stronger guarantee of data integrity.
Consider adding a custom validator:
+// Add this before the schema export
+labAppointmentSchema.pre('save', function(next) {
+ if (this.collectionType === 'home_collection' && !this.collectionAddress?.street) {
+ next(new Error('Collection address is required for home collection appointments'));
+ } else {
+ next();
+ }
+});
+
export default mongoose.model('LabAppointment', labAppointmentSchema);Or alternatively, use a Mongoose validate function in the collectionAddress field definition. This ensures the constraint is enforced at the database layer regardless of the calling code.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/models/Lab/LabAppointment.js around lines 54 to 64, add a schema-level
conditional validator on collectionAddress so it is required when collectionType
=== 'home_collection'; implement this by replacing/augmenting the
collectionAddress definition with a validate function (or the validate property)
that checks this.collectionType and returns false with a clear message if
collectionAddress is missing or any required subfields (street, city, state,
zipCode, country, coordinates.lat, coordinates.lng) are absent/invalid when type
is 'home_collection'; keep the existing optional behavior for other
collectionType values and ensure the validator uses a non-arrow function so it
can access this.
| testResults: [ | ||
| { | ||
| testName: String, | ||
| testCode: String, | ||
| result: String, | ||
| normalRange: String, | ||
| unit: String, | ||
| isAbnormal: { type: Boolean, default: false }, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Add required constraints to testResults fields.
The testResults array items lack required field constraints, which could lead to incomplete or meaningless test data. At minimum, testName and result should be required to ensure each test result contains essential information.
Apply this diff to add required constraints:
testResults: [
{
- testName: String,
+ testName: { type: String, required: true },
testCode: String,
- result: String,
+ result: { type: String, required: true },
normalRange: String,
unit: String,
isAbnormal: { type: Boolean, default: 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.
| testResults: [ | |
| { | |
| testName: String, | |
| testCode: String, | |
| result: String, | |
| normalRange: String, | |
| unit: String, | |
| isAbnormal: { type: Boolean, default: false }, | |
| }, | |
| ], | |
| testResults: [ | |
| { | |
| testName: { type: String, required: true }, | |
| testCode: String, | |
| result: { type: String, required: true }, | |
| normalRange: String, | |
| unit: String, | |
| isAbnormal: { type: Boolean, default: false }, | |
| }, | |
| ], |
🤖 Prompt for AI Agents
In server/models/Lab/LabReport.js around lines 30 to 39, the testResults
subdocument fields lack required constraints; update the schema so that at
minimum testName and result have required: true (e.g., set testName: { type:
String, required: true } and result: { type: String, required: true }); keep
other fields as-is (or add required if desired), then run tests/validate model
usage to ensure no breaking changes.
Project
Change Type
Stack
Page Type
Route/API Endpoint Status
What Changed
Route/API Affected
Lab Admin Routes (/api/lab-admin)
POST /profile - Create lab admin profile
POST /lab - Create lab (with logo and photos upload)
GET /staff/search - Search for staff members
POST /staff/add - Add staff to lab
DELETE /staff/:staffId - Remove staff from lab
GET /staff - Get lab staff list
POST /tests - Add new test
PUT /tests/:testId - Update existing test
GET /lab/info - Get lab information
Lab Appointment Routes (/api/lab-appointment)
POST /book - Book lab appointment (patient)
GET /patient - Get patient's lab appointments (patient)
GET /lab - Get lab appointments (lab admin/staff)
PUT /:appointmentId/assign-staff - Assign staff for sample collection (lab admin)
PUT /:appointmentId/status - Update appointment status (lab admin/staff)
Lab Report Routes (/api/lab-report)
POST /upload/:appointmentId - Upload lab report (lab admin/staff)
GET /patient - Get patient lab reports (patient)
GET /doctor - Get doctor's patient reports (doctor)
PUT /:reportId/remarks - Add doctor remarks (doctor)
GET /:reportId - Get report details (authenticated users)
Lab Staff Routes (/api/lab-staff)
POST /profile - Create staff profile (with profile picture)
PUT /profile - Update staff profile (with profile picture)
GET /profile - Get staff profile
GET /profile/check - Check if staff profile exists
GET /assignments - Get staff assignments
GET /assignments/:appointmentId - Get assignment details
PUT /assignments/:appointmentId/status - Update assignment status
POST /assignments/:appointmentId/complete - Complete assignment
Lab Routes (/api/lab)
GET /search - Search labs
GET /:labId - Get lab details
GET /:labId/tests - Get lab tests
Description
backend for quicklab
Screenshots (If Applicable)
Mobile View
Desktop View
Code Quality
npx prettier --check .)Related Issues
Closes #NA
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.