Conversation
Added PaymentGateway component to invoice detail page allowing customers to pay outstanding invoices online. Includes user profile loading for auto-filling payment details and complete payment success/error handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Updated TimeLogResponse property references to match backend contract: - logId → id - workDate → date - hoursWorked → hours Resolves TypeScript compilation errors in production build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…rt Next.js server
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughMoves API base URL resolution from build-time to runtime via a generated Changes
Sequence Diagram(s)sequenceDiagram
participant Container as Container
participant Entrypoint as entrypoint.sh
participant Public as /public/runtime-config.js
participant Browser as Browser
participant Layout as RootLayout
participant APIClient as apiClient
Container->>Entrypoint: container start
Entrypoint->>Public: write runtime-config.js (NEXT_PUBLIC_API_BASE_URL)
Entrypoint->>Container: exec node server.js
Browser->>Layout: request page
Layout->>Public: load `/runtime-config.js` (beforeInteractive)
Public->>Browser: defines window.__RUNTIME_CONFIG__
Browser->>APIClient: initialize / call getRuntimeConfig()
APIClient->>Browser: read window.__RUNTIME_CONFIG__ → use base URL
sequenceDiagram
participant App as React App
participant NotifCtx as NotificationProvider
participant WS as websocketService (STOMP/SockJS)
participant Server as Backend WS
App->>NotifCtx: mount (userId)
NotifCtx->>WS: connect(userId, onNotification, onUnreadCount)
WS->>Server: open STOMP over SockJS -> subscribe /user/{id}/queue/notifications
Server->>WS: send notification message
WS->>NotifCtx: invoke onNotification(payload)
NotifCtx->>App: prepend notification, update unreadCount, show toast
App->>NotifCtx: markAsRead / fetchNotifications (REST) -> updates state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
entrypoint.sh (1)
5-9: Consider adding error handling and validation.The script generates the runtime config without validating the NEXT_PUBLIC_API_BASE_URL value or checking if the write operation succeeds. While this may be acceptable for a controlled environment, adding basic validation would improve robustness.
Apply this diff to add validation:
+# Validate that the target directory is writable +if [ ! -w /app/public ]; then + echo "ERROR: /app/public is not writable" >&2 + exit 1 +fi + # Generate runtime configuration file cat > /app/public/runtime-config.js <<EOF window.__RUNTIME_CONFIG__ = { NEXT_PUBLIC_API_BASE_URL: "${NEXT_PUBLIC_API_BASE_URL:-http://localhost:8080}" }; EOF + +# Verify the file was created +if [ ! -f /app/public/runtime-config.js ]; then + echo "ERROR: Failed to create runtime-config.js" >&2 + exit 1 +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/build.yaml(0 hunks)Dockerfile(2 hunks)entrypoint.sh(1 hunks)src/app/layout.tsx(2 hunks)src/config/runtime.ts(1 hunks)src/lib/apiClient.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/build.yaml
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/apiClient.ts (1)
src/config/runtime.ts (1)
config(24-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (3)
entrypoint.sh (1)
15-15: Good use of exec for PID 1 signal handling.Using
execensures thatnode server.jsbecomes PID 1 and receives signals directly, which is important for graceful shutdown in containers.Dockerfile (1)
47-49: Entrypoint integration looks correct.The entrypoint script is properly integrated:
- Copied and made executable before ownership transfer (line 47-49)
- Ownership is transferred to
nextjs:nodejs(line 52), which includes the entrypoint- Runs as non-root user
nextjs(line 55)- Has write access to
/app/public/for generating runtime-config.jsThe permission flow is correct for secure container execution.
Also applies to: 65-65
src/config/runtime.ts (1)
12-22: getRuntimeConfig implementation is correct.The function properly checks for browser environment and falls back to environment variables with a sensible default.
| {/* Runtime configuration - injected at container startup */} | ||
| <Script src="/runtime-config.js" strategy="beforeInteractive" /> |
There was a problem hiding this comment.
🧩 Analysis chain
Script strategy doesn't guarantee availability for module-level code.
While strategy="beforeInteractive" ensures the script loads before hydration, it doesn't guarantee execution before module-level code (like the constant assignment in apiClient.ts line 6). Modules are evaluated when imported, which happens during SSR or when the bundle is parsed, before this Script executes.
This means code that reads the runtime config at module level will still get the fallback value, not the runtime value.
Verify that the API client is not using the fallback URL by checking network requests in the browser after deployment:
The solution requires lazy evaluation in consuming code (see comment on apiClient.ts lines 5-6).
🏁 Script executed:
#!/bin/bash
# Search for module-level constant assignments that read from runtime config
rg -n -C3 'const.*=.*config\.' --type=ts --type=tsxLength of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for module-level constant assignments that read from runtime config
# Using corrected syntax without problematic --type flag combination
rg -n -C3 'const\s+\w+\s*=\s*.*config' --type=ts -g '*.tsx'
Length of output: 559
🏁 Script executed:
#!/bin/bash
# Directly check apiClient.ts for module-level config usage
cat src/app/apiClient.ts | head -20Length of output: 52
🏁 Script executed:
#!/bin/bash
# Search for any runtime config access patterns
rg -n 'window\..*config|globalThis\..*config' -g '*.ts' -g '*.tsx'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check what files exist in src/app directory
fd -t f --extension ts --extension tsx src/appLength of output: 333
---
🏁 Script executed:
```shell
#!/bin/bash
# Find layout.tsx files
fd -t f -e tsx -e ts layout
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Find apiClient files anywhere
fd -t f -e ts -e tsx apiClientLength of output: 20
🏁 Script executed:
#!/bin/bash
# List files in src/app with correct fd syntax
fd . src/app -t fLength of output: 2053
🏁 Script executed:
#!/bin/bash
# Check the actual layout.tsx mentioned in review comment
find . -name "layout.tsx" -path "*/app/*" | head -5Length of output: 51
🏁 Script executed:
#!/bin/bash
# Search for Script imports and runtime-config.js references
rg -n "runtime-config|Script.*beforeInteractive" Length of output: 203
---
🏁 Script executed:
```shell
#!/bin/bash
# Check src/lib/apiClient.ts content, especially around line 6
cat src/lib/apiClient.ts | head -20
Length of output: 650
🏁 Script executed:
#!/bin/bash
# Check src/app/layout.tsx around lines 33-34
cat src/app/layout.tsx | sed -n '25,45p'Length of output: 443
🏁 Script executed:
#!/bin/bash
# Search for any config access in apiClient.ts
rg -n -A2 -B2 'config|CONFIG' src/lib/apiClient.tsLength of output: 689
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the config/runtime file
fd -t f -e ts -e tsx runtime | grep -i config
Length of output: 21
🏁 Script executed:
#!/bin/bash
# Search for config directory
find src -type d -name configLength of output: 41
🏁 Script executed:
#!/bin/bash
# Look for runtime config pattern
rg -n "export.*config|fallback" --type=ts -g '*runtime*' -g '*config*'Length of output: 110
🏁 Script executed:
#!/bin/bash
# Check all imports from @/config
rg -n "from '@/config" --type=ts --type=tsxLength of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the full src/config/runtime.ts file
cat src/config/runtime.ts
Length of output: 617
---
🏁 Script executed:
```shell
#!/bin/bash
# Read entrypoint.sh to see what runtime-config.js contains
cat entrypoint.sh
Length of output: 387
Module-level config access prevents runtime URL override from taking effect.
The review comment correctly identifies a real timing issue. The client-side bundle parses and evaluates apiClient.ts line 6 before the Script tag executes, causing config to be set during module load rather than after window.__RUNTIME_CONFIG__ is available. This means API_BASE_URL will always use the build-time fallback value, not the container-injected runtime value from entrypoint.sh.
While getRuntimeConfig() has logic to check window.__RUNTIME_CONFIG__, the module-level constant assignment bypasses this:
export const config = getRuntimeConfig(); // Evaluated at bundle parse time
const API_BASE_URL = config.NEXT_PUBLIC_API_BASE_URL; // Before Script runs
The solution requires lazy evaluation—either access the runtime config at the point of use or use a getter function instead of a constant.
🤖 Prompt for AI Agents
In src/app/layout.tsx around lines 33-34, the runtime config Script is loaded
after module evaluation causing apiClient.ts's module-level config to be read
too early; change to lazy-evaluate the runtime config by removing any
module-level constant that calls getRuntimeConfig at import time and instead
call getRuntimeConfig (or expose a getApiBaseUrl() getter) at the point of use
(e.g., inside client functions or request constructors) so
window.__RUNTIME_CONFIG__ is read after the injected /runtime-config.js Script
runs; ensure no code relies on API_BASE_URL as a top-level constant and update
imports/call sites to use the getter or an inline call to getRuntimeConfig().
| }; | ||
| } | ||
|
|
||
| export const config = getRuntimeConfig(); |
There was a problem hiding this comment.
Module-level evaluation undermines runtime configuration.
Exporting config as a constant that's evaluated at module load time (line 24) means getRuntimeConfig() is called immediately when this module is imported, before the runtime-config.js Script executes. This causes consumers like apiClient.ts to receive the fallback value instead of the runtime value.
Consider one of these solutions:
Option 1 (Recommended): Export only the function and let consumers call it when needed
-export const config = getRuntimeConfig();
+// Export only the function - consumers should call it when needed
+// This ensures the latest runtime config is always readThen update apiClient.ts to call the function instead of using a module-level constant.
Option 2: Make config a getter
-export const config = getRuntimeConfig();
+// Getter ensures config is read dynamically
+export const config = {
+ get NEXT_PUBLIC_API_BASE_URL() {
+ return getRuntimeConfig().NEXT_PUBLIC_API_BASE_URL;
+ }
+};Option 3: Document the limitation and use memoization
If the fallback during SSR is intentional, document this behavior and consider memoization to avoid repeated calls while still allowing the config to update after initial load.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/config/runtime.ts at line 24: exporting a module-level constant config =
getRuntimeConfig() causes getRuntimeConfig() to run at import time and return
fallback before runtime-config.js runs; change the module to export the
getRuntimeConfig function (or export a getter) instead and update all consumers
(e.g., src/apiClient.ts) to call the function when they need the config so
runtime values are read after runtime-config.js executes; if you prefer keeping
a cached value, implement memoization inside the function so the first runtime
call caches the result without invoking getRuntimeConfig() at module load.
| // Use runtime configuration instead of build-time env vars | ||
| const API_BASE_URL = config.NEXT_PUBLIC_API_BASE_URL; |
There was a problem hiding this comment.
Module-level evaluation defeats runtime configuration.
The runtime config is accessed at module load time (line 6), but the Script tag in layout.tsx that populates window.__RUNTIME_CONFIG__ executes later during rendering. This creates a race condition:
- During SSR:
window.__RUNTIME_CONFIG__doesn't exist, so the fallback is used - During client initial load: If this module loads before the Script executes, the fallback is used instead of the runtime value
This defeats the purpose of runtime configuration. The API client will likely use http://localhost:8080 (the default fallback) instead of the actual runtime value.
Consider lazy evaluation to ensure the runtime config is read when needed, not at module load:
-// Use runtime configuration instead of build-time env vars
-const API_BASE_URL = config.NEXT_PUBLIC_API_BASE_URL;
+// Use runtime configuration instead of build-time env vars
+const getAPIBaseURL = () => {
+ if (typeof window !== 'undefined' && window.__RUNTIME_CONFIG__?.NEXT_PUBLIC_API_BASE_URL) {
+ return window.__RUNTIME_CONFIG__.NEXT_PUBLIC_API_BASE_URL;
+ }
+ return process.env.NEXT_PUBLIC_API_BASE_URL || 'http://localhost:8080';
+};
const apiClient = axios.create({
- baseURL: API_BASE_URL,
+ baseURL: getAPIBaseURL(),
headers: { 'Content-Type': 'application/json' },
});Alternatively, update the baseURL after the runtime config loads, or use axios defaults that can be modified later.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lib/apiClient.ts around lines 5-6, the module currently reads runtime
config at module load time which can occur before the layout Script sets
window.__RUNTIME_CONFIG__, causing a stale fallback to be used; change this to
lazy evaluation by removing the module-level constant and instead read
config.NEXT_PUBLIC_API_BASE_URL at request time — implement a getApiBaseUrl()
function that returns the runtime value (with fallback) and use it when creating
the axios instance or when making requests, or provide and call a
setApiBaseUrl(base) that updates axios.defaults.baseURL after the runtime script
runs so the client always uses the actual runtime config.
Notification UI
… and update related types
Enhance dashboards and integrate payment gateway
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/dashboard/appointments/[appointmentId]/page.tsx (1)
392-521: UseuserIdfor assigned employee identifiers
assignedEmployeeIds/employeeIdsare actual user IDs (the later checkappointment.assignedEmployeeIds?.includes(userId)confirms it). Here we store/checkemployee.username, so existing assignments never appear selected and the API receives usernames instead of IDs. That drops current assignees and submits invalid data.Switch the lookup/checkbox logic to
userId, e.g.- const emp = employees.find((e) => e.username === empId) + const emp = employees.find((e) => e.userId === empId) ... - checked={selectedEmployeeIds.includes(employee.username)} - onChange={() => toggleEmployeeSelection(employee.username)} + checked={selectedEmployeeIds.includes(employee.userId)} + onChange={() => toggleEmployeeSelection(employee.userId)}Keep everything in
selectedEmployeeIdsas IDs sohandleAssignEmployeessends the correct payload and the UI remains in sync.
🧹 Nitpick comments (6)
src/app/components/EditVehicleForm.tsx (1)
95-104: Remove redundantdisabledattribute and unnecessaryonChangehandler.The input has both
disabledandreadOnlyattributes, which is redundant. For a license plate identifier that should remain visible but not editable,readOnlyalone is sufficient. Additionally, theonChangehandler on line 99 serves no purpose since the field cannot be changed.Apply this diff:
<div> <label className="block text-sm font-medium mb-2 theme-text-secondary">License Plate</label> <input type="text" name="licensePlate" value={formData.licensePlate} - onChange={handleChange} className="w-full px-4 py-3 border rounded-xl focus:ring-2 focus:ring-blue-500 transition-all theme-bg-primary theme-text-primary automotive-border opacity-60 cursor-not-allowed" placeholder="e.g., ABC-1234" - disabled readOnly /> </div>src/app/components/dashboards/CustomerDashboard.tsx (1)
240-242: Consider focus management when opening the chat.When the chat opens, focus should move to the chat panel for better keyboard navigation and screen reader support.
+ const chatPanelRef = useRef<HTMLDivElement>(null); + + useEffect(() => { + if (chatOpen && chatPanelRef.current) { + chatPanelRef.current.focus(); + } + }, [chatOpen]); // In the render: - <div className="bg-white dark:bg-gray-800 rounded-lg shadow-2xl..."> + <div ref={chatPanelRef} tabIndex={-1} className="bg-white dark:bg-gray-800 rounded-lg shadow-2xl...">src/services/adminService.ts (1)
176-180: Clarify the comment to match the actual implementation.The comment describes a triple-nested structure (
res.data.data.data) but the implementation uses a different approach: it extractspaginatedDataand then conditionally accesses.dataonly if it's not already an array. This defensive pattern is good, but the comment is misleading.Update the comment to accurately describe the logic:
- // Backend returns ApiResponse<PaginatedResponse<AuditLogResponse>> - // So we need: res.data.data.data (ApiResponse.data -> PaginatedResponse.data -> array) + // Backend may return ApiResponse<PaginatedResponse<AuditLogResponse>> or ApiResponse<AuditLogResponse[]> + // Extract the paginated data wrapper, then get the inner array if present const paginatedData = res.data.data || res.data; return Array.isArray(paginatedData) ? paginatedData : (paginatedData.data || []);src/app/dashboard/projects/request/page.tsx (1)
93-93: Remove!isCustomerfrom the loading condition.Including
!isCustomerin the loading guard causes the skeleton to display during the redirect, which may create a visual flash before navigation completes. The redirectuseEffect(Lines 23-27) already handles non-customers, so this check is redundant here.Apply this diff:
- if (profileLoading || fetchingVehicles || !isCustomer) { + if (profileLoading || fetchingVehicles) {src/app/dashboard/projects/page.tsx (1)
39-40: Remove unusedisAdminUservariable.The variable is computed but never referenced in the component. This is flagged by static analysis.
Apply this diff:
- // Check if user is admin or super admin - const isAdminUser = roles.includes('SUPER_ADMIN') || roles.includes('ADMIN') -src/app/dashboard/projects/[projectId]/page.tsx (1)
19-19: Remove unused rejection state
rejectionReason/setRejectionReasonare never read—static analysis flags them, and they add noise. Please drop the state (or wire it into the UI if you still need it).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (37)
package.json(2 hunks)src/app/components/EditVehicleForm.tsx(2 hunks)src/app/components/NotificationBell.tsx(4 hunks)src/app/components/dashboards/CustomerDashboard.tsx(3 hunks)src/app/components/dashboards/EmployeeDashboard.tsx(3 hunks)src/app/contexts/NotificationContext.tsx(1 hunks)src/app/dashboard/admin/audit-logs/page.tsx(5 hunks)src/app/dashboard/admin/config/page.tsx(3 hunks)src/app/dashboard/admin/page.tsx(3 hunks)src/app/dashboard/admin/reports/page.tsx(4 hunks)src/app/dashboard/admin/service-types/page.tsx(3 hunks)src/app/dashboard/admin/users/page.tsx(6 hunks)src/app/dashboard/appointments/[appointmentId]/page.tsx(7 hunks)src/app/dashboard/appointments/book/page.tsx(2 hunks)src/app/dashboard/appointments/page.tsx(3 hunks)src/app/dashboard/invoices/[invoiceId]/page.tsx(5 hunks)src/app/dashboard/invoices/page.tsx(1 hunks)src/app/dashboard/layout.tsx(4 hunks)src/app/dashboard/payments/page.tsx(2 hunks)src/app/dashboard/projects/[projectId]/page.tsx(5 hunks)src/app/dashboard/projects/page.tsx(4 hunks)src/app/dashboard/projects/request/page.tsx(3 hunks)src/app/dashboard/schedule/page.tsx(4 hunks)src/app/dashboard/time-logs/page.tsx(13 hunks)src/app/globals.css(1 hunks)src/lib/apiClient.ts(2 hunks)src/services/adminService.ts(1 hunks)src/services/appointmentService.ts(2 hunks)src/services/projectService.ts(3 hunks)src/services/timeLoggingService.ts(1 hunks)src/services/websocketService.ts(1 hunks)src/types/admin.ts(1 hunks)src/types/appointment.ts(3 hunks)src/types/payment.ts(2 hunks)src/types/project.ts(2 hunks)src/types/timeLogging.ts(1 hunks)src/types/vehicle.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/apiClient.ts
🧰 Additional context used
🧬 Code graph analysis (19)
src/app/dashboard/admin/config/page.tsx (1)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)
src/app/dashboard/admin/page.tsx (1)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)
src/services/appointmentService.ts (1)
src/types/appointment.ts (3)
StatusUpdateDto(40-42)AppointmentResponseDto(15-30)AssignEmployeesRequestDto(133-135)
src/app/dashboard/time-logs/page.tsx (2)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)src/types/timeLogging.ts (2)
TimeLogResponse(19-30)TimeLogRequest(3-10)
src/app/dashboard/appointments/page.tsx (1)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)
src/app/dashboard/schedule/page.tsx (1)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)
src/app/dashboard/admin/users/page.tsx (5)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)src/types/admin.ts (1)
UserResponse(4-17)src/types/api.ts (2)
CreateEmployeeRequest(19-26)CreateAdminRequest(28-34)src/services/authService.ts (1)
authService(13-92)src/services/adminService.ts (1)
adminService(16-186)
src/app/contexts/NotificationContext.tsx (2)
src/types/notification.ts (1)
NotificationResponse(3-14)src/services/notificationService.ts (4)
notificationService(12-60)markAsRead(28-33)markAllAsRead(54-59)deleteNotification(36-39)
src/app/dashboard/layout.tsx (2)
src/app/contexts/DashboardContext.tsx (2)
DashboardProvider(15-49)useDashboard(51-57)src/app/contexts/NotificationContext.tsx (1)
NotificationProvider(34-200)
src/app/dashboard/appointments/[appointmentId]/page.tsx (6)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)src/types/admin.ts (1)
UserResponse(4-17)src/types/payment.ts (2)
InvoiceItemDto(81-87)CreateInvoiceDto(89-96)src/services/appointmentService.ts (1)
appointmentService(16-134)src/services/adminService.ts (1)
adminService(16-186)src/services/paymentService.ts (1)
paymentService(15-94)
src/app/dashboard/projects/page.tsx (2)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)src/types/project.ts (1)
ProjectResponseDto(59-79)
src/app/dashboard/appointments/book/page.tsx (1)
src/services/appointmentService.ts (1)
appointmentService(16-134)
src/app/components/NotificationBell.tsx (3)
src/app/contexts/NotificationContext.tsx (1)
useNotifications(21-27)src/services/notificationService.ts (2)
markAsRead(28-33)markAllAsRead(54-59)src/services/websocketService.ts (1)
isConnected(123-125)
src/app/dashboard/projects/request/page.tsx (2)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)src/types/vehicle.ts (1)
VehicleListItem(18-27)
src/app/dashboard/admin/reports/page.tsx (2)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)src/types/admin.ts (1)
ReportRequest(97-106)
src/app/dashboard/admin/audit-logs/page.tsx (3)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)src/types/admin.ts (1)
AuditLogResponse(123-134)src/services/adminService.ts (1)
adminService(16-186)
src/app/dashboard/admin/service-types/page.tsx (1)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)
src/app/dashboard/projects/[projectId]/page.tsx (3)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(51-57)src/types/project.ts (1)
ProjectResponseDto(59-79)src/services/projectService.ts (1)
projectService(16-137)
src/app/dashboard/invoices/[invoiceId]/page.tsx (2)
src/services/userService.ts (1)
userService(21-222)src/app/components/PaymentGateway.tsx (1)
PaymentGateway(34-154)
🪛 GitHub Actions: Build and Test Frontend_Web
src/app/dashboard/admin/users/page.tsx
[error] 80-80: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
🪛 GitHub Actions: Build, Test, and Package Frontend
src/app/dashboard/admin/users/page.tsx
[error] 80-80: ESLint: Unexpected any. Specify a different type (@typescript-eslint/no-explicit-any).
🪛 GitHub Check: Build and Test Frontend
src/app/dashboard/payments/page.tsx
[failure] 114-114:
Do not use an <a> element to navigate to /dashboard/invoices/. Use <Link /> from next/link instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages
src/services/websocketService.ts
[failure] 31-31:
Unexpected any. Specify a different type
[failure] 5-5:
Unexpected any. Specify a different type
src/app/dashboard/admin/users/page.tsx
[failure] 80-80:
Unexpected any. Specify a different type
src/app/dashboard/projects/page.tsx
[warning] 40-40:
'isAdminUser' is assigned a value but never used
src/app/dashboard/projects/[projectId]/page.tsx
[warning] 19-19:
'setRejectionReason' is assigned a value but never used
[warning] 19-19:
'rejectionReason' is assigned a value but never used
🪛 GitHub Check: Install, Lint and Build
src/app/dashboard/payments/page.tsx
[failure] 114-114:
Do not use an <a> element to navigate to /dashboard/invoices/. Use <Link /> from next/link instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages
src/services/websocketService.ts
[failure] 31-31:
Unexpected any. Specify a different type
[failure] 5-5:
Unexpected any. Specify a different type
src/app/dashboard/admin/users/page.tsx
[failure] 80-80:
Unexpected any. Specify a different type
src/app/dashboard/projects/page.tsx
[warning] 40-40:
'isAdminUser' is assigned a value but never used
src/app/dashboard/projects/[projectId]/page.tsx
[warning] 19-19:
'setRejectionReason' is assigned a value but never used
[warning] 19-19:
'rejectionReason' is assigned a value but never used
🔇 Additional comments (53)
src/app/globals.css (1)
209-227: Animation additions are well-designed and consistent.The new
slideInRightkeyframe animation and.animate-slide-in-rightutility class follow established patterns in the file. The timing (0.4s with ease-out) and transform values (translateX from 100% to 0) are appropriate for a slide-in effect. The implementation is clean and requires no changes.src/app/components/EditVehicleForm.tsx (1)
57-59: Good UX improvement!Displaying the license plate in the heading helps users clearly identify which vehicle they're editing.
src/app/components/dashboards/CustomerDashboard.tsx (2)
23-23: LGTM!The chat state initialization is clean and appropriate for toggling the widget visibility.
11-11: Import statement verified as correct.The AIChatWidget component exists at
src/app/components/chatbot/AIChatWidget.tsx, the import path resolves correctly via the@/alias configuration, and the component is properly used without props at line 241 as required.src/types/vehicle.ts (1)
20-20: LGTM! Consistent field addition.Adding
customerIdtoVehicleListItemaligns it with the fullVehicleinterface and enables customer identification in list views.src/services/timeLoggingService.ts (1)
18-19: LGTM! Helpful documentation.These inline comments clearly document the backend's role-based filtering behavior, which aids understanding of how
getMyTimeLogsworks for different user roles.src/app/dashboard/schedule/page.tsx (5)
6-6: LGTM! DashboardContext integration.Properly imports and consumes the DashboardContext for role-based access control.
Also applies to: 9-9
82-86: LGTM! Clean role checking implementation.The role checking logic is concise and follows a clear pattern with
hasRolehelper and derivedhasAccess/isAdminflags.
87-96: LGTM! Proper loading state handling.Combining
rolesLoadingandloadingensures the UI doesn't render prematurely while either roles or data are being fetched.
98-118: LGTM! Clear access denial UI.The Access Denied block provides clear messaging about who can access this page, following a consistent pattern with other protected pages in the codebase.
123-128: LGTM! Role-aware UI text.The conditional rendering of headers and empty state messages appropriately distinguishes between admin and employee views.
Also applies to: 215-217
src/app/dashboard/payments/page.tsx (1)
45-53: LGTM! Enhanced error handling.The 403 status check provides specific, user-friendly error messages for access denial scenarios, improving the UX.
src/types/timeLogging.ts (2)
32-50: LGTM! Summary types added.The new
TimeLogSummaryItemandTimeLogSummaryResponsetypes enable time log aggregation and reporting features with comprehensive breakdown support.
4-9: Field name migrations verified and coordinated correctly.The type changes in
TimeLogRequestare properly implemented across the codebase:
- Form state in
src/app/dashboard/time-logs/page.tsxcorrectly binds to new field names (hours,date,workType)handleSubmit(line 59) passesformDatadirectly totimeLoggingService.createTimeLog(formData)- Service layer expects
TimeLogRequestwith the new structure and posts to/time-logs- No old field name references found in service layer code
The HTML element IDs (
hoursWorked,workDate,taskType) are DOM identifiers for form labeling and don't affect the actual data being sent. The data payload uses the correct new field names throughout.src/app/components/dashboards/EmployeeDashboard.tsx (3)
42-44: LGTM! Updated validation check.The filter now correctly uses
log.idto validate time log entries, aligning with the updatedTimeLogResponsetype.
63-73: LGTM! Consistent field name updates.The
totalHoursThisWeekandworkSummaryByDaycalculations have been updated to use the new field names (hours,date) with proper numeric conversion and fallback handling.
204-209: LGTM! Updated rendering logic.The time log rendering now uses the updated field names (
id,date,hours) with appropriate fallback for the key prop and safe numeric conversion.src/app/dashboard/time-logs/page.tsx (8)
6-6: LGTM! DashboardContext integration.Properly imports and uses
useDashboardto obtain roles, loading state, and profile for role-based access control and filtering.Also applies to: 9-9, 24-25
10-10: LGTM! Enhanced state management for admin filtering.The refactored state (
allTimeLogs,viewFilter,filteredLogs) enables admins to toggle between viewing their own logs and all team logs, while non-admins always see their own.Also applies to: 13-13, 49-51
14-20: LGTM! Coordinated field name updates.The form state and UI have been updated to use the new field names (
serviceId,hours,date,workType) consistent with the updatedTimeLogRequesttype. MakingserviceIdrequired improves data integrity.Also applies to: 60-66, 190-244
35-39: LGTM! Robust data validation.Filtering for logs with valid
idfields prevents rendering errors from malformed data.
102-133: LGTM! Consistent access control pattern.The loading state handling and Access Denied UI follow the same pattern used in other protected pages, providing a consistent user experience.
152-175: LGTM! Admin-only filter tabs.The view filter UI is appropriately gated behind the
isAdmincheck and provides clear navigation between "My Time Logs" and "All Time Logs".
332-334: LGTM! Role-aware table rendering.The conditional Employee column (visible only in "all" view) and per-row delete permission checks (only for own logs) implement appropriate access controls.
Also applies to: 360-364, 369-378
347-382: LGTM! Updated field references.All time log property accesses have been correctly updated to use the new field names (
id,date,hours,workType) consistent with the updatedTimeLogResponsetype.package.json (1)
12-18: WebSocket dependencies are up-to-date with no known vulnerabilities.@stomp/stompjs (v7.2.1) is the latest stable release, and sockjs-client (v1.6.1) is also at its latest available version. Security advisories query returned no vulnerabilities for either package.
src/types/payment.ts (2)
120-128: LGTM! Well-designed part-payment tracking fields.The deposit and final payment tracking fields are comprehensive and properly optional. This design cleanly supports the 50-50 payment workflow mentioned in the summary.
89-96: No breaking changes detected—all invoice creation calls properly updated.The single invoice creation call in the codebase (
src/app/dashboard/appointments/[appointmentId]/page.tsx, line 300) correctly provides the requiredserviceOrProjectIdfield. No lingering references to the old optionalserviceId?andprojectId?structure exist in invoice-related code.src/app/dashboard/invoices/[invoiceId]/page.tsx (1)
178-193: LGTM! Payment handlers are well-structured.The success, error, and cancel handlers properly manage state, refresh invoice data, and provide clear user feedback. The async reload in
handlePaymentSuccessensures the UI reflects the updated payment status.src/types/project.ts (2)
63-63: LGTM! Well-integrated optional field.The optional
appointmentIdfield properly links projects to appointments, supporting the enhanced workflow mentioned in the summary.
81-91: LGTM! Status literals properly extend the type system.The new status literals (
PENDING_ADMIN_REVIEW,APPROVED,REJECTED) cleanly extend the ProjectStatus union type and align with the admin workflow changes in the service layer.src/services/projectService.ts (1)
69-82: LGTM! Response structure properly handles nested data.The changes to extract
res.data.datawith fallbacks properly handle the updated backend response structure, ensuring backward compatibility.src/app/dashboard/admin/page.tsx (1)
7-69: LGTM! Well-implemented role-based access control.The role checking pattern is clean and consistent:
- Properly combines loading states to avoid UI flicker
- Clear access denied messaging for unauthorized users
- Follows the same pattern used across other admin pages
src/app/dashboard/admin/config/page.tsx (1)
6-84: LGTM! Consistent role-based access control implementation.The access control pattern matches other admin pages exactly, providing a consistent security model across the admin section.
src/app/dashboard/layout.tsx (3)
171-174: LGTM! Payment History link properly gated by role.The conditional addition of the Payment History link for CUSTOMER role is well-implemented and aligns with the role-based navigation pattern.
8-37: Note: NotificationProvider will remount on profile changes.The
NotificationProvideris keyed implicitly by theuserIdprop derived fromprofile.id. If the profile changes (e.g., due to a refresh), the entire notification system will remount and reconnect websockets. This is likely intentional but could cause brief interruptions in real-time notifications.Consider whether this behavior is desired, or if the NotificationProvider should persist across profile refreshes.
29-37: The review comment is incorrect and should be dismissed.The type of
profile.idis well-defined: it's typed asnumberin theUserDtointerface. The conversion usingString(profile.id)is not redundant—it's necessary becauseNotificationProviderexpects theuserIdprop to bestring | null. The conditional checkprofile?.id ? String(profile.id) : nullcorrectly handles both the type conversion and the null case.src/app/dashboard/projects/request/page.tsx (2)
19-27: Redirect logic works correctly with loading guard.The useEffect redirect for non-customers only fires after
profileLoadingcompletes, and the loading guard at Line 93 prevents rendering until permissions are checked. This ensures clean authorization flow.
45-52: Vehicle filtering relies on optional profile.username.The filtering logic accesses
profile?.usernamewhich could beundefinedduring initial load. However, this is protected by the loading guard at Line 93 that checksprofileLoading, ensuringprofileis populated before this code executes.src/app/dashboard/appointments/page.tsx (2)
71-86: Role-based header actions implemented correctly.The conditional rendering properly shows "Book Appointment" for customers and "Check Availability" for employees/admins, aligning with role-specific workflows.
122-129: Empty-state CTA correctly restricted to customers.The "Book Your First Appointment" link only displays for users with the CUSTOMER role, consistent with the access pattern used in the header.
src/types/appointment.ts (2)
28-29: New vehicle arrival tracking fields added.The
vehicleArrivedAtandvehicleAcceptedByEmployeeIdfields support vehicle arrival workflow tracking. Ensure backend endpoints properly populate these optional fields.
132-135: AssignEmployeesRequestDto supports multi-employee assignment.The new interface aligns with the array-based
assignedEmployeeIdsfield inAppointmentResponseDto, enabling bulk employee assignment operations.src/app/dashboard/admin/audit-logs/page.tsx (2)
46-82: Access control and loading states properly implemented.The page correctly gates access to ADMIN/SUPER_ADMIN roles, shows loading skeletons while permissions are checked, and displays a clear access-denied message for unauthorized users.
27-29: Enhanced error handling with structured message extraction.The error handling now extracts server-provided messages with a fallback, improving user feedback when the audit log service is unavailable.
src/app/dashboard/projects/page.tsx (2)
102-104: Role-based project request button correctly gated.The "+ Request Project" button only displays for users with the CUSTOMER role, consistent with the access control pattern used across the dashboard.
175-175: Empty-state CTA properly restricted to customers.The "Request Your First Project" link only appears for customers when no projects exist, matching the expected workflow.
src/app/dashboard/admin/service-types/page.tsx (1)
31-67: Access control follows consistent admin pattern.The implementation matches the access control pattern used in other admin pages (audit-logs, reports), with proper role checks, loading guards, and access-denied UI for unauthorized users.
src/app/components/NotificationBell.tsx (2)
4-7: Refactored to use centralized notification context.The component now consumes shared notification state and actions from
NotificationContext, eliminating local state management and improving consistency across the application.
82-84: Real-time connection indicator added.The green dot displays when
isConnectedis true, providing visual feedback that the WebSocket connection is active for real-time notifications.src/app/dashboard/admin/reports/page.tsx (3)
38-73: Access control properly restricts page to admins.The implementation follows the established admin page pattern with role checks, loading guards, and access-denied UI for unauthorized users.
11-24: Report request structure updated to match backend interface.The changes align with the updated
ReportRequestinterface:reportType→type,from/to→fromDate/toDate, and the new requiredformatfield. The expanded report type options (6 types) provide broader reporting capabilities.
102-114: Report format selection added.The new format dropdown allows users to choose between PDF, Excel, CSV, and JSON output formats, enhancing report flexibility.
| {/* Floating AI Chat Widget */} | ||
| <div className="fixed bottom-6 right-6 z-50"> | ||
| {chatOpen ? ( | ||
| <div className="bg-white dark:bg-gray-800 rounded-lg shadow-2xl w-96 h-[32rem] flex flex-col border border-gray-200 dark:border-gray-700"> | ||
| <div className="flex items-center justify-between p-4 border-b border-gray-200 dark:border-gray-700"> | ||
| <h3 className="text-lg font-semibold text-indigo-700 dark:text-indigo-400">TechTorque AI Assistant</h3> | ||
| <button | ||
| onClick={() => setChatOpen(false)} | ||
| className="text-gray-500 hover:text-gray-700 dark:text-gray-400 dark:hover:text-gray-200 transition-colors" | ||
| aria-label="Close chat" | ||
| > | ||
| <svg className="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M6 18L18 6M6 6l12 12" /> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
| <div className="flex-1 overflow-hidden"> | ||
| <AIChatWidget /> | ||
| </div> | ||
| </div> | ||
| ) : ( | ||
| <button | ||
| onClick={() => setChatOpen(true)} | ||
| className="bg-indigo-600 hover:bg-indigo-700 text-white rounded-full p-4 shadow-lg transition-all hover:scale-110 flex items-center gap-2" | ||
| aria-label="Open chat" | ||
| > | ||
| <svg className="w-6 h-6" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M8 10h.01M12 10h.01M16 10h.01M9 16H5a2 2 0 01-2-2V6a2 2 0 012-2h14a2 2 0 012 2v8a2 2 0 01-2 2h-5l-5 5v-5z" /> | ||
| </svg> | ||
| <span className="font-medium">Chat</span> | ||
| </button> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Add error boundary and improve mobile responsiveness.
The chat widget implementation has two concerns:
-
No error boundary: If
AIChatWidgetthrows an error, it could break the entire dashboard. Wrap it in an error boundary to gracefully handle failures. -
Mobile responsiveness: The fixed width of
w-96(384px) is too wide for mobile screens. Consider using responsive classes likew-full sm:w-96and adjusting positioning for smaller screens.
Consider this approach:
- <div className="fixed bottom-6 right-6 z-50">
+ <div className="fixed bottom-4 right-4 sm:bottom-6 sm:right-6 z-50">
{chatOpen ? (
- <div className="bg-white dark:bg-gray-800 rounded-lg shadow-2xl w-96 h-[32rem] flex flex-col border border-gray-200 dark:border-gray-700">
+ <div className="bg-white dark:bg-gray-800 rounded-lg shadow-2xl w-[calc(100vw-2rem)] sm:w-96 h-[32rem] flex flex-col border border-gray-200 dark:border-gray-700">For the error boundary, create a wrapper component or use a library like react-error-boundary:
import { ErrorBoundary } from 'react-error-boundary';
// In the render:
<div className="flex-1 overflow-hidden">
<ErrorBoundary fallback={<div className="p-4 text-center text-red-500">Chat unavailable</div>}>
<AIChatWidget />
</ErrorBoundary>
</div>| onClick={() => setChatOpen(false)} | ||
| className="text-gray-500 hover:text-gray-700 dark:text-gray-400 dark:hover:text-gray-200 transition-colors" | ||
| aria-label="Close chat" | ||
| > | ||
| <svg className="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M6 18L18 6M6 6l12 12" /> | ||
| </svg> | ||
| </button> |
There was a problem hiding this comment.
Add ESC key support for better accessibility.
Consider adding keyboard event handling to allow users to close the chat widget using the ESC key, which is a common UX pattern for dismissible overlays.
+ useEffect(() => {
+ const handleEscape = (e: KeyboardEvent) => {
+ if (e.key === 'Escape' && chatOpen) {
+ setChatOpen(false);
+ }
+ };
+
+ document.addEventListener('keydown', handleEscape);
+ return () => document.removeEventListener('keydown', handleEscape);
+ }, [chatOpen]);
+
return (Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/components/dashboards/CustomerDashboard.tsx around lines 231 to 238,
add keyboard handling so the ESC key also closes the chat: implement a useEffect
in the component that registers a keydown listener which checks for event.key
=== 'Escape' (or event.keyCode === 27 for older support) and calls
setChatOpen(false) when triggered, ensure the listener is added only while the
chat is open (or always but gated) and remove the listener in the cleanup
function to avoid leaks; include setChatOpen (and chat open state if used) in
the effect dependencies.
| // Show toast notification | ||
| const showToast = useCallback((notification: NotificationResponse) => { | ||
| setToastNotification(notification); | ||
|
|
||
| // Auto-hide toast after 5 seconds | ||
| setTimeout(() => { | ||
| setToastNotification(null); | ||
| }, 5000); | ||
| }, []); |
There was a problem hiding this comment.
Prevent toast timers from stomping each other
Each toast schedules setTimeout but never clears the previous handle. If two notifications arrive close together, the first timer still fires and dismisses the second toast early (and it can fire after unmount). Track the timer with useRef, clear it before starting a new one, and clean it up on unmount.
Suggested fix:
-import React, { createContext, useContext, useState, useEffect, useCallback, ReactNode } from 'react';
+import React, { createContext, useContext, useState, useEffect, useCallback, useRef, ReactNode } from 'react';
…
- const [toastNotification, setToastNotification] = useState<NotificationResponse | null>(null);
+ const [toastNotification, setToastNotification] = useState<NotificationResponse | null>(null);
+ const toastTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
…
const showToast = useCallback((notification: NotificationResponse) => {
setToastNotification(notification);
-
- // Auto-hide toast after 5 seconds
- setTimeout(() => {
- setToastNotification(null);
- }, 5000);
- }, []);
+ if (toastTimerRef.current) {
+ clearTimeout(toastTimerRef.current);
+ }
+ toastTimerRef.current = setTimeout(() => {
+ setToastNotification(null);
+ toastTimerRef.current = null;
+ }, 5000);
+ }, []);
+
+ useEffect(() => () => {
+ if (toastTimerRef.current) {
+ clearTimeout(toastTimerRef.current);
+ }
+ }, []);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/contexts/NotificationContext.tsx around lines 105 to 113, the
showToast function schedules a setTimeout for auto-hide but never clears prior
timers, causing earlier timers to dismiss newer toasts (and potentially fire
after unmount); create a ref (e.g., toastTimerRef) to hold the timeout id, call
clearTimeout on the existing id before starting a new setTimeout and store the
new id in the ref, and add a useEffect cleanup that clears the ref's timeout on
unmount to prevent timers firing after component teardown.
| } catch (err: any) { | ||
| console.error('Failed to create user:', err); | ||
| const errorMessage = err?.response?.status === 403 | ||
| ? 'Permission denied. You do not have the required permissions to create this user type.' | ||
| : err?.response?.data?.message || 'Failed to create user. Please try again.'; | ||
| alert(errorMessage); | ||
| } |
There was a problem hiding this comment.
Replace err: any with a typed narrowing
The ESLint failure stems from the catch (err: any). Switch the catch parameter to unknown and narrow before reading response data, e.g.
- } catch (err: any) {
- console.error('Failed to create user:', err);
- const errorMessage = err?.response?.status === 403
- ? 'Permission denied. You do not have the required permissions to create this user type.'
- : err?.response?.data?.message || 'Failed to create user. Please try again.';
+ } catch (err: unknown) {
+ console.error('Failed to create user:', err);
+ const axiosError = err as { response?: { status?: number; data?: { message?: string } } };
+ const errorMessage = axiosError.response?.status === 403
+ ? 'Permission denied. You do not have the required permissions to create this user type.'
+ : axiosError.response?.data?.message || 'Failed to create user. Please try again.';
alert(errorMessage);
}📝 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.
| } catch (err: any) { | |
| console.error('Failed to create user:', err); | |
| const errorMessage = err?.response?.status === 403 | |
| ? 'Permission denied. You do not have the required permissions to create this user type.' | |
| : err?.response?.data?.message || 'Failed to create user. Please try again.'; | |
| alert(errorMessage); | |
| } | |
| } catch (err: unknown) { | |
| console.error('Failed to create user:', err); | |
| const axiosError = err as { response?: { status?: number; data?: { message?: string } } }; | |
| const errorMessage = axiosError.response?.status === 403 | |
| ? 'Permission denied. You do not have the required permissions to create this user type.' | |
| : axiosError.response?.data?.message || 'Failed to create user. Please try again.'; | |
| alert(errorMessage); | |
| } |
🧰 Tools
🪛 GitHub Actions: Build and Test Frontend_Web
[error] 80-80: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
🪛 GitHub Actions: Build, Test, and Package Frontend
[error] 80-80: ESLint: Unexpected any. Specify a different type (@typescript-eslint/no-explicit-any).
🪛 GitHub Check: Build and Test Frontend
[failure] 80-80:
Unexpected any. Specify a different type
🪛 GitHub Check: Install, Lint and Build
[failure] 80-80:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
In src/app/dashboard/admin/users/page.tsx around lines 80-86, change the catch
parameter from err: any to err: unknown and narrow it before accessing
response/data: replace catch(err: any) with catch(err: unknown), then use a type
guard (or Axios.isAxiosError) to detect an object with a response shape (e.g.,
function hasResponse(e: unknown): e is { response: { status?: number; data?: {
message?: string } } } { return typeof e === 'object' && e !== null &&
'response' in e; }) and use that guard to safely read
response.status/response.data.message; fallback to a generic message when the
guard fails or err is not that shape, and keep the console.error by stringifying
or logging err after narrowing.
| useEffect(() => { | ||
| const loadUserProfile = async () => { | ||
| try { | ||
| const profile = await userService.getCurrentProfile() | ||
| setUserProfile(profile.data) | ||
| } catch (err) { | ||
| console.error('Failed to load user profile:', err) | ||
| } | ||
| } | ||
| void loadUserProfile() | ||
| }, []) |
There was a problem hiding this comment.
Add user-visible error handling for profile loading failures.
The profile loading only logs errors to console. If profile loading fails, the PaymentGateway won't render (line 356), but users won't know why. This could be confusing, especially if the invoice has a balance due.
Consider showing a warning message when profile fails to load but payment is available:
useEffect(() => {
const loadUserProfile = async () => {
try {
const profile = await userService.getCurrentProfile()
setUserProfile(profile.data)
} catch (err) {
console.error('Failed to load user profile:', err)
+ setError('Unable to load profile information. Some features may be limited.')
}
}
void loadUserProfile()
}, [])Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/dashboard/invoices/[invoiceId]/page.tsx around lines 75 to 85, the
useEffect only logs profile loading failures to the console; add user-visible
error handling by creating a new state (e.g., profileLoadError: string | null)
and set it inside the catch with a friendly message like "We couldn't load your
profile — some payment options may be unavailable."; keep the console.error for
debugging but also set profileLoadError so the UI can show it; then, where the
PaymentGateway / invoice payment UI is rendered (near line ~356), conditionally
render a visible warning/alert component when profileLoadError is non-null and
the invoice has a balance due, so users see the issue and any guidance (e.g.,
retry or contact support).
| async adminRejectProject(projectId: string, reason?: string): Promise<ApiResponse> { | ||
| const res = await api.post(`/projects/${projectId}/admin/reject`, null, { | ||
| params: { reason } | ||
| }); | ||
| return res.data; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the null body with query parameter pattern.
The adminRejectProject method sends null as the request body while passing reason as a query parameter. Ensure this matches the backend API expectations, as many REST APIs expect rejection reasons in the request body.
🌐 Web query:
What is the standard REST API pattern for sending rejection reasons - in the request body or as a query parameter?
💡 Result:
Short answer: put rejection reasons in the request body (as structured fields) when the client is intentionally sending a rejection/decision; use the response body (not query parameters) to return error/rejection details from the server—formatted per the Problem Details spec (application/problem+json). Query parameters are for filtering/identifiers only and are not appropriate for carrying descriptive/structured reasons. [1][2][3]
Why (brief):
- Request body: designed for structured input (POST/PUT/PATCH). Use JSON fields like "reason", "code", "notes" so clients and servers can parse and evolve the shape safely. [3]
- Response body for errors: RFC 7807 / RFC 9457 define standard machine-readable error payloads (type, title, status, detail, extensions) delivered in the response body (application/problem+json). Do not use query strings for error details. [1][2]
If you want, I can show a minimal request/response JSON example using RFC 7807.
🏁 Script executed:
#!/bin/bash
# Search for backend endpoint handlers for project admin reject endpoint
rg -n "admin.*reject|/projects" --type=ts --type=js -A 5 -B 2 | head -100Length of output: 7384
🏁 Script executed:
#!/bin/bash
# Search for any backend/API route definitions
fd -e "ts" -e "js" | xargs grep -l "admin.*reject\|/projects.*reject" | head -20Length of output: 30
🏁 Script executed:
#!/bin/bash
# Look for route/controller files that might define the endpoint
fd -type f \( -name "*route*" -o -name "*controller*" -o -name "*handler*" \) | head -20Length of output: 170
🏁 Script executed:
#!/bin/bash
# Search for any API documentation or comments about the reject endpoint
rg -n "reject.*reason|reason.*reject" -C 3 --type=ts --type=jsLength of output: 1167
🏁 Script executed:
#!/bin/bash
# Check axios post pattern with params - verify what's standard in this codebase
rg -n "api\.post.*params" --type=ts --type=js -A 2 -B 2 | head -50Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for test files that might show expected API behavior
fd -type f -name "*test*" -o -name "*spec*" | xargs grep -l "adminReject" 2>/dev/null | head -10Length of output: 170
Align adminRejectProject to use request body for reason, matching REST standards and the existing rejectQuote pattern in the same file.
The method currently sends null as the body with reason as a query parameter. However, rejection reasons should be in the request body (as structured fields) when the client is intentionally sending a rejection/decision, and query parameters are for filtering/identifiers only and are not appropriate for carrying descriptive/structured reasons.
The inconsistency is visible within the same file: rejectQuote (line 94–96) correctly uses api.post(..., { reason }) with the reason in the body, while adminRejectProject uses api.post(..., null, { params: { reason } }). Change adminRejectProject to:
api.post(`/projects/${projectId}/admin/reject`, { reason })
🤖 Prompt for AI Agents
In src/services/projectService.ts around lines 115 to 120, adminRejectProject
currently sends the reason as a query param (api.post(..., null, { params: {
reason } })); change it to include the reason in the request body to match the
rejectQuote pattern by calling api.post(`/projects/${projectId}/admin/reject`, {
reason }) so the rejection reason is sent as structured payload rather than a
query parameter; keep the existing return of res.data.
| onDisconnect: () => { | ||
| console.log('WebSocket disconnected'); | ||
| this.connected = false; | ||
| this.handleReconnect(userId, onNotification, onUnreadCount); | ||
| }, | ||
| }); | ||
|
|
||
| this.client.activate(); | ||
| } | ||
|
|
||
| private handleReconnect(userId: string, onNotification: NotificationCallback, onUnreadCount: UnreadCountCallback): void { | ||
| if (this.reconnectAttempts < this.maxReconnectAttempts) { | ||
| this.reconnectAttempts++; | ||
| console.log(`Attempting to reconnect (${this.reconnectAttempts}/${this.maxReconnectAttempts})...`); | ||
| setTimeout(() => { | ||
| this.connect(userId, onNotification, onUnreadCount); | ||
| }, 5000 * this.reconnectAttempts); | ||
| } else { | ||
| console.error('Max reconnection attempts reached. Please refresh the page.'); | ||
| } | ||
| } | ||
|
|
||
| disconnect(): void { | ||
| if (this.notificationSubscription) { | ||
| this.notificationSubscription.unsubscribe(); | ||
| this.notificationSubscription = null; | ||
| } | ||
|
|
||
| if (this.countSubscription) { | ||
| this.countSubscription.unsubscribe(); | ||
| this.countSubscription = null; | ||
| } | ||
|
|
||
| if (this.client) { | ||
| this.client.deactivate(); | ||
| this.client = null; | ||
| } | ||
|
|
||
| this.connected = false; | ||
| this.userId = null; | ||
| this.reconnectAttempts = 0; | ||
| console.log('WebSocket disconnected and cleaned up'); | ||
| } |
There was a problem hiding this comment.
Disconnect immediately re-establishes the socket
onDisconnect always calls handleReconnect, so when consumers invoke disconnect() the service still spins up a brand-new connection (because STOMP fires onDisconnect after deactivate). That makes the public disconnect() unusable. Please gate reconnection attempts behind a flag set only for unexpected drops, e.g. shouldAttemptReconnect, flip it to false in disconnect(), and only call handleReconnect when the flag is still true.
🤖 Prompt for AI Agents
In src/services/websocketService.ts around lines 79 to 121, onDisconnect always
triggers handleReconnect which causes a new connection even when the public
disconnect() was invoked; add a boolean flag (e.g. shouldAttemptReconnect) to
the class, set it true only for unexpected disconnects and false in
disconnect(), modify onDisconnect to call handleReconnect only when
shouldAttemptReconnect is true, ensure connect() or successful activation resets
shouldAttemptReconnect to true for future unexpected drops and reset
reconnectAttempts when a successful connection occurs.
| quantity: number; | ||
| unitPrice: number; | ||
| totalPrice: number; | ||
| itemType?: string; // LABOR, PARTS, SERVICE_FEE, etc. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use a union type for itemType instead of plain string.
The comment indicates specific allowed values (LABOR, PARTS, SERVICE_FEE, etc.), but the type is string, which allows any value. This reduces type safety and makes the code more error-prone.
Apply this diff:
- itemType?: string; // LABOR, PARTS, SERVICE_FEE, etc.
+ itemType?: 'LABOR' | 'PARTS' | 'SERVICE_FEE' | 'DIAGNOSTIC' | 'OTHER';📝 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.
| itemType?: string; // LABOR, PARTS, SERVICE_FEE, etc. | |
| itemType?: 'LABOR' | 'PARTS' | 'SERVICE_FEE' | 'DIAGNOSTIC' | 'OTHER'; |
🤖 Prompt for AI Agents
In src/types/payment.ts around line 86, the itemType field is declared as a
plain string but should be narrowed to the allowed values; change the
declaration to a union (e.g. 'LABOR' | 'PARTS' | 'SERVICE_FEE' | 'MATERIAL' |
'OTHER' or similar domain-specific values) or replace with an exported enum/type
alias and use that type here, then update any places that construct or compare
itemType to use the new union/enum values (and adjust tests/serializers that
assume arbitrary strings).
…functionality for service types
…s in AppointmentDetailPage
…te API base URL configuration
…e; update appointment handling and UI for better clarity and user experience
… status transitions
…pointment status type
…bSocket factory type assertion
…t and documentation
…ogic in project pages
Randitha superbranch
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (5)
src/services/websocketService.ts (1)
81-85: Critical: Disconnect immediately re-establishes the socket.The issue flagged in the previous review is still not fixed. When
disconnect()is called, it invokesclient.deactivate()which triggers theonDisconnectcallback, which unconditionally callshandleReconnect(). This makes intentional disconnections impossible.Add a flag to distinguish intentional from unexpected disconnects:
private reconnectAttempts: number = 0; private maxReconnectAttempts: number = 5; + private shouldReconnect: boolean = true; connect(userId: string, onNotification: NotificationCallback, onUnreadCount: UnreadCountCallback): void { if (this.connected && this.userId === userId) { console.log('WebSocket already connected for user:', userId); return; } + this.shouldReconnect = true; this.userId = userId; ... onConnect: () => { console.log('WebSocket connected successfully'); this.connected = true; this.reconnectAttempts = 0; + this.shouldReconnect = true; ... }, onDisconnect: () => { console.log('WebSocket disconnected'); this.connected = false; - this.handleReconnect(userId, onNotification, onUnreadCount); + if (this.shouldReconnect) { + this.handleReconnect(userId, onNotification, onUnreadCount); + } }, }); ... disconnect(): void { + this.shouldReconnect = false; + if (this.notificationSubscription) {src/app/dashboard/admin/users/page.tsx (1)
80-87: Address the unresolved type safety issue.The past review correctly identified the
err: anyproblem. Disabling ESLint (// eslint-disable-next-line) doesn't fix the underlying type safety issue—it just silences the warning.Apply the recommended fix from the previous review using proper type narrowing:
- } catch (err) { + } catch (err: unknown) { console.error('Failed to create user:', err); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const error = err as any; - const errorMessage = error?.response?.status === 403 + const axiosError = err as { response?: { status?: number; data?: { message?: string } } }; + const errorMessage = axiosError.response?.status === 403 ? 'Permission denied. You do not have the required permissions to create this user type.' - : error?.response?.data?.message || 'Failed to create user. Please try again.'; + : axiosError.response?.data?.message || 'Failed to create user. Please try again.'; alert(errorMessage); }src/lib/apiClient.ts (1)
3-45: Runtime config still resolves too early, so overrides never win.
configis evaluated when the module loads, so if this bundle runs before/runtime-config.jsfinishes executing (or during SSR),API_BASE_URLlocks onto the fallback and the axios instance keeps that value forever. The runtime override injected by the script is therefore ignored—the exact race we called out previously. Please defer readingwindow.__RUNTIME_CONFIG__until request time (or update the axios defaults after the script runs) so the true runtime base URL is always used.-import { config } from '@/config/runtime'; - -// Use runtime configuration instead of build-time env vars -const API_BASE_URL = config.NEXT_PUBLIC_API_BASE_URL; - -const apiClient = axios.create({ - baseURL: `${API_BASE_URL}/api/v1`, - headers: { 'Content-Type': 'application/json' }, -}); +const resolveApiBaseUrl = () => { + if (typeof window !== 'undefined') { + const runtimeBase = window.__RUNTIME_CONFIG__?.NEXT_PUBLIC_API_BASE_URL; + if (runtimeBase) { + return `${runtimeBase}/api/v1`; + } + } + const fallbackBase = process.env.NEXT_PUBLIC_API_BASE_URL ?? 'http://localhost:8080'; + return `${fallbackBase}/api/v1`; +}; + +const apiClient = axios.create({ + headers: { 'Content-Type': 'application/json' }, +}); // Request interceptor (Your existing code is perfect) apiClient.interceptors.request.use( (config) => { + config.baseURL = resolveApiBaseUrl(); const token = Cookies.get('tt_access_token');src/app/dashboard/projects/[projectId]/page.tsx (1)
72-96: Point admin approval at the admin endpoint
projectService.adminApproveProjectstill posts to/projects/${projectId}/approve, the customer route. Admin approvals will keep 409’ing instead of transitioning to the admin statuses. Please move the service call to the dedicated admin endpoint before shipping.Update
src/services/projectService.ts:async adminApproveProject(projectId: string): Promise<ApiResponse> { - const res = await api.post(`/projects/${projectId}/approve`); + const res = await api.post(`/projects/${projectId}/admin/approve`); return res.data; },src/types/appointment.ts (1)
19-19: Ensure schedule page migration is complete.The migration from
assignedEmployeeIdtoassignedEmployeeIdswas previously flagged as incomplete in the schedule page. Verify thatsrc/app/dashboard/schedule/page.tsxhas been updated to use the array field.
🧹 Nitpick comments (16)
src/services/websocketService.ts (3)
48-56: Add explicit type assertion after JSON.parse.
JSON.parsereturnsany, so the notification variable is implicitlyanyeven though it's passed to a callback expectingNotificationResponse. Add an explicit type assertion for type safety.Apply this diff:
- const notification = JSON.parse(message.body); + const notification: NotificationResponse = JSON.parse(message.body);
91-101: Backoff is linear, not exponential.The current implementation uses
5000 * this.reconnectAttempts, which creates a linear backoff (5s, 10s, 15s, 20s, 25s) rather than exponential. This is acceptable for the current use case, but consider documenting this or using true exponential backoff if needed.For true exponential backoff:
- }, 5000 * this.reconnectAttempts); + }, Math.min(5000 * Math.pow(2, this.reconnectAttempts - 1), 30000));
103-123: Make disconnect async and await deactivation.The
client.deactivate()method returns a Promise that should be awaited to ensure graceful shutdown. Making this method async allows proper cleanup sequencing.- disconnect(): void { + async disconnect(): Promise<void> { + this.shouldReconnect = false; + if (this.notificationSubscription) { this.notificationSubscription.unsubscribe(); this.notificationSubscription = null; } if (this.countSubscription) { this.countSubscription.unsubscribe(); this.countSubscription = null; } if (this.client) { - this.client.deactivate(); + await this.client.deactivate(); this.client = null; } this.connected = false; this.userId = null; this.reconnectAttempts = 0; console.log('WebSocket disconnected and cleaned up'); }.githooks/pre-push (1)
1-19: Consider adding basic environment validation.The script is functional but could be more robust. Consider adding checks for prerequisites to provide better error messages.
Optional enhancement to add environment validation:
#!/bin/bash # Git pre-push hook to run build check # This hook runs before pushing to ensure the project builds successfully +# Check if npm is available +if ! command -v npm &> /dev/null; then + echo "❌ npm is not installed or not in PATH" + exit 1 +fi + +# Check if package.json exists +if [ ! -f "package.json" ]; then + echo "❌ package.json not found" + exit 1 +fi + echo "🏗️ Running build check before push..." # Run build check npm run build.githooks/pre-commit (1)
1-19: Consider adding basic environment validation.Similar to the pre-push hook, this script would benefit from prerequisite checks for better error messages.
Optional enhancement to add environment validation:
#!/bin/bash # Git pre-commit hook to run ESLint # This hook runs on every commit to ensure code quality +# Check if npm is available +if ! command -v npm &> /dev/null; then + echo "❌ npm is not installed or not in PATH" + exit 1 +fi + +# Check if package.json exists +if [ ! -f "package.json" ]; then + echo "❌ package.json not found" + exit 1 +fi + echo "🔍 Running ESLint check..." # Run lint check npm run lintsrc/app/dashboard/admin/users/page.tsx (1)
103-107: Validate role array before API call.While the UI validation prevents submitting with zero roles, a user could potentially manipulate the state. Consider adding server-side validation or a runtime check before the API call.
// Validation: Ensure at least one role is selected if (editingRoles.length === 0) { alert('Please select at least one role for the user.'); return; } + + // Ensure roles are valid + const validRoles = ['CUSTOMER', 'EMPLOYEE', 'ADMIN', 'SUPER_ADMIN']; + const invalidRoles = editingRoles.filter(role => !validRoles.includes(role)); + if (invalidRoles.length > 0) { + alert(`Invalid roles detected: ${invalidRoles.join(', ')}`); + return; + }src/types/admin.ts (1)
28-62: Consider adding a type mapper utility.The field name mismatch between request types (e.g.,
price,durationMinutes) and response types (e.g.,basePriceLKR,estimatedDurationMinutes) could lead to mapping errors. While the comments help, a utility function would make conversions explicit and type-safe.Example utility function:
// In a utils file export function mapServiceTypeToRequest( response: Partial<ServiceTypeResponse> ): Partial<CreateServiceTypeRequest> { return { name: response.name, category: response.category, description: response.description, price: response.basePriceLKR, durationMinutes: response.estimatedDurationMinutes, }; }src/app/dashboard/vehicles/[vehicleId]/page.tsx (1)
106-112: Review the type assertion pattern.The code uses
as unknown as stringwhen passing photo URLs to the Image component (Lines 107, 125). This double-cast suggests thevehicle.photostype doesn't match the expectedstring[]type.Check the
Vehicletype definition and either:
- Update the type to accurately reflect the photo URL structure
- Add proper validation/transformation if photos are stored in a different format
#!/bin/bash # Check the Vehicle type definition for the photos field ast-grep --pattern $'interface Vehicle { $$$ photos$_$$$ $$$ }'Also applies to: 125-125
src/app/dashboard/layout.tsx (1)
173-176: Consider conditional array construction for cleaner code.The Payment History link is added via
pushafter the base items are defined. While functional, conditionally including items during array construction is more declarative and easier to maintain.Consider this approach:
const baseSection: NavSection = { title: 'Overview', items: [ { href: '/dashboard', label: 'Home' }, { href: '/dashboard/vehicles', label: 'Vehicles' }, { href: '/dashboard/appointments', label: 'Appointments' }, { href: '/dashboard/invoices', label: 'Invoices' }, { href: '/dashboard/projects', label: 'Projects' }, { href: '/dashboard/notifications', label: 'Notifications' }, + ...(hasRole('CUSTOMER') ? [{ href: '/dashboard/payments', label: 'Payment History' }] : []), ], } - - // Add Payments link only for customers - if (hasRole('CUSTOMER')) { - baseSection.items.push({ href: '/dashboard/payments', label: 'Payment History' }) - }src/app/components/RoleSwitcher.tsx (2)
84-89: Add error handling for role switch operation.If
onRoleChangethrows an error, the dropdown remains open androuter.refresh()is never called, leaving the UI in an inconsistent state.Apply this diff:
const handleRoleSwitch = (role: string) => { - onRoleChange(role); - setIsOpen(false); - // Optionally refresh the page to update the dashboard view - router.refresh(); + try { + onRoleChange(role); + setIsOpen(false); + // Optionally refresh the page to update the dashboard view + router.refresh(); + } catch (error) { + console.error('Failed to switch role:', error); + // Optionally show user feedback + } };
94-111: Improve accessibility with ARIA attributes and keyboard navigation.The role switcher lacks proper accessibility support:
- No
aria-expandedto indicate dropdown state- No
aria-labelfor screen readers- No keyboard navigation support (Escape to close, Arrow keys)
Consider adding:
<button onClick={() => setIsOpen(!isOpen)} onKeyDown={(e) => { if (e.key === 'Escape') setIsOpen(false); }} aria-expanded={isOpen} aria-label="Switch role view" aria-haspopup="menu" className={`flex items-center gap-2 px-3 py-2 rounded-lg transition-all ${currentConfig.bgColor} hover:opacity-80`} title="Switch role view" >For the dropdown menu items, add
role="menuitem"and keyboard handlers for arrow key navigation.src/components/TimeTracker.tsx (2)
36-52: Consider less frequent polling or WebSocket for time updates.The component polls the backend every second while a session is active. For multiple concurrent users, this creates significant network and backend load.
Consider these alternatives:
- Less frequent polling: Poll every 5-10 seconds and interpolate locally for smoother display
- WebSocket updates: Use the existing WebSocket infrastructure (NotificationContext) to push time updates
- Client-side timer: Track elapsed time locally after initial load, only polling occasionally to sync
Example with local timer:
useEffect(() => { if (!timeSession?.active) return; // Update locally every second const localInterval = setInterval(() => { setElapsedSeconds(prev => prev + 1); }, 1000); // Sync with server every 30 seconds const syncInterval = setInterval(async () => { const session = await appointmentService.getActiveTimeSession(appointmentId); if (session) setElapsedSeconds(session.elapsedSeconds); }, 30000); return () => { clearInterval(localInterval); clearInterval(syncInterval); }; }, [appointmentId, timeSession?.active]);
71-74: Consider custom confirmation dialog for better UX.Using
window.confirmcreates a blocking native dialog with inconsistent styling across browsers. A custom confirmation modal would provide better user experience and brand consistency.While the current implementation works, consider using a custom modal component that matches the app's design system for a more polished experience.
src/app/dashboard/appointments/[appointmentId]/page.tsx (1)
263-268: Enhance invoice validation for data integrity.The validation checks description and unit price but misses other important validations that could lead to invalid invoices being created.
Apply this diff to add comprehensive validation:
// Validation - const hasEmptyItems = invoiceItems.some(item => !item.description.trim() || item.unitPrice <= 0) + const hasEmptyItems = invoiceItems.some(item => + !item.description.trim() || + item.unitPrice <= 0 || + item.quantity <= 0 || + item.totalPrice !== item.quantity * item.unitPrice + ) if (hasEmptyItems) { - setError('Please fill in all invoice items with valid descriptions and prices') + setError('Please fill in all invoice items with valid descriptions, quantities, and prices') return }src/app/dashboard/admin/service-types/page.tsx (2)
129-149: Optimize status toggle to use partial update.The
handleToggleActivefunction sends all service type fields when only theactivestatus needs to change. This is inefficient and could lead to unintended overwrites if the data is stale.Apply this diff:
const handleToggleActive = async (service: ServiceTypeResponse) => { try { const newStatus = !service.active; console.log(`Toggling service ${service.id} from ${service.active} to ${newStatus}`); - await adminService.updateServiceType(service.id, { - name: service.name, - category: service.category, - description: service.description || '', - price: service.basePriceLKR, - durationMinutes: service.estimatedDurationMinutes, - active: newStatus, - }); + await adminService.updateServiceType(service.id, { + active: newStatus, + }); console.log('Toggle successful, reloading list...'); await loadServiceTypes();
78-92: Add explicit validation for numeric inputs.Using
parseFloat(formData.price) || 0silently converts invalid input to 0, which could allow creating service types with zero price unintentionally.Add explicit validation before submission:
const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); // Validate numeric fields const price = parseFloat(formData.price); if (isNaN(price) || price <= 0) { alert('Please enter a valid price greater than 0'); return; } if (formData.durationMinutes < 15 || formData.durationMinutes > 480) { alert('Duration must be between 15 and 480 minutes'); return; } try { // ... rest of the function } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
.githooks/pre-commit(1 hunks).githooks/pre-push(1 hunks)GIT_HOOKS.md(1 hunks)README.md(1 hunks)package.json(2 hunks)public/runtime-config.js(1 hunks)setup-hooks.sh(1 hunks)src/app/auth/login/page.tsx(3 hunks)src/app/auth/register/page.tsx(2 hunks)src/app/auth/resend-verification/page.tsx(1 hunks)src/app/components/AddVehicleForm.tsx(1 hunks)src/app/components/NotificationBell.tsx(6 hunks)src/app/components/RoleSwitcher.tsx(1 hunks)src/app/contexts/DashboardContext.tsx(3 hunks)src/app/dashboard/admin/service-types/page.tsx(4 hunks)src/app/dashboard/admin/users/page.tsx(6 hunks)src/app/dashboard/appointments/[appointmentId]/page.tsx(8 hunks)src/app/dashboard/appointments/book/page.tsx(8 hunks)src/app/dashboard/layout.tsx(5 hunks)src/app/dashboard/page.tsx(2 hunks)src/app/dashboard/payments/page.tsx(3 hunks)src/app/dashboard/projects/[projectId]/page.tsx(5 hunks)src/app/dashboard/projects/page.tsx(4 hunks)src/app/dashboard/projects/request/page.tsx(3 hunks)src/app/dashboard/schedule/page.tsx(5 hunks)src/app/dashboard/vehicles/[vehicleId]/page.tsx(3 hunks)src/app/profile/page.tsx(3 hunks)src/components/ProfilePhotoUpload.tsx(2 hunks)src/components/TimeTracker.tsx(1 hunks)src/lib/apiClient.ts(2 hunks)src/services/adminService.ts(4 hunks)src/services/appointmentService.ts(2 hunks)src/services/websocketService.ts(1 hunks)src/types/admin.ts(2 hunks)src/types/appointment.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- GIT_HOOKS.md
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/app/dashboard/projects/page.tsx
🧰 Additional context used
🧬 Code graph analysis (18)
src/lib/apiClient.ts (1)
src/config/runtime.ts (1)
config(24-24)
src/app/dashboard/layout.tsx (3)
src/app/contexts/DashboardContext.tsx (2)
DashboardProvider(17-63)useDashboard(65-71)src/app/contexts/NotificationContext.tsx (1)
NotificationProvider(34-200)src/app/components/RoleSwitcher.tsx (1)
RoleSwitcher(43-163)
src/components/TimeTracker.tsx (2)
src/types/appointment.ts (1)
TimeSessionResponse(139-148)src/services/appointmentService.ts (1)
appointmentService(17-162)
src/app/dashboard/projects/request/page.tsx (3)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(65-71)src/types/vehicle.ts (1)
VehicleListItem(18-27)src/services/vehicleService.ts (1)
vehicleService(13-84)
src/app/dashboard/admin/users/page.tsx (5)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(65-71)src/types/admin.ts (1)
UserResponse(4-17)src/types/api.ts (2)
CreateEmployeeRequest(19-26)CreateAdminRequest(28-34)src/services/authService.ts (1)
authService(13-92)src/services/adminService.ts (1)
adminService(17-190)
src/app/components/NotificationBell.tsx (3)
src/app/contexts/NotificationContext.tsx (1)
useNotifications(21-27)src/services/notificationService.ts (2)
markAsRead(28-33)markAllAsRead(54-59)src/services/websocketService.ts (1)
isConnected(125-127)
src/app/contexts/DashboardContext.tsx (2)
src/services/userService.ts (1)
userService(21-222)src/types/api.ts (1)
UserDto(37-51)
src/app/auth/login/page.tsx (1)
src/types/api.ts (1)
LoginRequest(4-7)
src/app/dashboard/page.tsx (2)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(65-71)src/types/api.ts (1)
UserDto(37-51)
src/app/dashboard/appointments/[appointmentId]/page.tsx (8)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(65-71)src/types/admin.ts (1)
UserResponse(4-17)src/types/payment.ts (2)
InvoiceItemDto(81-87)CreateInvoiceDto(89-96)src/services/appointmentService.ts (1)
appointmentService(17-162)src/services/adminService.ts (1)
adminService(17-190)src/services/paymentService.ts (1)
paymentService(15-94)src/components/TimeTracker.tsx (1)
TimeTracker(12-176)src/types/appointment.ts (1)
AppointmentStatus(32-39)
src/services/adminService.ts (1)
src/types/admin.ts (2)
CreateServiceTypeRequest(40-50)ServiceTypeResponse(28-38)
src/app/dashboard/projects/[projectId]/page.tsx (2)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(65-71)src/services/projectService.ts (1)
projectService(16-137)
src/services/websocketService.ts (1)
src/types/notification.ts (1)
NotificationResponse(3-14)
src/app/auth/resend-verification/page.tsx (2)
src/services/authService.ts (1)
authService(13-92)src/app/components/ThemeToggle.tsx (1)
ThemeToggle(5-51)
src/app/dashboard/admin/service-types/page.tsx (3)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(65-71)src/types/admin.ts (3)
ServiceTypeResponse(28-38)UpdateServiceTypeRequest(52-62)CreateServiceTypeRequest(40-50)src/services/adminService.ts (1)
adminService(17-190)
src/app/dashboard/schedule/page.tsx (1)
src/app/contexts/DashboardContext.tsx (1)
useDashboard(65-71)
src/services/appointmentService.ts (1)
src/types/appointment.ts (4)
StatusUpdateDto(41-43)AppointmentResponseDto(15-30)AssignEmployeesRequestDto(134-136)TimeSessionResponse(139-148)
src/app/dashboard/appointments/book/page.tsx (1)
src/services/appointmentService.ts (1)
appointmentService(17-162)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (22)
src/services/websocketService.ts (2)
6-7: LGTM! Type definitions are properly typed.The callback types now use
NotificationResponseandnumberinstead ofany, which addresses the typing concerns from the previous review.
134-136: LGTM! Singleton pattern is appropriate.Using a singleton for the WebSocket service ensures a single shared connection across the application, which is the correct approach for this use case.
src/app/auth/register/page.tsx (2)
118-118: LGTM!The comment clearly documents the intentional removal of automatic redirection, which aligns with the improved UX flow below.
183-207: The resend verification route exists and the implementation is correct.The new email verification panel is a good UX improvement. The
/auth/resend-verificationroute exists atsrc/app/auth/resend-verification/page.tsx, the email parameter is properly URL-encoded, and the link construction is valid.The emoji "📧" on line 185 could optionally be replaced with an icon component for better accessibility, but the current implementation is acceptable.
README.md (1)
46-50: LGTM!The documentation clearly explains the Git hooks setup with an appropriate command and reference to detailed documentation. Marking this as "Recommended" rather than required is a good practice.
setup-hooks.sh (1)
14-22: LGTM!The user feedback and instructions are clear and helpful. Documenting the
--no-verifybypass option is particularly valuable for emergency situations.src/app/profile/page.tsx (1)
3-3: LGTM!The migration to Next.js Image components is properly implemented with appropriate width/height props and conditional rendering to prevent empty src errors.
Also applies to: 342-342, 351-351
src/types/admin.ts (1)
40-62: Type definitions align with backend contract.The request types correctly map to backend field names (
pricevsbasePriceLKR,durationMinutesvsestimatedDurationMinutes). Inline comments clearly document the differences, which helps prevent confusion.src/app/dashboard/appointments/book/page.tsx (2)
67-82: Filtering past time slots improves UX.The
filteredAvailableSlotslogic correctly prevents users from booking appointments in the past, which is a good user experience improvement.
141-149: Confirm backend expects local datetime format.The code formats
requestedDateTimeas${date}T${time}:00without timezone info, which produces strings like2025-11-15T14:30:00. The comment states the backend expectsLocalDateTime.Please verify the backend API contract to ensure it accepts ISO 8601 local datetime format (without timezone offset) and doesn't require timezone information or UTC conversion.
src/app/dashboard/vehicles/[vehicleId]/page.tsx (1)
3-3: Image component migration implemented correctly.The Next.js Image components are properly used with appropriate dimensions and conditional rendering to prevent errors with missing photos.
Also applies to: 106-112, 125-125
src/app/dashboard/layout.tsx (2)
45-45: LGTM: Role-based navigation filtering.The navigation correctly filters based on
activeRolewhen set, falling back to all roles otherwise. This properly supports the role-switching feature.
159-160: LGTM: hasRole helper placement.Moving the
hasRolehelper to the top ofbuildNavigationimproves readability since it's used throughout the function.src/app/components/RoleSwitcher.tsx (2)
48-63: LGTM: Outside-click detection with proper cleanup.The outside-click detection correctly adds and removes the event listener based on
isOpen. The cleanup function safely removes the listener even if it wasn't added, which is harmless.
65-68: Add defensive check for undefined roles.The condition checks
roles.length <= 1but doesn't guard againstrolesbeingundefined, which would cause a runtime error.Apply this diff:
// Only show switcher if user has multiple roles - if (!roles || roles.length <= 1) { + if (!roles || !Array.isArray(roles) || roles.length <= 1) { return null; }Likely an incorrect or invalid review comment.
src/components/TimeTracker.tsx (1)
18-34: LGTM: Time session loading with proper dependency handling.The
loadTimeSessionfunction is correctly called whenappointmentIdchanges. Theeslint-disablecomment forexhaustive-depsis appropriate sinceloadTimeSessionis defined inline and would cause unnecessary re-runs if included.src/types/appointment.ts (1)
133-148: LGTM: Well-structured types for employee assignment and time tracking.The new
AssignEmployeesRequestDtoandTimeSessionResponsetypes are comprehensive and properly structured with appropriate optional fields.src/app/components/NotificationBell.tsx (1)
4-7: LGTM: Clean integration with NotificationContext.The component properly consumes the centralized notification state and methods from the context, improving maintainability and reducing code duplication.
src/app/dashboard/appointments/[appointmentId]/page.tsx (3)
71-72: LGTM: Safe initialization of selectedEmployeeIds from appointment data.The code correctly handles the case where
assignedEmployeeIdsmight be undefined and creates a new array reference to avoid mutation issues.
107-110: Timezone handling remains ambiguous; backend implementation unclear.The inconsistency between the type definition (
ISO 8601 format) and the code comment (backend expects LocalDateTime) indicates a potential documentation or design issue. The code sends an unqualified datetime string (YYYY-MM-DDTHH:MM:00with no timezone), which differs from valid ISO 8601.While this pattern appears intentional (repeated identically in both booking and rescheduling), it's unclear:
- Whether the backend interprets this as the user's browser timezone or the service location's timezone
- How conflicts are handled when users reschedule from a different timezone
The original concern about timezone conflicts is valid. Either:
- The type annotation should reflect the actual format being sent
- Backend documentation should clarify timezone expectations
- Comments should specify whose timezone is expected (user's or service location's)
521-521: Verify backend aligns with frontend's username assumption.The frontend code consistently expects
assignedEmployeeIdsto contain usernames (verified at lines 375, 487, 495, and 521). However, the backend code is not in this repository, so I cannot confirm the backend actually stores/returns usernames rather than user IDs. If there's a mismatch, authorization checks on line 521 would fail silently or incorrectly grant access.Confirm that the backend
/appointments/{appointmentId}/assign-employeesendpoint and the appointment data model store employee identifiers as usernames, not user IDs.src/services/appointmentService.ts (1)
92-102: LGTM: Proper handling of 204 No Content response.The method correctly interprets a 204 status as "no active session" and returns
null, while re-throwing other errors for proper error handling upstream.
| chmod +x .githooks/pre-commit .githooks/pre-push | ||
|
|
||
| # Configure git to use our hooks directory | ||
| git config core.hooksPath .githooks |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for critical commands.
The script doesn't verify that critical operations succeed. If chmod or git config fail, the script will still report success, potentially leaving the hooks in a broken state.
Apply this diff to add proper error handling:
+# Check if git is available
+if ! command -v git &> /dev/null; then
+ echo "❌ git is not installed or not in PATH"
+ exit 1
+fi
+
+# Check if hook files exist
+if [ ! -f ".githooks/pre-commit" ] || [ ! -f ".githooks/pre-push" ]; then
+ echo "❌ Hook files not found in .githooks directory"
+ exit 1
+fi
+
# Make hooks executable
-chmod +x .githooks/pre-commit .githooks/pre-push
+chmod +x .githooks/pre-commit .githooks/pre-push || {
+ echo "❌ Failed to make hooks executable"
+ exit 1
+}
# Configure git to use our hooks directory
-git config core.hooksPath .githooks
+git config core.hooksPath .githooks || {
+ echo "❌ Failed to configure git hooks path"
+ exit 1
+}📝 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.
| chmod +x .githooks/pre-commit .githooks/pre-push | |
| # Configure git to use our hooks directory | |
| git config core.hooksPath .githooks | |
| # Check if git is available | |
| if ! command -v git &> /dev/null; then | |
| echo "❌ git is not installed or not in PATH" | |
| exit 1 | |
| fi | |
| # Check if hook files exist | |
| if [ ! -f ".githooks/pre-commit" ] || [ ! -f ".githooks/pre-push" ]; then | |
| echo "❌ Hook files not found in .githooks directory" | |
| exit 1 | |
| fi | |
| # Make hooks executable | |
| chmod +x .githooks/pre-commit .githooks/pre-push || { | |
| echo "❌ Failed to make hooks executable" | |
| exit 1 | |
| } | |
| # Configure git to use our hooks directory | |
| git config core.hooksPath .githooks || { | |
| echo "❌ Failed to configure git hooks path" | |
| exit 1 | |
| } |
🤖 Prompt for AI Agents
In setup-hooks.sh around lines 9 to 12, the script runs critical commands (chmod
and git config) without checking their exit status; add error handling so
failures stop the script and report which command failed. Either enable strict
failure at the top (set -euo pipefail) or check each command's return value and
on failure print a clear error message identifying the failed command and exit
with a non‑zero status (for example use an if-not/then block or a short-circuit
|| handler that echoes the error and exits). Ensure the messages are specific
(e.g., "chmod failed" / "git config failed") and that the script exits
immediately when one of these operations fails.
| const Icon = ({ d, size = 10 }: { d: string; size?: number }) => ( | ||
| <svg className={`w-${size} h-${size} text-white`} fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> | ||
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={d} /> | ||
| </svg> | ||
| ); |
There was a problem hiding this comment.
Fix dynamic Tailwind class generation.
Template literals like w-${size} won't work with Tailwind CSS because classes must be statically determinable at build time for PurgeCSS/JIT to include them.
Apply this diff to use explicit class mapping or inline styles:
const Icon = ({ d, size = 10 }: { d: string; size?: number }) => (
- <svg className={`w-${size} h-${size} text-white`} fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
+ <svg style={{ width: `${size * 0.25}rem`, height: `${size * 0.25}rem` }} className="text-white" fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={d} />
</svg>
);Or use a size mapping object:
+const sizeClasses = {
+ 6: 'w-6 h-6',
+ 10: 'w-10 h-10',
+ 12: 'w-12 h-12',
+} as const;
+
const Icon = ({ d, size = 10 }: { d: string; size?: number }) => (
- <svg className={`w-${size} h-${size} text-white`} fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
+ <svg className={`${sizeClasses[size as keyof typeof sizeClasses] || 'w-10 h-10'} text-white`} fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={d} />
</svg>
);📝 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 Icon = ({ d, size = 10 }: { d: string; size?: number }) => ( | |
| <svg className={`w-${size} h-${size} text-white`} fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> | |
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={d} /> | |
| </svg> | |
| ); | |
| const Icon = ({ d, size = 10 }: { d: string; size?: number }) => ( | |
| <svg style={{ width: `${size * 0.25}rem`, height: `${size * 0.25}rem` }} className="text-white" fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> | |
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={d} /> | |
| </svg> | |
| ); |
| const Icon = ({ d, size = 10 }: { d: string; size?: number }) => ( | |
| <svg className={`w-${size} h-${size} text-white`} fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> | |
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={d} /> | |
| </svg> | |
| ); | |
| const sizeClasses = { | |
| 6: 'w-6 h-6', | |
| 10: 'w-10 h-10', | |
| 12: 'w-12 h-12', | |
| } as const; | |
| const Icon = ({ d, size = 10 }: { d: string; size?: number }) => ( | |
| <svg className={`${sizeClasses[size as keyof typeof sizeClasses] || 'w-10 h-10'} text-white`} fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> | |
| <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={d} /> | |
| </svg> | |
| ); |
| if (payload?.roles && payload.roles.length > 0) { | ||
| const sortedRoles = [...payload.roles].sort((a, b) => { | ||
| const priority = { SUPER_ADMIN: 0, ADMIN: 1, EMPLOYEE: 2, CUSTOMER: 3 }; | ||
| return (priority[a as keyof typeof priority] ?? 99) - (priority[b as keyof typeof priority] ?? 99); | ||
| }); | ||
| setActiveRole(sortedRoles[0]); | ||
| } |
There was a problem hiding this comment.
Reset activeRole when no roles are available
If the profile reloads without roles (or the fetch fails), activeRole keeps the previous privileged value, so downstream UI keeps rendering the wrong role-specific paths even though the user no longer qualifies. Please clear it before re-applying the highest role to avoid stale authorizations.
Apply this diff:
- // Set initial active role to the highest privilege role
- if (payload?.roles && payload.roles.length > 0) {
- const sortedRoles = [...payload.roles].sort((a, b) => {
+ // Set initial active role to the highest privilege role
+ const roles = payload?.roles ?? [];
+ if (roles.length > 0) {
+ const sortedRoles = [...roles].sort((a, b) => {
const priority = { SUPER_ADMIN: 0, ADMIN: 1, EMPLOYEE: 2, CUSTOMER: 3 };
return (priority[a as keyof typeof priority] ?? 99) - (priority[b as keyof typeof priority] ?? 99);
});
setActiveRole(sortedRoles[0]);
+ } else {
+ setActiveRole('');
}Also set setActiveRole('') in the catch branch so failures don't leave a stale value behind.
📝 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 (payload?.roles && payload.roles.length > 0) { | |
| const sortedRoles = [...payload.roles].sort((a, b) => { | |
| const priority = { SUPER_ADMIN: 0, ADMIN: 1, EMPLOYEE: 2, CUSTOMER: 3 }; | |
| return (priority[a as keyof typeof priority] ?? 99) - (priority[b as keyof typeof priority] ?? 99); | |
| }); | |
| setActiveRole(sortedRoles[0]); | |
| } | |
| const roles = payload?.roles ?? []; | |
| if (roles.length > 0) { | |
| const sortedRoles = [...roles].sort((a, b) => { | |
| const priority = { SUPER_ADMIN: 0, ADMIN: 1, EMPLOYEE: 2, CUSTOMER: 3 }; | |
| return (priority[a as keyof typeof priority] ?? 99) - (priority[b as keyof typeof priority] ?? 99); | |
| }); | |
| setActiveRole(sortedRoles[0]); | |
| } else { | |
| setActiveRole(''); | |
| } |
🤖 Prompt for AI Agents
In src/app/contexts/DashboardContext.tsx around lines 30 to 36, the code
currently preserves the previous activeRole when payload.roles is missing or
empty and in failure paths, causing stale privileged state; before
computing/sorting roles clear the activeRole (setActiveRole('')) so it cannot
remain set when no roles are returned, then if payload.roles exists and
non-empty setActiveRole(sortedRoles[0]); additionally ensure any catch/failure
branch also calls setActiveRole('') so fetch errors cannot leave a stale role
value.
| useEffect(() => { | ||
| const autoCheckAvailability = async () => { | ||
| if (!form.serviceTypeId || !form.date) { | ||
| setAvailability(null) | ||
| return | ||
| } | ||
|
|
||
| const duration = selectedServiceType?.estimatedDurationMinutes ?? 60 | ||
| const duration = selectedServiceType?.estimatedDurationMinutes ?? 60 | ||
|
|
||
| try { | ||
| setCheckingAvailability(true) | ||
| const result = await appointmentService.checkAvailability({ | ||
| date: form.date, | ||
| serviceType: form.serviceTypeId, | ||
| duration, | ||
| }) | ||
| setAvailability(result) | ||
| setError(null) | ||
| } catch (err: unknown) { | ||
| const message = (err as { response?: { data?: { message?: string } } })?.response?.data?.message || | ||
| 'Failed to check availability' | ||
| setError(message) | ||
| setAvailability(null) | ||
| } finally { | ||
| setCheckingAvailability(false) | ||
| try { | ||
| setCheckingAvailability(true) | ||
| const result = await appointmentService.checkAvailability({ | ||
| date: form.date, | ||
| serviceType: selectedServiceType?.name || '', | ||
| duration, | ||
| }) | ||
| setAvailability(result) | ||
| setError(null) | ||
| } catch (err: unknown) { | ||
| const message = (err as { response?: { data?: { message?: string } } })?.response?.data?.message || | ||
| 'Failed to check availability' | ||
| setError(message) | ||
| setAvailability(null) | ||
| } finally { | ||
| setCheckingAvailability(false) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const timer = setTimeout(autoCheckAvailability, 300) // Debounce by 300ms | ||
| return () => clearTimeout(timer) | ||
| }, [form.serviceTypeId, form.date, selectedServiceType]) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify service name uniqueness for availability check.
The availability check now uses selectedServiceType?.name (Line 110) instead of an ID. If service names aren't unique in the database, this could return incorrect availability data.
Run the following script to check if service names are used as unique identifiers in the backend:
🏁 Script executed:
#!/bin/bash
# Search for service type lookups by name in appointment/availability logic
rg -nP --type=ts --type=java -C3 'serviceType.*name|findByName|getByServiceType' -g '!node_modules' -g '!dist'Length of output: 1985
Need to verify the appointmentService implementation and whether service type IDs are available. Let me investigate further.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find appointmentService implementation and checkAvailability method
rg -nP --type=ts 'checkAvailability|appointmentService' -C5 -g '!node_modules' -g '!dist' | head -100
Length of output: 7752
🏁 Script executed:
#!/bin/bash
# Search for selectedServiceType construction to see what fields are available
rg -nP --type=ts 'selectedServiceType\s*=' -C3 -g '!node_modules' -g '!dist'Length of output: 1157
🏁 Script executed:
#!/bin/bash
# Look for service type interface/type definitions
rg -nP --type=ts '(type|interface).*ServiceType' -C3 -g '!node_modules' -g '!dist'Length of output: 2687
🏁 Script executed:
#!/bin/bash
# Check the form structure for serviceTypeId
rg -nP --type=ts 'form\.serviceTypeId|serviceTypeId' -C3 -g '!node_modules' -g '!dist' | head -80Length of output: 6279
🏁 Script executed:
#!/bin/bash
# Look for availability API endpoint or backend logic
fd -e ts -e tsx -e java -g '!node_modules' -g '!dist' | xargs grep -l 'availability\|checkAvailability' | head -10Length of output: 92
I found a critical discrepancy. Let me verify the availability page implementation more carefully.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full context of the refreshAvailability function in availability page
cat src/app/dashboard/appointments/availability/page.tsx | grep -A 20 "const refreshAvailability"
Length of output: 746
🏁 Script executed:
#!/bin/bash
# Search for backend API handlers or routes that handle availability endpoint
rg -nP 'appointments/availability|checkAvailability' --type=ts --type=java -C5 -g '!node_modules' -g '!dist'Length of output: 3818
🏁 Script executed:
#!/bin/bash
# Check if there's database logic for service type lookup
rg -nP 'findByName|findById|WHERE.*name|WHERE.*id' --type=java --type=ts -C3 -g '!node_modules' -g '!dist' | grep -i service | head -50Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the full context around line 110 in book/page.tsx to understand the code flow
sed -n '95,130p' src/app/dashboard/appointments/book/page.tsxLength of output: 1163
Change book/page.tsx to pass service type ID instead of name, matching availability/page.tsx pattern.
The availability check reveals an inconsistency between two pages calling the same API. In availability/page.tsx (line 98), the code passes serviceTypeId (the ID), but in book/page.tsx (line 110), it passes selectedServiceType?.name (the name). The book page should use the ID to match the pattern in the availability page, which is the standard approach for database lookups.
Change line 110 from:
serviceType: selectedServiceType?.name || '',
to:
serviceType: selectedServiceType?.id || '',
🤖 Prompt for AI Agents
In src/app/dashboard/appointments/book/page.tsx around lines 97 to 127, the
availability call is passing the service type name instead of its ID; update the
payload so serviceType uses the selectedServiceType?.id (fallback to empty
string) instead of selectedServiceType?.name, ensuring the checkAvailability
call matches availability/page.tsx and uses the ID for lookups.
| function DashboardShellWithNotifications({ children }: { children: ReactNode }) { | ||
| const { profile } = useDashboard() | ||
|
|
||
| return ( | ||
| <NotificationProvider userId={profile?.id ? String(profile.id) : null}> | ||
| <DashboardShell>{children}</DashboardShell> | ||
| </NotificationProvider> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Verify safe conversion of userId for NotificationProvider.
The userId prop converts profile?.id to a string without checking if id is defined. If profile.id is undefined, this results in the string "undefined" being passed instead of null.
Apply this diff to safely handle the conversion:
function DashboardShellWithNotifications({ children }: { children: ReactNode }) {
const { profile } = useDashboard()
return (
- <NotificationProvider userId={profile?.id ? String(profile.id) : null}>
+ <NotificationProvider userId={profile?.id != null ? String(profile.id) : null}>
<DashboardShell>{children}</DashboardShell>
</NotificationProvider>
)
}🤖 Prompt for AI Agents
In src/app/dashboard/layout.tsx around lines 30 to 38, the userId prop currently
does String(profile?.id) which will produce the literal "undefined" when
profile.id is undefined; change the conversion to only call String(...) when
profile?.id is not null/undefined and otherwise pass null (e.g. compute const
userId = profile?.id != null ? String(profile.id) : null and use
<NotificationProvider userId={userId}>), ensuring you check both null and
undefined so "undefined" is never sent.
| alt="Profile preview" | ||
| className="w-full h-auto object-cover" | ||
| /> | ||
| <Image src={previewUrl || photo || ""} alt="Profile preview" width={400} height={300} className="w-full h-auto object-cover" /> |
There was a problem hiding this comment.
Prevent Next.js Image error with empty src.
The Image component will throw an error if src is an empty string. The expression previewUrl || photo || "" can evaluate to "" when both are falsy.
Apply this diff to render the Image only when a valid source exists:
- <Image src={previewUrl || photo || ""} alt="Profile preview" width={400} height={300} className="w-full h-auto object-cover" />
+ {(previewUrl || photo) && (
+ <Image src={previewUrl || photo} alt="Profile preview" width={400} height={300} className="w-full h-auto object-cover" />
+ )}📝 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.
| <Image src={previewUrl || photo || ""} alt="Profile preview" width={400} height={300} className="w-full h-auto object-cover" /> | |
| {(previewUrl || photo) && ( | |
| <Image src={previewUrl || photo} alt="Profile preview" width={400} height={300} className="w-full h-auto object-cover" /> | |
| )} |
🤖 Prompt for AI Agents
In src/components/ProfilePhotoUpload.tsx around line 158, the Image src
currently falls back to an empty string (previewUrl || photo || ""), which can
cause Next.js Image to throw when src is empty; change the component to only
render the Image when a valid source exists (e.g., check if previewUrl or photo
is truthy) and otherwise render a fallback (null or a placeholder element),
ensuring the conditional prevents passing an empty string to the Image
component.
4 participants
Summary by CodeRabbit
New Features
Access Control
UI / UX
Documentation