Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
src/modules/appointment/appointment.helper.ts (1)
93-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the new resource-selection helpers.
pickByLowestLoad,getFreeResources, andisResourceFreeare new, non-trivial pure functions handling core booking logic (lowest-load selection, tie-break shuffling, free-slot filtering), but the existing spec file only importsmergeSlotsandisGridAligned. Given these functions directly affect which technician/bay gets booked, dedicated test coverage (e.g. tie counts, empty candidate lists, topK boundary) would materially reduce regression risk.Want me to draft the test cases for these functions?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/appointment/appointment.helper.ts` around lines 93 - 115, Add unit tests in the existing appointment helper spec for pickByLowestLoad, getFreeResources, and isResourceFree, importing each helper alongside mergeSlots and isGridAligned. Cover lowest-load ordering, randomized tie handling, empty candidate lists, topK limits, and free-slot filtering, including representative occupied and available resource cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.ts`:
- Line 36: Remove the custom `app.set('query parser', 'extended')` configuration
from the application setup in `main.ts`, allowing Express to use its default
simple parser for the flat query parameters consumed by the app.
In `@src/modules/appointment/appointment.controller.ts`:
- Around line 7-17: Update the AppointmentController route configuration so
getAvailability resolves to /dealerships/:id/availability instead of being
prefixed with /appointments, while preserving its existing handler parameters
and service call.
In `@src/modules/appointment/appointment.service.ts`:
- Around line 197-209: Update the candidate retry flow surrounding the
appointmentRepo.save call so every failed candidate rolls back all writes before
continuing. Use a savepoint or repository-level transaction boundary
encompassing the appointment and reservation inserts, release it on success, and
roll it back in the ER_DUP_ENTRY/1062 retry path while preserving the existing
duplicate vehicle booking ConflictException behavior.
- Line 213: Update the candidate-exhaustion path in the appointment booking flow
around the NotFoundException using AppointmentErrorMessages.NO_RESOURCE to
return a conflict response instead. Preserve refreshed booking alternatives when
available, distinguishing resources made unavailable by concurrent bookings from
genuinely nonexistent resources.
- Around line 124-125: Update the appointment validation flow around
validateTime, mergeSlots, and the business-hours check near lines 216–228 to
derive business hours from the dealership-local calendar date rather than the
UTC date. Validate the complete merged interval, rejecting appointments whose
end time extends beyond closing, while preserving valid appointments that fall
within the local business-day hours.
In `@src/modules/appointment/dtos/create-appointment.dto.ts`:
- Line 1: Update the four foreign-key fields in CreateAppointmentDto to use
class-validator’s IsInt validation instead of IsNumber, while retaining
IsPositive. Add the IsInt import and ensure all four IDs reject decimal values.
- Around line 32-33: Update the startAt validation in the create-appointment DTO
to require an ISO date-time with an explicit timezone offset, preventing
timezone-less values from reaching toUtc(). Preserve the existing string
property and reject inputs without a UTC offset or timezone designator.
In `@src/modules/appointment/dtos/get-availability.dto.ts`:
- Line 1: Add the IsPositive validator to the serviceTypeId property in
GetAvailabilityDto, alongside its existing validation decorators, and update the
class-validator import accordingly so zero and negative IDs are rejected.
In `@src/modules/appointment/tests/appointment.service.spec.ts`:
- Around line 35-43: Update the appointmentRepoMock used by the cancellation
tests to include exists instead of the unused findOne method, and configure save
to return the cancellation-appropriate fixture rather than the confirmed
fixture. Ensure the mock shape matches the repository methods invoked by
cancelAppointment().
- Around line 249-251: Update the appointment service test assertions to
reference the mock object directly, replacing appointmentRepo method assertions
with appointmentRepoMock.lockTechnician, appointmentRepoMock.lockServiceBay,
appointmentRepoMock.save, and the corresponding mock properties at the other
affected assertions. Preserve the existing call-count expectations while
avoiding unbound-method lint violations.
In `@TASKS.md`:
- Around line 174-181: Update the Task 12 documentation in TASKS.md to remove
all remaining reschedule requirements, including the reschedule tests and the
locked PATCH contract entry. Keep the task scope limited to cancel and fetch
behavior, with no references implying rescheduling is required.
---
Nitpick comments:
In `@src/modules/appointment/appointment.helper.ts`:
- Around line 93-115: Add unit tests in the existing appointment helper spec for
pickByLowestLoad, getFreeResources, and isResourceFree, importing each helper
alongside mergeSlots and isGridAligned. Cover lowest-load ordering, randomized
tie handling, empty candidate lists, topK limits, and free-slot filtering,
including representative occupied and available resource cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8cc8b02d-6d80-43b7-b125-3eb9d9503b15
📒 Files selected for processing (27)
TASKS.mdsrc/main.tssrc/modules/appointment/appointment.controller.tssrc/modules/appointment/appointment.helper.tssrc/modules/appointment/appointment.module.tssrc/modules/appointment/appointment.repository.tssrc/modules/appointment/appointment.service.tssrc/modules/appointment/constants/appointment.constant.tssrc/modules/appointment/constants/appointment.message.tssrc/modules/appointment/dtos/create-appointment.dto.tssrc/modules/appointment/dtos/get-availability.dto.tssrc/modules/appointment/entities/appointment.entity.tssrc/modules/appointment/entities/resource-reservation.entity.tssrc/modules/appointment/interfaces/resource-reservation.type.tssrc/modules/appointment/tests/appointment.service.spec.tssrc/modules/dealership/dealership.controller.tssrc/modules/dealership/tests/dealership.controller.spec.tssrc/modules/dealership/tests/dealership.service.spec.tssrc/modules/service-bay/service-bay.controller.tssrc/modules/service-bay/service-bay.service.tssrc/modules/service-type/constants/service-type.message.tssrc/modules/service-type/service-type.helper.tssrc/modules/service-type/service-type.service.tssrc/modules/technician/technician.controller.tssrc/modules/technician/technician.service.tssrc/shared/utils/common.helper.tssrc/shared/utils/date.helper.ts
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Request timed out after 900000ms (requestId=c2a42ce6-3fdf-401c-874a-0cc2243335de) |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.gitignore (1)
57-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not ignore the entire
.githubdirectory.This can silently exclude GitHub Actions, CODEOWNERS, Dependabot, and issue-template changes. Ignore only the intended local files, unless this repository intentionally never tracks
.githubcontent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore around lines 57 - 60, Update the .gitignore entry for .github so it no longer ignores the entire directory. Remove that broad pattern and add only the intended local .github files or subpaths, preserving tracking for workflows, CODEOWNERS, Dependabot, and issue templates.src/modules/appointment/tests/appointment.service.spec.ts (1)
258-302: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test exercises the savepoint rollback path.
rollbackToSavepointis mocked (line 76) and asserted as "not called" for the happy path (line 300), but no test forces a conflict (e.g.,reservationRepo.saverejecting, or a lock returningnull/throwing) to verifyrollbackToSavepointis actually invoked and propagated correctly on booking failure. Given this is the core safety mechanism for concurrent double-booking prevention, it's worth covering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/appointment/tests/appointment.service.spec.ts` around lines 258 - 302, Add a failure-path test alongside the existing createAppointment transaction tests that forces a booking conflict, such as making reservationRepo.save reject, then call service.createAppointment and assert the failure propagates. Verify appointmentRepo.rollbackToSavepoint is called with 'sp_1_1', while the successful savepoint release is not performed, covering the rollback behavior exercised by createAppointment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/dealership/tests/dealership.service.spec.ts`:
- Around line 100-102: Update the missing-update test for
DealershipService.update to mock repository.findOne with a null result instead
of configuring repository.exists. Keep the expectation for NotFoundException
unchanged so the test exercises the service’s actual entity lookup path.
In `@src/modules/technician/technician.controller.ts`:
- Around line 44-47: Update TechnicianController.findQualification to use the
active-only lookup by passing true to technicianService.findBy (or invoking the
existing active-only helper), ensuring qualification results exclude inactive
technicians.
---
Nitpick comments:
In @.gitignore:
- Around line 57-60: Update the .gitignore entry for .github so it no longer
ignores the entire directory. Remove that broad pattern and add only the
intended local .github files or subpaths, preserving tracking for workflows,
CODEOWNERS, Dependabot, and issue templates.
In `@src/modules/appointment/tests/appointment.service.spec.ts`:
- Around line 258-302: Add a failure-path test alongside the existing
createAppointment transaction tests that forces a booking conflict, such as
making reservationRepo.save reject, then call service.createAppointment and
assert the failure propagates. Verify appointmentRepo.rollbackToSavepoint is
called with 'sp_1_1', while the successful savepoint release is not performed,
covering the rollback behavior exercised by createAppointment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 467d2e2f-4636-4e22-9e07-ab019ab90384
📒 Files selected for processing (36)
.gitignoreTASKS.mdsrc/main.tssrc/modules/appointment/appointment.controller.tssrc/modules/appointment/appointment.helper.tssrc/modules/appointment/appointment.module.tssrc/modules/appointment/appointment.repository.tssrc/modules/appointment/appointment.service.tssrc/modules/appointment/constants/appointment.constant.tssrc/modules/appointment/constants/appointment.message.tssrc/modules/appointment/dtos/create-appointment.dto.tssrc/modules/appointment/dtos/get-availability.dto.tssrc/modules/appointment/entities/appointment.entity.tssrc/modules/appointment/entities/resource-reservation.entity.tssrc/modules/appointment/interfaces/resource-reservation.type.tssrc/modules/appointment/tests/appointment.helper.spec.tssrc/modules/appointment/tests/appointment.service.spec.tssrc/modules/customer/tests/customer.service.spec.tssrc/modules/dealership/dealership.controller.tssrc/modules/dealership/tests/dealership.controller.spec.tssrc/modules/dealership/tests/dealership.service.spec.tssrc/modules/service-bay/service-bay.controller.tssrc/modules/service-bay/service-bay.service.tssrc/modules/service-bay/tests/service-bay.service.spec.tssrc/modules/service-type/constants/service-type.message.tssrc/modules/service-type/service-type.helper.tssrc/modules/service-type/service-type.service.tssrc/modules/service-type/tests/service-type.service.spec.tssrc/modules/technician/technician.controller.tssrc/modules/technician/technician.service.tssrc/modules/technician/tests/technician.service.spec.tssrc/modules/vehicle/tests/vehicle.service.spec.tssrc/shared/utils/common.helper.tssrc/shared/utils/date.helper.tstest/appointment.e2e-spec.tstest/jest-e2e.json
🚧 Files skipped from review as they are similar to previous changes (20)
- src/modules/service-type/service-type.helper.ts
- src/shared/utils/date.helper.ts
- src/modules/appointment/interfaces/resource-reservation.type.ts
- src/modules/service-type/constants/service-type.message.ts
- src/modules/appointment/constants/appointment.constant.ts
- src/modules/appointment/dtos/create-appointment.dto.ts
- src/modules/appointment/appointment.module.ts
- src/modules/appointment/constants/appointment.message.ts
- src/shared/utils/common.helper.ts
- src/modules/dealership/dealership.controller.ts
- src/modules/appointment/entities/resource-reservation.entity.ts
- src/modules/service-bay/service-bay.controller.ts
- src/modules/appointment/dtos/get-availability.dto.ts
- src/modules/technician/technician.service.ts
- TASKS.md
- src/modules/service-type/service-type.service.ts
- src/modules/appointment/entities/appointment.entity.ts
- src/modules/service-bay/service-bay.service.ts
- src/modules/appointment/appointment.service.ts
- src/modules/appointment/appointment.helper.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
TASKS.md (1)
174-183: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove
reschedulefrom the locked PATCH contract.Task 12 now covers only cancellation and fetching, but line 196 still documents
{ action: 'cancel' | 'reschedule', startAt?: string }. Update it to{ action: 'cancel' }, or restore rescheduling consistently across the implementation and task scope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@TASKS.md` around lines 174 - 183, Update the Task 12 locked PATCH contract to allow only { action: 'cancel' }, removing reschedule and startAt from the documented payload; keep the task scope focused on cancellation and fetching.
🧹 Nitpick comments (1)
src/modules/service-type/tests/service-type.service.spec.ts (1)
84-84: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the exception type as well as its message.
This test is named for
BadRequestException, buttoThrow(message)would also pass for any error with the same message. Keep the synchronous assertion and add a class check.Proposed fix
-import { NotFoundException } from '`@nestjs/common`'; +import { BadRequestException, NotFoundException } from '`@nestjs/common`'; ... - expect(() => service.create(invalidDto)).toThrow(ServiceTypeErrorMessages.INVALID_DURATION); + const create = () => service.create(invalidDto); + expect(create).toThrow(BadRequestException); + expect(create).toThrow(ServiceTypeErrorMessages.INVALID_DURATION);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/service-type/tests/service-type.service.spec.ts` at line 84, Update the synchronous assertion in the service.create invalid-duration test to verify that it throws BadRequestException while preserving the existing INVALID_DURATION message check; use an assertion form that checks both the exception class and message rather than matching the message alone.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/appointment/appointment.repository.ts`:
- Around line 49-59: Validate the name parameter in createSavepoint,
rollbackToSavepoint, and releaseSavepoint before interpolating it into SQL,
allowing only a safe savepoint identifier format and rejecting invalid names.
Keep the existing queries unchanged for valid names and do not use bind
parameters for the identifier.
---
Duplicate comments:
In `@TASKS.md`:
- Around line 174-183: Update the Task 12 locked PATCH contract to allow only {
action: 'cancel' }, removing reschedule and startAt from the documented payload;
keep the task scope focused on cancellation and fetching.
---
Nitpick comments:
In `@src/modules/service-type/tests/service-type.service.spec.ts`:
- Line 84: Update the synchronous assertion in the service.create
invalid-duration test to verify that it throws BadRequestException while
preserving the existing INVALID_DURATION message check; use an assertion form
that checks both the exception class and message rather than matching the
message alone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ad8ff12-f76a-4714-b84e-0035a8c61a50
📒 Files selected for processing (37)
.gitignoreTASKS.mdsrc/main.tssrc/modules/appointment/appointment.controller.tssrc/modules/appointment/appointment.helper.tssrc/modules/appointment/appointment.module.tssrc/modules/appointment/appointment.repository.tssrc/modules/appointment/appointment.service.tssrc/modules/appointment/constants/appointment.constant.tssrc/modules/appointment/constants/appointment.message.tssrc/modules/appointment/dtos/create-appointment.dto.tssrc/modules/appointment/dtos/get-availability.dto.tssrc/modules/appointment/entities/appointment.entity.tssrc/modules/appointment/entities/resource-reservation.entity.tssrc/modules/appointment/interfaces/resource-reservation.type.tssrc/modules/appointment/tests/appointment.helper.spec.tssrc/modules/appointment/tests/appointment.service.spec.tssrc/modules/customer/tests/customer.service.spec.tssrc/modules/dealership/dealership.controller.tssrc/modules/dealership/dealership.service.tssrc/modules/dealership/tests/dealership.controller.spec.tssrc/modules/dealership/tests/dealership.service.spec.tssrc/modules/service-bay/service-bay.controller.tssrc/modules/service-bay/service-bay.service.tssrc/modules/service-bay/tests/service-bay.service.spec.tssrc/modules/service-type/constants/service-type.message.tssrc/modules/service-type/service-type.helper.tssrc/modules/service-type/service-type.service.tssrc/modules/service-type/tests/service-type.service.spec.tssrc/modules/technician/technician.controller.tssrc/modules/technician/technician.service.tssrc/modules/technician/tests/technician.service.spec.tssrc/modules/vehicle/tests/vehicle.service.spec.tssrc/shared/utils/common.helper.tssrc/shared/utils/date.helper.tstest/appointment.e2e-spec.tstest/jest-e2e.json
💤 Files with no reviewable changes (2)
- src/modules/dealership/dealership.service.ts
- src/modules/dealership/tests/dealership.controller.spec.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- src/modules/appointment/interfaces/resource-reservation.type.ts
- src/modules/appointment/constants/appointment.message.ts
- src/modules/service-type/constants/service-type.message.ts
- test/jest-e2e.json
- src/modules/appointment/dtos/create-appointment.dto.ts
- src/modules/appointment/entities/resource-reservation.entity.ts
- .gitignore
- src/shared/utils/common.helper.ts
- src/main.ts
- src/modules/appointment/appointment.module.ts
- src/modules/dealership/dealership.controller.ts
- src/modules/service-bay/service-bay.controller.ts
- test/appointment.e2e-spec.ts
- src/modules/appointment/entities/appointment.entity.ts
- src/modules/vehicle/tests/vehicle.service.spec.ts
- src/modules/service-bay/service-bay.service.ts
- src/modules/technician/technician.service.ts
- src/shared/utils/date.helper.ts
- src/modules/service-bay/tests/service-bay.service.spec.ts
- src/modules/appointment/dtos/get-availability.dto.ts
- src/modules/technician/tests/technician.service.spec.ts
- src/modules/appointment/appointment.helper.ts
- src/modules/appointment/appointment.service.ts
Comment on lines
+49
to
+59
| async createSavepoint(name: string): Promise<void> { | ||
| return this.query(`SAVEPOINT ${name}`); | ||
| } | ||
|
|
||
| async rollbackToSavepoint(name: string): Promise<void> { | ||
| return this.query(`ROLLBACK TO SAVEPOINT ${name}`); | ||
| } | ||
|
|
||
| async releaseSavepoint(name: string): Promise<void> { | ||
| return this.query(`RELEASE SAVEPOINT ${name}`); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate savepoint names before interpolating them into SQL.
The unrestricted name parameter makes these methods raw SQL-injection sinks. Bind parameters cannot represent identifiers, so restrict names to a safe identifier format before executing the query.
Proposed fix
+ private validateSavepointName(name: string): string {
+ if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(name)) {
+ throw new Error('Invalid savepoint name');
+ }
+ return name;
+ }
+
async createSavepoint(name: string): Promise<void> {
- return this.query(`SAVEPOINT ${name}`);
+ return this.query(`SAVEPOINT ${this.validateSavepointName(name)}`);
}
async rollbackToSavepoint(name: string): Promise<void> {
- return this.query(`ROLLBACK TO SAVEPOINT ${name}`);
+ return this.query(`ROLLBACK TO SAVEPOINT ${this.validateSavepointName(name)}`);
}
async releaseSavepoint(name: string): Promise<void> {
- return this.query(`RELEASE SAVEPOINT ${name}`);
+ return this.query(`RELEASE SAVEPOINT ${this.validateSavepointName(name)}`);
}📝 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.
Suggested change
| async createSavepoint(name: string): Promise<void> { | |
| return this.query(`SAVEPOINT ${name}`); | |
| } | |
| async rollbackToSavepoint(name: string): Promise<void> { | |
| return this.query(`ROLLBACK TO SAVEPOINT ${name}`); | |
| } | |
| async releaseSavepoint(name: string): Promise<void> { | |
| return this.query(`RELEASE SAVEPOINT ${name}`); | |
| } | |
| private validateSavepointName(name: string): string { | |
| if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(name)) { | |
| throw new Error('Invalid savepoint name'); | |
| } | |
| return name; | |
| } | |
| async createSavepoint(name: string): Promise<void> { | |
| return this.query(`SAVEPOINT ${this.validateSavepointName(name)}`); | |
| } | |
| async rollbackToSavepoint(name: string): Promise<void> { | |
| return this.query(`ROLLBACK TO SAVEPOINT ${this.validateSavepointName(name)}`); | |
| } | |
| async releaseSavepoint(name: string): Promise<void> { | |
| return this.query(`RELEASE SAVEPOINT ${this.validateSavepointName(name)}`); | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 49-49: Avoid SQL injection
Context: this.query(SAVEPOINT ${name})
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-typescript)
[error] 53-53: Avoid SQL injection
Context: this.query(ROLLBACK TO SAVEPOINT ${name})
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-typescript)
[error] 57-57: Avoid SQL injection
Context: this.query(RELEASE SAVEPOINT ${name})
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-typescript)
🪛 OpenGrep (1.25.0)
[ERROR] 50-50: SQL query built via string concatenation or template literal passed to query()/execute(). Use parameterized queries instead.
(coderabbit.sql-injection.raw-query-concat-js)
[ERROR] 54-54: SQL query built via string concatenation or template literal passed to query()/execute(). Use parameterized queries instead.
(coderabbit.sql-injection.raw-query-concat-js)
[ERROR] 58-58: SQL query built via string concatenation or template literal passed to query()/execute(). Use parameterized queries instead.
(coderabbit.sql-injection.raw-query-concat-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/appointment/appointment.repository.ts` around lines 49 - 59,
Validate the name parameter in createSavepoint, rollbackToSavepoint, and
releaseSavepoint before interpolating it into SQL, allowing only a safe
savepoint identifier format and rejecting invalid names. Keep the existing
queries unchanged for valid names and do not use bind parameters for the
identifier.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/common/filters/exception.filter.ts`:
- Line 84: Update the duplicate-key error message at the message field in
exception.filter.ts to use grammatically correct wording, preferably “This data
already exists.” or an equivalent specific conflict message, while preserving
the existing API error behavior.
In `@src/configs/app.config.ts`:
- Line 15: Update the logging setting in the application configuration so
TypeORM logging is disabled in production while remaining enabled in appropriate
non-production environments. Use the existing environment/configuration
mechanism rather than hardcoding logging: true for every deployment.
- Around line 43-46: Update the Swagger configuration’s options object to nest
tagsSorter and operationsSorter inside swaggerOptions, preserving their existing
values so Swagger UI applies both sorting settings.
In `@src/modules/appointment/appointment.repository.ts`:
- Around line 50-52: Update the vehicleId filter in the appointment query
builder to compare against the appointment.vehicle_id column instead of
appointment.customer_id, while preserving the existing conditional parameter
binding and dealershipId filter.
In `@src/modules/appointment/entities/appointment.entity.ts`:
- Around line 22-27: Remove the orphaned `@ManyToOne`(() => Customer, ...)
decorator from the vehicle relation in the appointment entity. Keep the
`@ManyToOne`(() => Vehicle, ...), `@JoinColumn`, and vehicleId mapping unchanged so
vehicle is registered only as a Vehicle relation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 90d736a2-4069-4400-b21d-9e78a061c5b4
📒 Files selected for processing (49)
.gitignoreTASKS.mdsrc/common/filters/exception.filter.tssrc/configs/app.config.tssrc/configs/config.interface.tssrc/database/database.module.tssrc/database/entities/base.entity.tssrc/database/entities/timestamp.entity.tssrc/main.tssrc/modules/appointment/appointment.controller.tssrc/modules/appointment/appointment.helper.tssrc/modules/appointment/appointment.module.tssrc/modules/appointment/appointment.repository.tssrc/modules/appointment/appointment.service.tssrc/modules/appointment/constants/appointment.constant.tssrc/modules/appointment/constants/appointment.message.tssrc/modules/appointment/dtos/create-appointment.dto.tssrc/modules/appointment/dtos/get-availability.dto.tssrc/modules/appointment/dtos/search-appointment.dto.tssrc/modules/appointment/entities/appointment.entity.tssrc/modules/appointment/entities/resource-reservation.entity.tssrc/modules/appointment/interfaces/appointment-search.type.tssrc/modules/appointment/interfaces/resource-reservation.type.tssrc/modules/appointment/tests/appointment.helper.spec.tssrc/modules/appointment/tests/appointment.service.spec.tssrc/modules/customer/tests/customer.service.spec.tssrc/modules/dealership/dealership.controller.tssrc/modules/dealership/dealership.service.tssrc/modules/dealership/entities/dealership.entity.tssrc/modules/dealership/tests/dealership.controller.spec.tssrc/modules/dealership/tests/dealership.service.spec.tssrc/modules/service-bay/entities/service-bay.entity.tssrc/modules/service-bay/service-bay.controller.tssrc/modules/service-bay/service-bay.service.tssrc/modules/service-bay/tests/service-bay.service.spec.tssrc/modules/service-type/constants/service-type.message.tssrc/modules/service-type/service-type.helper.tssrc/modules/service-type/service-type.service.tssrc/modules/service-type/tests/service-type.service.spec.tssrc/modules/technician/entities/technician.entity.tssrc/modules/technician/technician.controller.tssrc/modules/technician/technician.service.tssrc/modules/technician/tests/technician.service.spec.tssrc/modules/vehicle/entities/vehicle.entity.tssrc/modules/vehicle/tests/vehicle.service.spec.tssrc/shared/utils/common.helper.tssrc/shared/utils/date.helper.tstest/appointment.e2e-spec.tstest/jest-e2e.json
💤 Files with no reviewable changes (3)
- src/database/entities/timestamp.entity.ts
- src/modules/dealership/tests/dealership.controller.spec.ts
- src/modules/dealership/dealership.service.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- src/modules/appointment/constants/appointment.message.ts
- .gitignore
- src/modules/service-type/service-type.helper.ts
- test/jest-e2e.json
- src/modules/appointment/dtos/create-appointment.dto.ts
- src/shared/utils/common.helper.ts
- src/modules/service-bay/service-bay.service.ts
- src/modules/service-type/service-type.service.ts
- src/modules/appointment/tests/appointment.helper.spec.ts
- src/modules/appointment/interfaces/resource-reservation.type.ts
- src/modules/appointment/entities/resource-reservation.entity.ts
- src/modules/dealership/dealership.controller.ts
- src/modules/service-type/tests/service-type.service.spec.ts
- src/modules/service-bay/tests/service-bay.service.spec.ts
- src/modules/service-type/constants/service-type.message.ts
- test/appointment.e2e-spec.ts
- src/modules/technician/technician.controller.ts
- src/shared/utils/date.helper.ts
- TASKS.md
- src/modules/technician/tests/technician.service.spec.ts
- src/modules/appointment/dtos/get-availability.dto.ts
- src/modules/service-bay/service-bay.controller.ts
- src/modules/customer/tests/customer.service.spec.ts
- src/modules/dealership/tests/dealership.service.spec.ts
| keepConnectionAlive: true, | ||
| extra: { connectionLimit: 10 }, | ||
| timezone: 'Z', | ||
| logging: true, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and inspect the exact context around the cited line.
git ls-files src/configs/app.config.ts
wc -l src/configs/app.config.ts
cat -n src/configs/app.config.ts | sed -n '1,220p'
# Search for other TypeORM / logging configuration that may override or contextualize this setting.
rg -n "logging\s*:|TypeOrmModule|DataSource|typeorm" src . --glob '!node_modules' --glob '!dist' --glob '!build'Repository: hytnht/appointment-scheduler
Length of output: 16060
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the config interface and database wiring.
cat -n src/configs/config.interface.ts
printf '\n---\n'
cat -n src/database/database.module.ts
printf '\n---\n'
cat -n src/app.module.ts
printf '\n---\n'
cat -n src/main.ts
# Locate Swagger setup and how the config is consumed.
rg -n "SwaggerModule|createDocument|swagger\.|tagsSorter|operationsSorter|SwaggerCustomOptions|swaggerOptions|options:" src testRepository: hytnht/appointment-scheduler
Length of output: 6753
Disable TypeORM logging in production
logging: true turns on query/error logging in every environment, including production. Make it environment-aware or configurable so SQL details and log volume don’t spill into prod logs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/configs/app.config.ts` at line 15, Update the logging setting in the
application configuration so TypeORM logging is disabled in production while
remaining enabled in appropriate non-production environments. Use the existing
environment/configuration mechanism rather than hardcoding logging: true for
every deployment.
Source: MCP tools
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/appointment/dtos/create-appointment.dto.ts`:
- Around line 26-28: Replace the `@MinLength` decorator on startAt with an
explicit timezone-suffix validator such as `@Matches`, requiring either Z or a
numeric ±HH:MM offset, while retaining `@IsDateString`({ strict: true }) for date
validation. Remove the length-based check so valid timezone-aware timestamps
like those ending in Z are accepted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6395d069-238b-422a-a662-d4149092b84d
📒 Files selected for processing (49)
.gitignoreTASKS.mdsrc/common/filters/exception.filter.tssrc/configs/app.config.tssrc/configs/config.interface.tssrc/database/database.module.tssrc/database/entities/base.entity.tssrc/database/entities/timestamp.entity.tssrc/main.tssrc/modules/appointment/appointment.controller.tssrc/modules/appointment/appointment.helper.tssrc/modules/appointment/appointment.module.tssrc/modules/appointment/appointment.repository.tssrc/modules/appointment/appointment.service.tssrc/modules/appointment/constants/appointment.constant.tssrc/modules/appointment/constants/appointment.message.tssrc/modules/appointment/dtos/create-appointment.dto.tssrc/modules/appointment/dtos/get-availability.dto.tssrc/modules/appointment/dtos/search-appointment.dto.tssrc/modules/appointment/entities/appointment.entity.tssrc/modules/appointment/entities/resource-reservation.entity.tssrc/modules/appointment/interfaces/appointment-search.type.tssrc/modules/appointment/interfaces/resource-reservation.type.tssrc/modules/appointment/tests/appointment.helper.spec.tssrc/modules/appointment/tests/appointment.service.spec.tssrc/modules/customer/tests/customer.service.spec.tssrc/modules/dealership/dealership.controller.tssrc/modules/dealership/dealership.service.tssrc/modules/dealership/entities/dealership.entity.tssrc/modules/dealership/tests/dealership.controller.spec.tssrc/modules/dealership/tests/dealership.service.spec.tssrc/modules/service-bay/entities/service-bay.entity.tssrc/modules/service-bay/service-bay.controller.tssrc/modules/service-bay/service-bay.service.tssrc/modules/service-bay/tests/service-bay.service.spec.tssrc/modules/service-type/constants/service-type.message.tssrc/modules/service-type/service-type.helper.tssrc/modules/service-type/service-type.service.tssrc/modules/service-type/tests/service-type.service.spec.tssrc/modules/technician/entities/technician.entity.tssrc/modules/technician/technician.controller.tssrc/modules/technician/technician.service.tssrc/modules/technician/tests/technician.service.spec.tssrc/modules/vehicle/entities/vehicle.entity.tssrc/modules/vehicle/tests/vehicle.service.spec.tssrc/shared/utils/common.helper.tssrc/shared/utils/date.helper.tstest/appointment.e2e-spec.tstest/jest-e2e.json
💤 Files with no reviewable changes (3)
- src/database/entities/timestamp.entity.ts
- src/modules/dealership/dealership.service.ts
- src/modules/dealership/tests/dealership.controller.spec.ts
🚧 Files skipped from review as they are similar to previous changes (36)
- src/shared/utils/common.helper.ts
- src/modules/appointment/tests/appointment.helper.spec.ts
- src/modules/appointment/constants/appointment.message.ts
- src/modules/appointment/appointment.module.ts
- src/modules/service-type/constants/service-type.message.ts
- src/modules/service-type/tests/service-type.service.spec.ts
- src/modules/service-type/service-type.helper.ts
- src/modules/appointment/interfaces/appointment-search.type.ts
- src/database/entities/base.entity.ts
- .gitignore
- src/main.ts
- test/jest-e2e.json
- src/modules/vehicle/tests/vehicle.service.spec.ts
- src/modules/service-bay/tests/service-bay.service.spec.ts
- src/modules/appointment/appointment.repository.ts
- src/common/filters/exception.filter.ts
- test/appointment.e2e-spec.ts
- src/modules/vehicle/entities/vehicle.entity.ts
- src/modules/service-type/service-type.service.ts
- src/modules/appointment/dtos/get-availability.dto.ts
- src/modules/customer/tests/customer.service.spec.ts
- src/modules/service-bay/service-bay.controller.ts
- src/modules/service-bay/entities/service-bay.entity.ts
- src/modules/dealership/dealership.controller.ts
- src/modules/technician/tests/technician.service.spec.ts
- src/modules/technician/entities/technician.entity.ts
- src/modules/appointment/entities/resource-reservation.entity.ts
- src/modules/dealership/tests/dealership.service.spec.ts
- src/shared/utils/date.helper.ts
- src/modules/technician/technician.service.ts
- src/modules/appointment/appointment.controller.ts
- src/modules/service-bay/service-bay.service.ts
- TASKS.md
- src/modules/technician/technician.controller.ts
- src/modules/appointment/entities/appointment.entity.ts
- src/modules/appointment/appointment.helper.ts
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/appointment/appointment.repository.ts`:
- Around line 15-22: Update reserveResources() and its
`@Transactional`(REQUIRES_NEW) flow so ER_DUP_ENTRY from createAppointment() is
not swallowed after the appointment is persisted: move retry/fallback handling
outside the transactional scope, or rethrow the error from within it, ensuring
failed reservations roll back the appointment and associated ResourceReservation
writes atomically.
In `@src/modules/service-bay/service-bay.controller.ts`:
- Around line 19-21: Update the controller method parameter for the active query
in the service-bay endpoint to use ParseBoolPipe({ optional: true }) so string
query values are parsed explicitly and omitted values remain undefined. Keep
passing the resulting boolean to serviceBayService.findByDealership.
In `@src/modules/technician/technician.controller.ts`:
- Around line 21-27: Update the findByDealership method to explicitly transform
the active query parameter from its runtime string representation into a boolean
before passing it to technicianService.findBy, preserving undefined when the
parameter is omitted.
In `@src/modules/vehicle/entities/vehicle.entity.ts`:
- Around line 20-24: Add a database migration that updates existing vehicle
tables to the new customer_id column schema required by Vehicle’s customer
relation and customerId field. Include the corresponding rollback operation, and
ensure the migration handles existing production databases without relying on
TypeORM synchronization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 50e3f16b-f5d2-4bc6-a41f-5c86ae96225f
📒 Files selected for processing (49)
.gitignoreTASKS.mdsrc/common/filters/exception.filter.tssrc/configs/app.config.tssrc/configs/config.interface.tssrc/database/database.module.tssrc/database/entities/base.entity.tssrc/database/entities/timestamp.entity.tssrc/main.tssrc/modules/appointment/appointment.controller.tssrc/modules/appointment/appointment.helper.tssrc/modules/appointment/appointment.module.tssrc/modules/appointment/appointment.repository.tssrc/modules/appointment/appointment.service.tssrc/modules/appointment/constants/appointment.constant.tssrc/modules/appointment/constants/appointment.message.tssrc/modules/appointment/dtos/create-appointment.dto.tssrc/modules/appointment/dtos/get-availability.dto.tssrc/modules/appointment/dtos/search-appointment.dto.tssrc/modules/appointment/entities/appointment.entity.tssrc/modules/appointment/entities/resource-reservation.entity.tssrc/modules/appointment/interfaces/appointment-search.type.tssrc/modules/appointment/interfaces/resource-reservation.type.tssrc/modules/appointment/tests/appointment.helper.spec.tssrc/modules/appointment/tests/appointment.service.spec.tssrc/modules/customer/tests/customer.service.spec.tssrc/modules/dealership/dealership.controller.tssrc/modules/dealership/dealership.service.tssrc/modules/dealership/entities/dealership.entity.tssrc/modules/dealership/tests/dealership.controller.spec.tssrc/modules/dealership/tests/dealership.service.spec.tssrc/modules/service-bay/entities/service-bay.entity.tssrc/modules/service-bay/service-bay.controller.tssrc/modules/service-bay/service-bay.service.tssrc/modules/service-bay/tests/service-bay.service.spec.tssrc/modules/service-type/constants/service-type.message.tssrc/modules/service-type/service-type.helper.tssrc/modules/service-type/service-type.service.tssrc/modules/service-type/tests/service-type.service.spec.tssrc/modules/technician/entities/technician.entity.tssrc/modules/technician/technician.controller.tssrc/modules/technician/technician.service.tssrc/modules/technician/tests/technician.service.spec.tssrc/modules/vehicle/entities/vehicle.entity.tssrc/modules/vehicle/tests/vehicle.service.spec.tssrc/shared/utils/common.helper.tssrc/shared/utils/date.helper.tstest/appointment.e2e-spec.tstest/jest-e2e.json
💤 Files with no reviewable changes (3)
- src/database/entities/timestamp.entity.ts
- src/modules/dealership/dealership.service.ts
- src/modules/dealership/tests/dealership.controller.spec.ts
🚧 Files skipped from review as they are similar to previous changes (36)
- src/modules/service-type/constants/service-type.message.ts
- src/modules/appointment/interfaces/appointment-search.type.ts
- src/modules/appointment/constants/appointment.message.ts
- src/modules/appointment/dtos/search-appointment.dto.ts
- src/configs/app.config.ts
- src/modules/service-type/service-type.helper.ts
- src/modules/appointment/constants/appointment.constant.ts
- src/modules/appointment/appointment.module.ts
- .gitignore
- src/modules/dealership/entities/dealership.entity.ts
- src/modules/service-type/tests/service-type.service.spec.ts
- src/database/entities/base.entity.ts
- src/main.ts
- src/modules/service-type/service-type.service.ts
- src/database/database.module.ts
- src/modules/appointment/entities/resource-reservation.entity.ts
- src/modules/appointment/tests/appointment.helper.spec.ts
- src/modules/customer/tests/customer.service.spec.ts
- src/modules/service-bay/tests/service-bay.service.spec.ts
- src/modules/dealership/tests/dealership.service.spec.ts
- test/appointment.e2e-spec.ts
- src/modules/service-bay/service-bay.service.ts
- test/jest-e2e.json
- src/common/filters/exception.filter.ts
- src/modules/service-bay/entities/service-bay.entity.ts
- src/modules/appointment/dtos/get-availability.dto.ts
- src/shared/utils/date.helper.ts
- src/modules/appointment/appointment.controller.ts
- src/modules/appointment/entities/appointment.entity.ts
- src/modules/technician/tests/technician.service.spec.ts
- src/modules/vehicle/tests/vehicle.service.spec.ts
- src/modules/technician/entities/technician.entity.ts
- TASKS.md
- src/modules/appointment/appointment.helper.ts
- src/modules/appointment/tests/appointment.service.spec.ts
- src/modules/appointment/appointment.service.ts
Comment on lines
20
to
24
| @ManyToOne(() => Customer, { createForeignKeyConstraints: false }) | ||
| @JoinColumn({ name: 'customer_id' }) | ||
| customer: Customer; | ||
| @Column({ type: 'int', unsigned: true }) | ||
| @Column({ name: 'customer_id', type: 'int', unsigned: true }) | ||
| customerId: number; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a . | rg 'migration|migrations|database|vehicle'
rg -n -C3 "customerId|customer_id" src migrations test 2>/dev/null || trueRepository: hytnht/appointment-scheduler
Length of output: 12415
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files | rg '(^|/)(migration|migrations|database|vehicle|app\.module|data-source|typeorm|ormconfig)'
printf '\n== database module ==\n'
cat -n src/database/database.module.ts
printf '\n== vehicle entity ==\n'
cat -n src/modules/vehicle/entities/vehicle.entity.ts
printf '\n== search for synchronize/migrations config ==\n'
rg -n "synchronize|migrations|migrationRun|autoLoadEntities|typeorm" src test . -g '!node_modules' -g '!dist' || trueRepository: hytnht/appointment-scheduler
Length of output: 14227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app config ==\n'
cat -n src/configs/app.config.ts
printf '\n== any migration files ==\n'
git ls-files | rg '(^|/)(migration|migrations)/|typeorm.*config|data-source'
printf '\n== full vehicle/customer schema references ==\n'
rg -n -C2 "customer_id|customerId" src/modules/customer src/modules/vehicle src/configs src/database test 2>/dev/null || trueRepository: hytnht/appointment-scheduler
Length of output: 2008
Add a migration for customer_id. src/configs/app.config.ts has synchronize: process.env.NODE_ENV !== 'production', and there are no migration files, so this column rename will break existing databases unless the schema is updated separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/vehicle/entities/vehicle.entity.ts` around lines 20 - 24, Add a
database migration that updates existing vehicle tables to the new customer_id
column schema required by Vehicle’s customer relation and customerId field.
Include the corresponding rollback operation, and ensure the migration handles
existing production databases without relying on TypeORM synchronization.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/service-bay/dtos/get-by-dealership.dto.ts`:
- Around line 5-9: Update the active field transformation in the
get-by-dealership DTO so only supported "1" and "true" values convert to true,
while recognized false values retain their intended behavior and unsupported
values such as "garbage" pass through unchanged for `@IsBoolean`() to reject;
preserve undefined as undefined.
In `@src/modules/technician/dtos/get-by-dealership.dto.ts`:
- Around line 5-9: Update the active field transform in the DTO so missing input
returns undefined and unsupported values remain unchanged rather than being
coerced to false. Convert only recognized true values and explicitly supported
false representations, preserving `@IsOptional`() and `@IsBoolean`() validation for
valid inputs.
In `@TASKS.md`:
- Line 150: Update the Task 11 status marker in TASKS.md from the malformed [[✓]
form to the documented [✓] format, preserving the existing dependency text.
- Line 157: Update Task 11 in TASKS.md to replace the typeorm-transaction
instruction with the locked TypeORM queryRunner mechanism for beginning the
atomic booking transaction. Keep the surrounding task steps unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ca89665-e40c-42b7-915b-1d0c834d7640
📒 Files selected for processing (51)
.gitignoreTASKS.mdsrc/common/filters/exception.filter.tssrc/configs/app.config.tssrc/configs/config.interface.tssrc/database/database.module.tssrc/database/entities/base.entity.tssrc/database/entities/timestamp.entity.tssrc/main.tssrc/modules/appointment/appointment.controller.tssrc/modules/appointment/appointment.helper.tssrc/modules/appointment/appointment.module.tssrc/modules/appointment/appointment.repository.tssrc/modules/appointment/appointment.service.tssrc/modules/appointment/constants/appointment.constant.tssrc/modules/appointment/constants/appointment.message.tssrc/modules/appointment/dtos/create-appointment.dto.tssrc/modules/appointment/dtos/get-availability.dto.tssrc/modules/appointment/dtos/search-appointment.dto.tssrc/modules/appointment/entities/appointment.entity.tssrc/modules/appointment/entities/resource-reservation.entity.tssrc/modules/appointment/interfaces/appointment-search.type.tssrc/modules/appointment/interfaces/resource-reservation.type.tssrc/modules/appointment/tests/appointment.helper.spec.tssrc/modules/appointment/tests/appointment.service.spec.tssrc/modules/customer/tests/customer.service.spec.tssrc/modules/dealership/dealership.controller.tssrc/modules/dealership/dealership.service.tssrc/modules/dealership/entities/dealership.entity.tssrc/modules/dealership/tests/dealership.controller.spec.tssrc/modules/dealership/tests/dealership.service.spec.tssrc/modules/service-bay/dtos/get-by-dealership.dto.tssrc/modules/service-bay/entities/service-bay.entity.tssrc/modules/service-bay/service-bay.controller.tssrc/modules/service-bay/service-bay.service.tssrc/modules/service-bay/tests/service-bay.service.spec.tssrc/modules/service-type/constants/service-type.message.tssrc/modules/service-type/service-type.helper.tssrc/modules/service-type/service-type.service.tssrc/modules/service-type/tests/service-type.service.spec.tssrc/modules/technician/dtos/get-by-dealership.dto.tssrc/modules/technician/entities/technician.entity.tssrc/modules/technician/technician.controller.tssrc/modules/technician/technician.service.tssrc/modules/technician/tests/technician.service.spec.tssrc/modules/vehicle/entities/vehicle.entity.tssrc/modules/vehicle/tests/vehicle.service.spec.tssrc/shared/utils/common.helper.tssrc/shared/utils/date.helper.tstest/appointment.e2e-spec.tstest/jest-e2e.json
💤 Files with no reviewable changes (3)
- src/database/entities/timestamp.entity.ts
- src/modules/dealership/dealership.service.ts
- src/modules/dealership/tests/dealership.controller.spec.ts
🚧 Files skipped from review as they are similar to previous changes (40)
- src/modules/appointment/interfaces/appointment-search.type.ts
- src/modules/appointment/constants/appointment.message.ts
- src/shared/utils/common.helper.ts
- src/modules/appointment/tests/appointment.helper.spec.ts
- src/modules/dealership/dealership.controller.ts
- src/modules/appointment/dtos/search-appointment.dto.ts
- src/modules/appointment/dtos/create-appointment.dto.ts
- src/modules/service-type/service-type.service.ts
- src/modules/appointment/dtos/get-availability.dto.ts
- src/modules/service-bay/tests/service-bay.service.spec.ts
- src/main.ts
- .gitignore
- src/modules/service-type/tests/service-type.service.spec.ts
- src/modules/service-type/constants/service-type.message.ts
- src/modules/vehicle/entities/vehicle.entity.ts
- src/configs/config.interface.ts
- src/modules/dealership/tests/dealership.service.spec.ts
- src/database/entities/base.entity.ts
- src/modules/customer/tests/customer.service.spec.ts
- src/modules/appointment/entities/resource-reservation.entity.ts
- src/modules/appointment/appointment.repository.ts
- src/modules/appointment/constants/appointment.constant.ts
- src/database/database.module.ts
- src/modules/service-bay/service-bay.service.ts
- src/modules/service-bay/entities/service-bay.entity.ts
- src/modules/technician/entities/technician.entity.ts
- src/shared/utils/date.helper.ts
- src/modules/vehicle/tests/vehicle.service.spec.ts
- src/modules/appointment/appointment.module.ts
- src/modules/appointment/appointment.helper.ts
- test/appointment.e2e-spec.ts
- src/modules/technician/technician.service.ts
- src/modules/technician/technician.controller.ts
- src/modules/appointment/entities/appointment.entity.ts
- src/modules/dealership/entities/dealership.entity.ts
- src/modules/technician/tests/technician.service.spec.ts
- src/common/filters/exception.filter.ts
- src/modules/appointment/tests/appointment.service.spec.ts
- src/modules/appointment/appointment.controller.ts
- src/modules/appointment/appointment.service.ts
Comment on lines
+5
to
+9
| @IsOptional() | ||
| @IsBoolean() | ||
| @Type(() => String) | ||
| @Transform(({ value }) => ['1', 'true'].includes(value)) | ||
| active?: boolean; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file =="
sed -n '1,120p' src/modules/service-bay/dtos/get-by-dealership.dto.ts
echo
echo "== search ValidationPipe =="
rg -n "ValidationPipe|transform: true|enableImplicitConversion|class-transformer|class-validator" src test . -g '!node_modules' | head -n 200Repository: hytnht/appointment-scheduler
Length of output: 5243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/main.ts =="
sed -n '1,120p' src/main.ts
echo
echo "== src/modules/technician/dtos/get-by-dealership.dto.ts =="
sed -n '1,120p' src/modules/technician/dtos/get-by-dealership.dto.ts
echo
echo "== node probe: dependency availability =="
node - <<'JS'
for (const mod of ['class-transformer', 'class-validator']) {
try {
const resolved = require.resolve(mod);
console.log(mod + ': ' + resolved);
} catch (e) {
console.log(mod + ': NOT_FOUND');
}
}
JSRepository: hytnht/appointment-scheduler
Length of output: 2550
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
require('reflect-metadata');
const { plainToInstance, Transform, Type } = require('class-transformer');
const { validateSync, IsBoolean, IsOptional } = require('class-validator');
class GetServiceBayQuery {}
// Mimic decorator application order from TypeScript (bottom-up).
Transform(({ value }) => ['1', 'true'].includes(value))(GetServiceBayQuery.prototype, 'active');
Type(() => String)(GetServiceBayQuery.prototype, 'active');
IsBoolean()(GetServiceBayQuery.prototype, 'active');
IsOptional()(GetServiceBayQuery.prototype, 'active');
const inputs = [
undefined,
null,
'',
'1',
'true',
'false',
'0',
'garbage',
true,
false,
1,
0,
];
for (const value of inputs) {
const inst = plainToInstance(GetServiceBayQuery, { active: value }, {
enableImplicitConversion: true,
});
const errors = validateSync(inst, { stopAtFirstError: true });
console.log(JSON.stringify({
input: value,
transformed: inst.active,
transformedType: typeof inst.active,
valid: errors.length === 0,
errors: errors.map(e => e.constraints),
}));
}
JSRepository: hytnht/appointment-scheduler
Length of output: 1206
Preserve unsupported active values
@Transform turns any non-"1"/"true" input into false, so ?active=garbage passes @IsBoolean() and filters inactive bays. Return unknown values unchanged (and keep undefined as-is) so validation can reject them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/service-bay/dtos/get-by-dealership.dto.ts` around lines 5 - 9,
Update the active field transformation in the get-by-dealership DTO so only
supported "1" and "true" values convert to true, while recognized false values
retain their intended behavior and unsupported values such as "garbage" pass
through unchanged for `@IsBoolean`() to reject; preserve undefined as undefined.
Comment on lines
+5
to
+9
| @IsOptional() | ||
| @IsBoolean() | ||
| @Type(() => String) | ||
| @Transform(({ value }) => ['1', 'true'].includes(value)) | ||
| active?: boolean; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
echo "== file =="
sed -n '1,120p' src/modules/technician/dtos/get-by-dealership.dto.ts
echo "\n== search similar patterns =="
rg -n "`@Transform`\\(|`@IsBoolean`\\(|`@Type`\\(\\) => String|active\\?: boolean" src -g '*.ts'
echo "\n== validation pipe / transform config =="
rg -n "ValidationPipe|transform:|enableImplicitConversion|whitelist|forbidNonWhitelisted" src -g '*.ts'Repository: hytnht/appointment-scheduler
Length of output: 1808
Keep unsupported active values unchanged @Transform maps every non-'1'/'true' input to false, so invalid strings and a missing query param both pass @IsBoolean() and filter inactive technicians. Return undefined for missing input and leave unknown values untouched; map accepted false values explicitly if needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/technician/dtos/get-by-dealership.dto.ts` around lines 5 - 9,
Update the active field transform in the DTO so missing input returns undefined
and unsupported values remain unchanged rather than being coerced to false.
Convert only recognized true values and explicitly supported false
representations, preserving `@IsOptional`() and `@IsBoolean`() validation for valid
inputs.
| ## Task 11 — Booking transaction | ||
|
|
||
| `[ ]` _(depends on: Task 10)_ | ||
| `[[✓]` _(depends on: Task 10)_ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the malformed Task 11 status marker.
[[✓] has an extra opening bracket and does not match the documented status format. Change it to [✓].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@TASKS.md` at line 150, Update the Task 11 status marker in TASKS.md from the
malformed [[✓] form to the documented [✓] format, preserving the existing
dependency text.
| 2. Compute slots from validated `startAt` | ||
| 3. Verify ≥1 qualified tech at dealership → 422 if none | ||
| 4. Begin transaction | ||
| 4. Begin transaction with typeorm-transaction |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show targeted sections with line numbers
sed -n '145,205p' TASKS.md | cat -n
# Search for relevant references in TASKS.md
rg -n "typeorm-transaction|queryRunner|transaction" TASKS.mdRepository: hytnht/appointment-scheduler
Length of output: 3416
Use one authoritative transaction mechanism in TASKS.md
Task 11 still says to begin the booking transaction with typeorm-transaction, but the locked decisions specify TypeORM queryRunner for the atomic booking txn. Align the task text with the locked decision so the implementation doesn’t drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@TASKS.md` at line 157, Update Task 11 in TASKS.md to replace the
typeorm-transaction instruction with the locked TypeORM queryRunner mechanism
for beginning the atomic booking transaction. Keep the surrounding task steps
unchanged.
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes