Refactor: break up monolithic +page.svelte into 14 files#38
Refactor: break up monolithic +page.svelte into 14 files#38TheOneWhoBurns merged 2 commits intorental-systemfrom
Conversation
Rate limiting was in-memory (Map), which breaks under multi-instance scaling since each instance tracks independently. Now uses a rate_limits table with upsert logic and piggyback cleanup. Operator cookie was an unsigned plain integer, forgeable by anyone who knows an ID. Now HMAC-signed with COOKIE_SECRET env var and verified via getVerifiedOperatorId() across all endpoints. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the 4076-line monolith with a layered architecture: - Service layer (7 files): typed fetch wrappers for each API domain - Toast store: shared notification state - Tab sections (5 files): smart containers owning domain modals - Page orchestrator (~260 lines): tabs, shared DeleteModal, cross-tab wiring Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive modal-driven UI layer for managing rentals, reservations, and tours, including new delete/edit modals with passcode verification, a service layer for API communication, database-backed rate limiting, cookie security enhancements, and toast notifications. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@src/lib/components/DeleteModal.svelte`:
- Around line 59-71: Replace the native <input> in DeleteModal.svelte with the
Material Web text field component (e.g., the MW3 textfield used in the project)
so the control is consistent and accessible; keep the same attributes
(type="password", maxlength="4", inputmode="numeric", placeholder) but bind the
component's .value explicitly to the passcode variable via an on:input handler
that sets passcode (do not use Svelte bind:value), and preserve the existing
Enter key behavior by handling on:keydown to call submit(). Ensure the label is
properly associated with the text field (use the component's label prop or a
for/id pairing) and remove the standalone span label so accessibility semantics
come from the material component.
In `@src/lib/components/RentalCard.svelte`:
- Around line 46-98: The card uses a <button> wrapper but nests interactive
controls (md-icon-button, md-filled-tonal-button), which is invalid; change the
outer element in RentalCard.svelte from a <button class="rental-card"> to a
non-interactive <div class="rental-card" role="button" tabindex="0">, keep the
existing onclick/onEdit invocation by wiring keyboard activation (handle
Enter/Space to call onEdit(rental.id)) and preserve disabled state via
aria-disabled or a data-disabled attribute so inner buttons remain operable;
update CSS selectors that relied on :disabled (e.g., .rental-card:disabled) to
target the new attribute/class instead and ensure onDelete and onClose handlers
(onDelete, onClose) still call e.stopPropagation() to prevent outer activation.
In `@src/lib/components/ReservationCard.svelte`:
- Around line 28-33: The reservation card button ('button' element with class
"reservation-card" that calls onEdit(reservation) and is disabled by {loading})
lacks an accessible name for screen readers; add an accessible name by providing
an appropriate aria-label (or aria-labelledby to reference visible text inside
the card) that describes the action and reservation (e.g., "Open reservation for
[guest/name]" or similar), ensuring the label updates if reservation info
changes and that it is present even when the button is disabled.
In `@src/lib/components/TourBookingCard.svelte`:
- Around line 20-24: The button with class "tour-card" used to trigger
onEdit(booking) lacks an accessible name; add an aria-label to the button (in
TourBookingCard.svelte) that clearly describes the action such as "Edit booking"
plus a unique booking identifier or title (e.g., include booking.id or
booking.name when available) so screen readers know what will be edited; keep
the aria-label dynamic alongside the existing onclick/onEdit,
disabled={loading}, and booking usage.
In `@src/lib/sections/PreviousRentalsTab.svelte`:
- Around line 4-8: Guard against a missing rentals prop by giving it a default
empty array when destructuring from $props; change the current let { rentals }:
{ rentals: any[] } = $props() to ensure rentals is initialized (e.g., set
rentals = [] if $props().rentals is undefined) so usages like rentals.length
won't throw in PreviousRentalsTab.svelte.
- Around line 11-15: The decorative icon in PreviousRentalsTab.svelte is missing
ARIA handling; update the <span class="material-symbols-rounded
empty-icon">history</span> element to include aria-hidden="true" so screen
readers ignore it, matching the pattern used in EditRentalModal.svelte and
CreateRentalModal.svelte.
In `@src/lib/sections/StoreSalesTab.svelte`:
- Around line 60-62: Replace the plain <button class="delete-btn"> element with
a Material Web 3 md-icon-button while preserving the onclick handler and
aria-label: use the md-icon-button component (the element that matches your
project’s MD3 import) and wire its onclick to the existing
onPromptDelete('storeSale', sale.id, sale.productName || 'Sale') call, keep the
delete icon inside the md-icon-button (using the same material icon name
"delete") and retain aria-label="Delete sale" so accessibility and event
handling via the onPromptDelete function remain unchanged.
In `@src/lib/server/auth.ts`:
- Around line 96-104: In verifyCookieValue, guard against malformed hex
signatures before calling timingSafeEqual: validate that the sig part contains
only hex characters (e.g., match against /^[0-9a-fA-F]+$/) and create a Buffer
from the expected signature (Buffer.from(expected, 'hex')) and compare its
length to Buffer.from(sig, 'hex'); if the sig has non-hex chars or the buffers
differ in length, return null instead of calling timingSafeEqual. This ensures
createHmac/expected and Buffer.from(sig, 'hex') are safe to compare in
timingSafeEqual.
In `@src/lib/services/shift.service.ts`:
- Around line 14-37: endShift currently skips HTTP error checking and opens
report links without noopener; update endShift to check res.ok after fetch and
handle non-OK responses by parsing JSON or text (similar to api.ts) and
throwing/returning a structured error, ensuring callers can handle failures, and
when opening the returned URL use a safe opener (e.g., window.open(data.url,
'_blank', 'noopener,noreferrer') or create an anchor with rel="noopener
noreferrer" and target="_blank") to prevent tab-nabbing; also preserve existing
blob download flow (URL.createObjectURL, a.download, cleanup) and ensure errors
during blob parsing are caught and surfaced.
🧹 Nitpick comments (16)
src/lib/server/db/schema.ts (1)
206-210: Add an index onreset_atfor cleanup queries.
The cleanup path filters byreset_at < now; an index keeps this fast as the table grows.♻️ Suggested index
-export const rateLimits = pgTable('rate_limits', { - key: text('key').primaryKey(), - count: integer('count').notNull().default(1), - resetAt: timestamp('reset_at', { withTimezone: true }).notNull() -}); +export const rateLimits = pgTable( + 'rate_limits', + { + key: text('key').primaryKey(), + count: integer('count').notNull().default(1), + resetAt: timestamp('reset_at', { withTimezone: true }).notNull() + }, + (table) => [index('idx_rate_limits_reset_at').on(table.resetAt)] +);As per coding guidelines, "src/**/*.ts: Review TypeScript files for: Database query efficiency".
src/lib/services/api.ts (1)
1-21: Throw anErrorinstance so stacks/instanceofchecks work.
Right now a plain object is thrown; wrapping it preserves stack traces and common error handling.🛠️ Suggested adjustment
-export interface ApiError { - message: string; - status: number; - data?: any; -} +export interface ApiError extends Error { + status: number; + data?: unknown; +} @@ - const err: ApiError = { - message: body.error || body.message || res.statusText, - status: res.status, - data: body - }; - throw err; + const err = Object.assign( + new Error(body.error || body.message || res.statusText), + { status: res.status, data: body } + ) as ApiError; + throw err;As per coding guidelines, "src/**/*.ts: Review TypeScript files for: Type safety and proper typing; API error handling".
src/lib/services/shift.service.ts (1)
3-12: Guard against non-array payloads (and type checklist items).
If the endpoint returns a non-array payload,.filterwill throw; a small guard keeps it safe and improves typing. Consider typing the shift summary response similarly.✅ Safer parsing
import { apiGet, apiPost } from './api'; + +type ClosingChecklistItem = { isActive?: boolean }; @@ const res = await apiGet('/api/closing-checklist'); const data = await res.json(); - return data.filter((item: any) => item.isActive !== false); + const items: ClosingChecklistItem[] = Array.isArray(data) ? data : []; + return items.filter((item) => item.isActive !== false);As per coding guidelines, "src/**/*.ts: Review TypeScript files for: Type safety and proper typing".
src/lib/sections/StoreSalesTab.svelte (1)
6-18: TypestoreSales/storeProductsprops instead ofany[].
Stronger typing will make template usage and service responses safer.As per coding guidelines, "**/*.{ts,tsx}: Use TypeScript for type-safe development in both backend and frontend code".
src/lib/services/rental.service.ts (1)
3-56: Introduce request/response types instead ofany.
These services are a central API surface; typing payloads and responses will make callers safer and clarify conflict shapes.As per coding guidelines, "/*.{ts,tsx}: Use TypeScript for type-safe development in both backend and frontend code" and "src//*.ts: Review TypeScript files for: Type safety and proper typing".
src/routes/api/guides/verify/+server.ts (1)
18-18: Consider wrappingrequest.json()in try-catch for consistency.Other routes in this PR (e.g.,
src/routes/api/admin/login/+server.tsLines 25-29,src/routes/api/operators/verify/+server.tsLines 19-23) wraprequest.json()in try-catch to return a 400 error for malformed JSON. This route lacks that handling, which could result in an unhandled exception on invalid JSON input.Suggested fix
- const { guideId, passcode } = await request.json(); + let body; + try { + body = await request.json(); + } catch { + return json({ error: 'Invalid request body' }, { status: 400 }); + } + + const { guideId, passcode } = body;src/lib/services/guide.service.ts (1)
3-9: Rate-limit errors are silently swallowed.When the server returns a 429 status with
Retry-Afterheader, this function returnsfalsewithout informing the caller about the rate-limiting. Users won't see feedback about why verification failed or when to retry.Consider returning additional context or throwing a specific error for rate-limiting scenarios so the UI can display an appropriate message.
Suggested approach
+export interface VerifyResult { + success: boolean; + rateLimited?: boolean; + retryAfter?: number; +} + -export async function verifyGuidePin(guideId: number, passcode: string): Promise<boolean> { +export async function verifyGuidePin(guideId: number, passcode: string): Promise<VerifyResult> { try { await apiPost('/api/guides/verify', { guideId, passcode }); - return true; - } catch { - return false; + return { success: true }; + } catch (e: any) { + if (e?.status === 429) { + return { success: false, rateLimited: true, retryAfter: e.retryAfter }; + } + return { success: false }; } }src/routes/api/shifts/start/+server.ts (1)
19-19: Missing try-catch forrequest.json().Similar to the guides/verify route, this endpoint doesn't wrap
request.json()in try-catch, whileadmin/loginandoperators/verifydo. Malformed JSON will cause an unhandled exception.Suggested fix
- const { operatorId, passcode } = await request.json(); + let body; + try { + body = await request.json(); + } catch { + return json({ error: 'Invalid request body' }, { status: 400 }); + } + + const { operatorId, passcode } = body;src/routes/+layout.server.ts (1)
14-32: Consider parallelizing operator and shift queries.The operator and shift queries are independent and could be fetched in parallel to reduce latency, similar to the pattern used in
src/routes/api/shifts/close/+server.ts(Lines 61-64).Suggested optimization
- const [operator] = await db - .select() - .from(operators) - .where(and(eq(operators.id, operatorId), eq(operators.isActive, true))); - - if (!operator) { - cookies.delete('operatorId', { path: '/' }); - return { operator: null, shift: null }; - } - - const [activeShift] = await db - .select() - .from(shifts) - .where(and(eq(shifts.operatorId, operatorId), isNull(shifts.endedAt))); + const [operatorResult, shiftResult] = await Promise.all([ + db.select().from(operators).where(and(eq(operators.id, operatorId), eq(operators.isActive, true))), + db.select().from(shifts).where(and(eq(shifts.operatorId, operatorId), isNull(shifts.endedAt))) + ]); + + const [operator] = operatorResult; + const [activeShift] = shiftResult; + + if (!operator) { + cookies.delete('operatorId', { path: '/' }); + return { operator: null, shift: null }; + }src/lib/stores/toast.ts (1)
3-21: Add aninfohelper (or drop theinfovariant).
Right now the union includes'info'but the store API never emits it.♻️ Suggested fix
function createToastStore() { const { subscribe, set } = writable<ToastState>({ message: '', variant: 'success', visible: false }); return { subscribe, success: (message: string) => set({ message, variant: 'success', visible: true }), error: (message: string) => set({ message, variant: 'error', visible: true }), warning: (message: string) => set({ message, variant: 'warning', visible: true }), + info: (message: string) => set({ message, variant: 'info', visible: true }), dismiss: () => set({ message: '', variant: 'success', visible: false }) }; }src/lib/sections/ActiveRentalsTab.svelte (1)
16-53: Replaceanywith concrete types for props/state.
This file is in TS mode; typed interfaces will prevent downstream runtime errors and align with the repo’s type‑safety guideline.♻️ Suggested approach
<script lang="ts"> + type Rental = { id: number; /* extend with fields used by modals/cards */ }; + type Product = { id: number; }; + type TrackedItem = { id: number; }; + type Category = { id: number; }; + type Guide = { id: number; }; + type Conflict = { id: number; }; + type RentalPayload = Record<string, unknown>; let { rentals, products, trackedItems, categories, guides, shiftId, operatorId, prefillFromReservation = null, fromReservationId = null, onDataChanged, onPromptDelete, onClearPrefill }: { - rentals: any[]; - products: any[]; - trackedItems: any[]; - categories: any[]; - guides: any[]; + rentals: Rental[]; + products: Product[]; + trackedItems: TrackedItem[]; + categories: Category[]; + guides: Guide[]; shiftId: number; operatorId: number; - prefillFromReservation?: any; + prefillFromReservation?: RentalPayload | null; fromReservationId?: number | null; onDataChanged: () => void; onPromptDelete: (type: string, id: number, label: string) => void; onClearPrefill: () => void; } = $props(); - let selectedRental = $state<any>(null); - let conflictData = $state<any[]>([]); - let pendingPayload = $state<any>(null); + let selectedRental = $state<Rental | null>(null); + let conflictData = $state<Conflict[]>([]); + let pendingPayload = $state<RentalPayload | null>(null);As per coding guidelines: “Use TypeScript for type-safe development in both backend and frontend code.”
Also applies to: 62-127
src/lib/sections/ReservationsTab.svelte (1)
9-71: Avoidanyin props/state—prefer explicit types.
This keeps the reservation flow type‑safe and consistent with TS usage across the repo.♻️ Suggested approach
<script lang="ts"> + type Reservation = { id: number; /* extend as needed */ }; + type Product = { id: number; }; + type TrackedItem = { id: number; }; + type Category = { id: number; }; + type Guide = { id: number; }; + type ReservationPayload = Record<string, unknown>; let { reservations, products, trackedItems, categories, guides, onDataChanged, onPromptDelete, onStartRentalFromReservation }: { - reservations: any[]; - products: any[]; - trackedItems: any[]; - categories: any[]; - guides: any[]; + reservations: Reservation[]; + products: Product[]; + trackedItems: TrackedItem[]; + categories: Category[]; + guides: Guide[]; onDataChanged: () => void; onPromptDelete: (type: string, id: number, label: string) => void; - onStartRentalFromReservation: (reservationId: number, prefill: any) => void; + onStartRentalFromReservation: (reservationId: number, prefill: ReservationPayload) => void; } = $props(); @@ - let selectedReservation = $state<any>(null); + let selectedReservation = $state<Reservation | null>(null);As per coding guidelines: “Use TypeScript for type-safe development in both backend and frontend code.”
src/lib/sections/ToursTab.svelte (1)
17-24: Consider adding TypeScript interfaces for props instead ofany[].The component uses
anyfor all array props (tourBookings,tourProducts,guides) and theselectedBookingstate. This reduces type safety and IDE support. Consider defining or importing interfaces that match the API response shapes.♻️ Suggested type definitions
+interface TourBooking { + id: number; + productName: string; + pax: number; + totalPrice: number; + unitPrice: number; + activityDate: string; + tourProductId: number; + guideId?: number | null; + status: string; +} + +interface TourProduct { + id: number; + name: string; + price: number; +} + +interface Guide { + id: number; + name: string; +} + let { tourBookings, tourProducts, guides, shiftId, onDataChanged, onPromptDelete }: { - tourBookings: any[]; - tourProducts: any[]; - guides: any[]; + tourBookings: TourBooking[]; + tourProducts: TourProduct[]; + guides: Guide[]; shiftId: number; onDataChanged: () => void; onPromptDelete: (type: string, id: number, label: string) => void; } = $props();As per coding guidelines: "Use TypeScript for type-safe development in both backend and frontend code"
src/lib/components/EditReservationModal.svelte (1)
45-63: Potential date format mismatch between display and stored value.The flatpickr
dateFormatis set to'Y-m-d H:i'for display, butonChangestores the full ISO string viadate.toISOString(). This works correctly for submission, but when the modal reopens, thedefaultDatereceives an ISO string which flatpickr can parse. However, if users see2024-01-15 10:30in the input but the value is2024-01-15T10:30:00.000Z, this could cause timezone-related confusion.Consider using a consistent format or explicitly handling timezone conversion if the business requires local time display.
src/lib/components/EditTourModal.svelte (1)
86-91: Handle emptytourProductsarray edge case.If
tourProductsis empty, the select will have no options, andproductIdcould remain as the initial booking value or become invalid. Consider adding validation or a placeholder option.♻️ Suggested improvement
<select class="form-select" bind:value={productId} disabled={loading || saving}> + {`#if` tourProducts.length === 0} + <option value={null} disabled>No products available</option> + {/if} {`#each` tourProducts as product} <option value={product.id}>{product.name} - ${(product.price / 100).toFixed(2)}/person</option> {/each} </select>src/lib/components/RentalCard.svelte (1)
88-96: Good event handling with stopPropagation, but redundant calls.The
stopPropagationon the parentcard-actionsdiv (line 88) already prevents clicks from bubbling to the card. The additionale.stopPropagation()calls inside each button handler (lines 89, 92) are redundant but harmless.The
aria-label="Delete rental"on the delete button is good for accessibility. Consider adding an explicitaria-labelto the close button as well for consistency, though the visible "Close" text provides sufficient context.
| <label class="form-label"> | ||
| <span class="material-symbols-rounded">lock</span> | ||
| <span class="md-title-small">Enter Passcode</span> | ||
| </label> | ||
| <input | ||
| type="password" | ||
| maxlength="4" | ||
| inputmode="numeric" | ||
| placeholder="4-digit passcode" | ||
| bind:value={passcode} | ||
| class="passcode-input" | ||
| onkeydown={(e) => { if (e.key === 'Enter') submit(); }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Material Web 3 text field component API Svelte binding
💡 Result:
@material/web Material 3 text fields (<md-filled-text-field>, <md-outlined-text-field>) expose a value property and fire native input / change events (bubbling; input is also composed), so in Svelte you typically bind by wiring value + on:input. [1]
Svelte (recommended): “controlled” binding
<script>
import '@material/web/textfield/filled-text-field.js';
// or: import '@material/web/textfield/outlined-text-field.js';
let value = '';
</script>
<md-filled-text-field
label="Name"
{value}
on:input={(e) => (value = e.currentTarget.value)}
/>This uses Svelte’s normal event handling and updates your state from the web component’s value. [1]
Why not bind:value directly?
Svelte’s built-in bind:value behavior is specifically documented for native form elements like <input> (where Svelte knows exactly which event to listen to). For web components, the reliable cross-version approach is the explicit value + on:input pattern above. [2]
Sources: Material Web text field API/events [1], Svelte binding docs [2].
[1] (material-web.dev)
[2] (svelte.dev)
Citations:
- 1: https://material-web.dev/components/text-field/?utm_source=openai
- 2: https://svelte.dev/docs/svelte/bind?utm_source=openai
🌐 Web query:
Material Design 3 md-filled-text-field maxlength password type attributes
💡 Result:
In Material Design 3 / Material Web, <md-filled-text-field> forwards common <input> constraints and supports:
-
Password field: set the
typeattribute topassword<md-filled-text-field label="Password" type="password"></md-filled-text-field>
(
typeis the underlying HTML input type.) [1] -
Max length: set
maxlength(attribute) /maxLength(JS property). Whenmaxlengthis present, the component shows a character counter automatically.<md-filled-text-field label="PIN" type="password" maxlength="12"></md-filled-text-field>
Material Web docs:
maxLength↔maxlength, where-1means “no limit”. [1]
Notes:
maxlengthis an HTML constraint attribute (integer ≥ 0; measured in UTF‑16 code units). [2]- For
<input type="password">,maxlengthis supported and participates in constraint validation. [3]
Sources: Material Web text-field docs [1], MDN maxlength [2], MDN password input attributes [3].
[1] (material-web.dev)
[2] (developer.mozilla.org)
[3] (developer.mozilla.org)
Citations:
- 1: https://material-web.dev/components/text-field/?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/maxlength?utm_source=openai
- 3: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/password?utm_source=openai
🏁 Script executed:
# Check if the file exists and examine the current implementation
cat -n src/lib/components/DeleteModal.svelte | sed -n '59,71p'Repository: TheOneWhoBurns/cpoint
Length of output: 559
Use a Material Web text field component instead of native <input> for UI consistency and accessibility.
The current native <input> doesn't follow the "Material Web 3 components" guideline, and the label isn't properly associated with the control. Additionally, bind:value is unreliable with web components—use explicit value property binding and on:input instead.
♻️ Suggested fix (align with Material Web 3 + accessibility)
<script lang="ts">
import '@material/web/button/filled-button.js';
import '@material/web/button/outlined-button.js';
import '@material/web/iconbutton/icon-button.js';
+ import '@material/web/textfield/filled-text-field.js';
@@
- <label class="form-label">
+ <label class="form-label" for="delete-passcode">
<span class="material-symbols-rounded">lock</span>
<span class="md-title-small">Enter Passcode</span>
</label>
- <input
- type="password"
- maxlength="4"
- inputmode="numeric"
- placeholder="4-digit passcode"
- bind:value={passcode}
- class="passcode-input"
- onkeydown={(e) => { if (e.key === 'Enter') submit(); }}
- />
+ <md-filled-text-field
+ id="delete-passcode"
+ type="password"
+ label="Enter Passcode"
+ maxlength="4"
+ inputmode="numeric"
+ placeholder="4-digit passcode"
+ value={passcode}
+ on:input={(e) => (passcode = e.currentTarget.value)}
+ class="passcode-input"
+ onkeydown={(e) => { if (e.key === 'Enter') submit(); }}
+ />Per coding guidelines: "Use Material Web 3 components for UI consistency across the tablet-optimized interface" and "Review Svelte components for Accessibility (ARIA labels, keyboard navigation)."
🤖 Prompt for AI Agents
In `@src/lib/components/DeleteModal.svelte` around lines 59 - 71, Replace the
native <input> in DeleteModal.svelte with the Material Web text field component
(e.g., the MW3 textfield used in the project) so the control is consistent and
accessible; keep the same attributes (type="password", maxlength="4",
inputmode="numeric", placeholder) but bind the component's .value explicitly to
the passcode variable via an on:input handler that sets passcode (do not use
Svelte bind:value), and preserve the existing Enter key behavior by handling
on:keydown to call submit(). Ensure the label is properly associated with the
text field (use the component's label prop or a for/id pairing) and remove the
standalone span label so accessibility semantics come from the material
component.
| <button | ||
| class="rental-card" | ||
| class:hourly={pricing?.type === 'hourly'} | ||
| class:fullday={pricing?.type !== 'hourly'} | ||
| onclick={() => onEdit(rental.id)} | ||
| disabled={loading} | ||
| > | ||
| <div class="card-header"> | ||
| <div class="customer-info"> | ||
| <span class="material-symbols-rounded customer-icon">person</span> | ||
| <div class="avatar"> | ||
| <span class="material-symbols-rounded">person</span> | ||
| </div> | ||
| <div class="customer-details"> | ||
| <span class="md-title-medium">{customer?.name || 'Unknown'}</span> | ||
| <span class="name">{customer?.name || 'Unknown'}</span> | ||
| {#if customer?.hotel} | ||
| <span class="md-body-small hotel-text"> | ||
| <span class="meta"> | ||
| <span class="material-symbols-rounded icon-xs">hotel</span> | ||
| {customer.hotel} | ||
| </span> | ||
| {/if} | ||
| {#if guideName} | ||
| <span class="md-body-small guide-text"> | ||
| <span class="material-symbols-rounded icon-xs">hiking</span> | ||
| {guideName} | ||
| </span> | ||
| {/if} | ||
| </div> | ||
| </div> | ||
| <div class="rental-type-badge" class:hourly={pricing?.type === 'hourly'} class:fullday={pricing?.type !== 'hourly'}> | ||
| <span class="type-badge" class:hourly={pricing?.type === 'hourly'}> | ||
| {pricing?.type === 'hourly' ? 'Hourly' : 'Full Day'} | ||
| </div> | ||
| </span> | ||
| </div> | ||
|
|
||
| <div class="rental-items-list"> | ||
| <div class="items-list"> | ||
| {#each items as item} | ||
| <div class="item-chip"> | ||
| <span class="material-symbols-rounded icon-sm"> | ||
| {item.code ? 'qr_code_2' : 'inventory_2'} | ||
| </span> | ||
| <span class="md-body-small"> | ||
| {item.name}{item.code ? ` (${item.code})` : ''}{item.quantity ? ` x${item.quantity}` : ''} | ||
| </span> | ||
| <span class="material-symbols-rounded icon-sm">{item.code ? 'qr_code_2' : 'inventory_2'}</span> | ||
| <span>{item.name}{item.code ? ` (${item.code})` : ''}{item.quantity ? ` x${item.quantity}` : ''}</span> | ||
| </div> | ||
| {/each} | ||
| </div> | ||
|
|
||
| <div class="rental-footer"> | ||
| <div class="card-footer"> | ||
| <div class="time-info"> | ||
| <span class="material-symbols-rounded icon-sm">schedule</span> | ||
| <span class="md-body-small">Started {new Date(rental.startedAt).toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' })}</span> | ||
| <span>{new Date(rental.startedAt).toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' })}</span> | ||
| <span class="elapsed-badge">{elapsedTime}</span> | ||
| </div> | ||
| <div class="rental-actions"> | ||
| <md-icon-button onclick={() => onEdit(rental.id)} disabled={loading} aria-label="Edit rental"> | ||
| <span class="material-symbols-rounded">edit</span> | ||
| <div class="card-actions" onclick={(e) => e.stopPropagation()}> | ||
| <md-icon-button onclick={(e: Event) => { e.stopPropagation(); onDelete(rental.id, customer?.name || 'Rental'); }} disabled={loading} aria-label="Delete rental"> | ||
| <span class="material-symbols-rounded delete-icon">delete</span> | ||
| </md-icon-button> | ||
| <md-filled-tonal-button onclick={() => onClose(rental.id)} disabled={loading}> | ||
| <md-filled-tonal-button onclick={(e: Event) => { e.stopPropagation(); onClose(rental.id); }} disabled={loading}> | ||
| <span class="material-symbols-rounded" slot="icon">check_circle</span> | ||
| Close Rental | ||
| Close | ||
| </md-filled-tonal-button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </button> |
There was a problem hiding this comment.
Invalid HTML: interactive elements nested inside a <button>.
The card is rendered as a <button> (line 46) but contains other interactive elements (<md-icon-button> on line 89, <md-filled-tonal-button> on line 92). Nesting buttons inside buttons is invalid HTML and causes accessibility issues—screen readers may not correctly announce or navigate to the inner controls.
Consider changing the card to a <div> with proper keyboard accessibility attributes.
♿ Proposed fix using a div with role and tabindex
-<button
+<div
+ role="button"
+ tabindex="0"
class="rental-card"
class:hourly={pricing?.type === 'hourly'}
class:fullday={pricing?.type !== 'hourly'}
onclick={() => onEdit(rental.id)}
+ onkeydown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); onEdit(rental.id); } }}
- disabled={loading}
->
+ aria-disabled={loading}
+>
<!-- ... card content ... -->
-</button>
+</div>Then update the disabled styling selector:
-.rental-card:disabled {
+.rental-card[aria-disabled="true"] {
opacity: 0.6;
cursor: not-allowed;
+ pointer-events: none;
}As per coding guidelines: "Review Svelte components for Accessibility (ARIA labels, keyboard navigation)".
🤖 Prompt for AI Agents
In `@src/lib/components/RentalCard.svelte` around lines 46 - 98, The card uses a
<button> wrapper but nests interactive controls (md-icon-button,
md-filled-tonal-button), which is invalid; change the outer element in
RentalCard.svelte from a <button class="rental-card"> to a non-interactive <div
class="rental-card" role="button" tabindex="0">, keep the existing
onclick/onEdit invocation by wiring keyboard activation (handle Enter/Space to
call onEdit(rental.id)) and preserve disabled state via aria-disabled or a
data-disabled attribute so inner buttons remain operable; update CSS selectors
that relied on :disabled (e.g., .rental-card:disabled) to target the new
attribute/class instead and ensure onDelete and onClose handlers (onDelete,
onClose) still call e.stopPropagation() to prevent outer activation.
| <button | ||
| class="reservation-card" | ||
| class:expired | ||
| onclick={() => onEdit(reservation)} | ||
| disabled={loading} | ||
| > |
There was a problem hiding this comment.
Add accessible name to the card button.
Same issue as TourBookingCard - the clickable card button needs an accessible name for screen reader users.
🔧 Suggested fix
<button
class="reservation-card"
class:expired
onclick={() => onEdit(reservation)}
disabled={loading}
+ aria-label="Edit reservation for {customer?.name || 'unknown customer'}"
>As per coding guidelines: "Accessibility (ARIA labels, keyboard navigation)"
📝 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.
| <button | |
| class="reservation-card" | |
| class:expired | |
| onclick={() => onEdit(reservation)} | |
| disabled={loading} | |
| > | |
| <button | |
| class="reservation-card" | |
| class:expired | |
| onclick={() => onEdit(reservation)} | |
| disabled={loading} | |
| aria-label="Edit reservation for {customer?.name || 'unknown customer'}" | |
| > |
🤖 Prompt for AI Agents
In `@src/lib/components/ReservationCard.svelte` around lines 28 - 33, The
reservation card button ('button' element with class "reservation-card" that
calls onEdit(reservation) and is disabled by {loading}) lacks an accessible name
for screen readers; add an accessible name by providing an appropriate
aria-label (or aria-labelledby to reference visible text inside the card) that
describes the action and reservation (e.g., "Open reservation for [guest/name]"
or similar), ensuring the label updates if reservation info changes and that it
is present even when the button is disabled.
| <button | ||
| class="tour-card" | ||
| onclick={() => onEdit(booking)} | ||
| disabled={loading} | ||
| > |
There was a problem hiding this comment.
Add accessible name to the card button.
The card is now a <button> for click-to-edit, but it lacks an accessible name for screen reader users. Consider adding aria-label to describe the action.
🔧 Suggested fix
<button
class="tour-card"
onclick={() => onEdit(booking)}
disabled={loading}
+ aria-label="Edit {booking.productName || 'booking'}"
>As per coding guidelines: "Accessibility (ARIA labels, keyboard navigation)"
📝 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.
| <button | |
| class="tour-card" | |
| onclick={() => onEdit(booking)} | |
| disabled={loading} | |
| > | |
| <button | |
| class="tour-card" | |
| onclick={() => onEdit(booking)} | |
| disabled={loading} | |
| aria-label="Edit {booking.productName || 'booking'}" | |
| > |
🤖 Prompt for AI Agents
In `@src/lib/components/TourBookingCard.svelte` around lines 20 - 24, The button
with class "tour-card" used to trigger onEdit(booking) lacks an accessible name;
add an aria-label to the button (in TourBookingCard.svelte) that clearly
describes the action such as "Edit booking" plus a unique booking identifier or
title (e.g., include booking.id or booking.name when available) so screen
readers know what will be edited; keep the aria-label dynamic alongside the
existing onclick/onEdit, disabled={loading}, and booking usage.
| let { | ||
| rentals | ||
| }: { | ||
| rentals: any[]; | ||
| } = $props(); |
There was a problem hiding this comment.
Guard against missing rentals prop to avoid runtime errors.
If a parent forgets to pass rentals, rentals.length will throw. Default to an empty array.
Proposed fix
-let {
- rentals
-}: {
- rentals: any[];
-} = $props();
+let {
+ rentals = []
+}: {
+ rentals?: any[];
+} = $props();📝 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.
| let { | |
| rentals | |
| }: { | |
| rentals: any[]; | |
| } = $props(); | |
| let { | |
| rentals = [] | |
| }: { | |
| rentals?: any[]; | |
| } = $props(); |
🤖 Prompt for AI Agents
In `@src/lib/sections/PreviousRentalsTab.svelte` around lines 4 - 8, Guard against
a missing rentals prop by giving it a default empty array when destructuring
from $props; change the current let { rentals }: { rentals: any[] } = $props()
to ensure rentals is initialized (e.g., set rentals = [] if $props().rentals is
undefined) so usages like rentals.length won't throw in
PreviousRentalsTab.svelte.
| {#if rentals.length === 0} | ||
| <div class="empty-state"> | ||
| <span class="material-symbols-rounded empty-icon">history</span> | ||
| <p class="md-body-large">No previous rentals</p> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find existing Material Web component usage to establish patterns
rg -n "<md-" -g "*.svelte" --max-count=20Repository: TheOneWhoBurns/cpoint
Length of output: 22089
🏁 Script executed:
# Find material-symbols-rounded usage to see how icons are currently handled
rg -n "material-symbols-rounded" -g "*.svelte" -B2 -A2Repository: TheOneWhoBurns/cpoint
Length of output: 50378
🏁 Script executed:
# Check if there are Material Web 3 components for empty states or cards
rg -n "empty" -g "*.svelte" -i --max-count=20Repository: TheOneWhoBurns/cpoint
Length of output: 6205
🏁 Script executed:
cat -n src/lib/sections/PreviousRentalsTab.svelte | head -50Repository: TheOneWhoBurns/cpoint
Length of output: 1278
🏁 Script executed:
# Check for Material Web 3 empty state or card components
rg -n "md-card|md-list|md-container" -g "*.svelte" --max-count=5Repository: TheOneWhoBurns/cpoint
Length of output: 47
🏁 Script executed:
# Check if Material Web 3 empty-state pattern uses containers/cards in other similar UI
rg -B2 -A2 "empty-state" src/lib/sections/ | head -40Repository: TheOneWhoBurns/cpoint
Length of output: 2583
🏁 Script executed:
# Verify icon accessibility patterns in Material Web components
rg "aria-hidden|role=\"presentation\"" -g "*.svelte" | head -10Repository: TheOneWhoBurns/cpoint
Length of output: 418
Mark the icon as decorative with aria-hidden="true".
The icon lacks ARIA handling. Add aria-hidden="true" to the <span class="material-symbols-rounded empty-icon">history</span> to prevent screen readers from announcing the decorative icon. This aligns with the project's accessibility standards (see similar patterns in EditRentalModal.svelte and CreateRentalModal.svelte).
🤖 Prompt for AI Agents
In `@src/lib/sections/PreviousRentalsTab.svelte` around lines 11 - 15, The
decorative icon in PreviousRentalsTab.svelte is missing ARIA handling; update
the <span class="material-symbols-rounded empty-icon">history</span> element to
include aria-hidden="true" so screen readers ignore it, matching the pattern
used in EditRentalModal.svelte and CreateRentalModal.svelte.
| <button class="delete-btn" onclick={() => onPromptDelete('storeSale', sale.id, sale.productName || 'Sale')} aria-label="Delete sale"> | ||
| <span class="material-symbols-rounded delete-icon">delete</span> | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/sections/StoreSalesTab.svelte | head -80 | tail -30Repository: TheOneWhoBurns/cpoint
Length of output: 1256
🏁 Script executed:
rg "md-icon-button" --type svelte -A 3 -B 1Repository: TheOneWhoBurns/cpoint
Length of output: 95
🏁 Script executed:
rg "onclick" --type svelte | head -20Repository: TheOneWhoBurns/cpoint
Length of output: 95
🏁 Script executed:
rg "on:click" --type svelte | head -20Repository: TheOneWhoBurns/cpoint
Length of output: 95
🏁 Script executed:
fd "*.svelte" -x grep -l "md-icon\|md-" {} \; 2>/dev/null | head -10Repository: TheOneWhoBurns/cpoint
Length of output: 47
🏁 Script executed:
rg "md-icon-button|md-icon" -A 2 | head -40Repository: TheOneWhoBurns/cpoint
Length of output: 2758
🏁 Script executed:
rg "on:click" -A 1 | head -40Repository: TheOneWhoBurns/cpoint
Length of output: 47
🏁 Script executed:
rg "onclick" -A 1 | head -40Repository: TheOneWhoBurns/cpoint
Length of output: 3189
🏁 Script executed:
find src -name "*.svelte" -exec grep -l "material\|md-" {} \; | head -10Repository: TheOneWhoBurns/cpoint
Length of output: 1762
🏁 Script executed:
cat src/lib/sections/StoreSalesTab.svelte | head -30Repository: TheOneWhoBurns/cpoint
Length of output: 876
Use a Material Web 3 md-icon-button with onclick handler for the delete action.
This keeps UI consistent with MD3 components used throughout the codebase and follows the established event-handling pattern.
🧩 Example adjustment
- <button class="delete-btn" onclick={() => onPromptDelete('storeSale', sale.id, sale.productName || 'Sale')} aria-label="Delete sale">
- <span class="material-symbols-rounded delete-icon">delete</span>
- </button>
+ <md-icon-button
+ class="delete-btn"
+ aria-label="Delete sale"
+ onclick={() => onPromptDelete('storeSale', sale.id, sale.productName || 'Sale')}
+ >
+ <span class="material-symbols-rounded">delete</span>
+ </md-icon-button>📝 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.
| <button class="delete-btn" onclick={() => onPromptDelete('storeSale', sale.id, sale.productName || 'Sale')} aria-label="Delete sale"> | |
| <span class="material-symbols-rounded delete-icon">delete</span> | |
| </button> | |
| <md-icon-button | |
| class="delete-btn" | |
| aria-label="Delete sale" | |
| onclick={() => onPromptDelete('storeSale', sale.id, sale.productName || 'Sale')} | |
| > | |
| <span class="material-symbols-rounded">delete</span> | |
| </md-icon-button> |
🤖 Prompt for AI Agents
In `@src/lib/sections/StoreSalesTab.svelte` around lines 60 - 62, Replace the
plain <button class="delete-btn"> element with a Material Web 3 md-icon-button
while preserving the onclick handler and aria-label: use the md-icon-button
component (the element that matches your project’s MD3 import) and wire its
onclick to the existing onPromptDelete('storeSale', sale.id, sale.productName ||
'Sale') call, keep the delete icon inside the md-icon-button (using the same
material icon name "delete") and retain aria-label="Delete sale" so
accessibility and event handling via the onPromptDelete function remain
unchanged.
| export function verifyCookieValue(signed: string): string | null { | ||
| const dot = signed.lastIndexOf('.'); | ||
| if (dot === -1) return null; | ||
| const value = signed.slice(0, dot); | ||
| const sig = signed.slice(dot + 1); | ||
| const expected = createHmac('sha256', getCookieSecret()).update(value).digest('hex'); | ||
| if (sig.length !== expected.length) return null; | ||
| if (!timingSafeEqual(Buffer.from(sig, 'hex'), Buffer.from(expected, 'hex'))) return null; | ||
| return value; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the specific lines
cat -n src/lib/server/auth.ts | sed -n '90,110p'Repository: TheOneWhoBurns/cpoint
Length of output: 1013
🌐 Web query:
Node.js Buffer.from hex invalid input behavior throws error
💡 Result:
Buffer.from(str, 'hex') does not throw on invalid hex input; it decodes what it can and then truncates.
- It truncates when it hits the first non-hex character (e.g.
Buffer.from('1ag123','hex')produces<Buffer 1a>). It also truncates if the string ends with a single trailing hex digit (odd length), e.g.Buffer.from('1a7','hex')produces<Buffer 1a>. No exception is thrown for these cases. [1]
Related (often confused) behavior:
buf.fill(value, 'hex')also truncates invalid characters, but throws if nothing valid remains (e.g.buf.fill('zz', 'hex')throws). [2]
Sources:
[1] Node.js Buffer docs (hex decoding truncation examples) https://nodejs.org/api/buffer.html
[2] Node.js Buffer docs (buf.fill() invalid hex throws when no valid data remains) https://nodejs.org/api/buffer.html
🌐 Web query:
Node.js timingSafeEqual throws on different buffer lengths
💡 Result:
crypto.timingSafeEqual(a, b) requires a and b to have the same byte length; if they differ, Node.js throws an error (rather than returning false) to preserve the guarantees of its constant‑time comparison algorithm. [1]
Fix/pattern (recommended): ensure equal-length inputs before calling it.
import { timingSafeEqual } from "node:crypto";
function safeEqual(a, b) {
if (a.length !== b.length) return false; // lengths must match
return timingSafeEqual(a, b);
}If you’re comparing values that may be different lengths (e.g., user input vs stored secret), a common approach is to compare fixed-length digests (e.g., both SHA-256 / HMAC outputs) so lengths are always equal. [1]
Source: Node.js crypto.timingSafeEqual docs (states same byte length required; error thrown otherwise). [1]
References:
[1] Node.js Crypto docs (crypto.timingSafeEqual), https://nodejs.org/api/crypto.html
Guard against malformed signature hex before timingSafeEqual.
Malformed cookies can cause timingSafeEqual to throw a 500 error. If the signature contains invalid hex characters, Buffer.from(sig, 'hex') silently truncates the input, producing a buffer shorter than expected. While the string length check passes, the resulting buffer lengths differ, causing timingSafeEqual to throw.
Validate hex and buffer lengths before comparing:
🛡️ Safer verification
const value = signed.slice(0, dot);
const sig = signed.slice(dot + 1);
const expected = createHmac('sha256', getCookieSecret()).update(value).digest('hex');
if (sig.length !== expected.length) return null;
+ if (!/^[0-9a-f]+$/i.test(sig)) return null;
- if (!timingSafeEqual(Buffer.from(sig, 'hex'), Buffer.from(expected, 'hex'))) return null;
+ const sigBuf = Buffer.from(sig, 'hex');
+ const expBuf = Buffer.from(expected, 'hex');
+ if (sigBuf.length !== expBuf.length) return null;
+ if (!timingSafeEqual(sigBuf, expBuf)) return null;
return value;🤖 Prompt for AI Agents
In `@src/lib/server/auth.ts` around lines 96 - 104, In verifyCookieValue, guard
against malformed hex signatures before calling timingSafeEqual: validate that
the sig part contains only hex characters (e.g., match against /^[0-9a-fA-F]+$/)
and create a Buffer from the expected signature (Buffer.from(expected, 'hex'))
and compare its length to Buffer.from(sig, 'hex'); if the sig has non-hex chars
or the buffers differ in length, return null instead of calling timingSafeEqual.
This ensures createHmac/expected and Buffer.from(sig, 'hex') are safe to compare
in timingSafeEqual.
| export async function endShift() { | ||
| const res = await fetch('/api/shifts/close', { method: 'POST' }); | ||
| const contentType = res.headers.get('content-type') || ''; | ||
|
|
||
| if (contentType.includes('application/json')) { | ||
| const data = await res.json(); | ||
| if (data.url) { | ||
| window.open(data.url, '_blank'); | ||
| } | ||
| return data; | ||
| } | ||
|
|
||
| const blob = await res.blob(); | ||
| const url = URL.createObjectURL(blob); | ||
| const a = document.createElement('a'); | ||
| a.href = url; | ||
| const disposition = res.headers.get('content-disposition') || ''; | ||
| const match = disposition.match(/filename="?([^"]+)"?/); | ||
| a.download = match?.[1] || 'shift-report.xlsx'; | ||
| document.body.appendChild(a); | ||
| a.click(); | ||
| document.body.removeChild(a); | ||
| URL.revokeObjectURL(url); | ||
| return { success: true }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "shift.service.ts" -type fRepository: TheOneWhoBurns/cpoint
Length of output: 100
🏁 Script executed:
cat src/lib/services/shift.service.tsRepository: TheOneWhoBurns/cpoint
Length of output: 1214
🏁 Script executed:
find . -name "api.ts" -o -name "api.js" | head -5Repository: TheOneWhoBurns/cpoint
Length of output: 90
🏁 Script executed:
cat src/lib/services/api.tsRepository: TheOneWhoBurns/cpoint
Length of output: 1375
Add error handling to endShift() and use noopener for report links to prevent tab-nabbing attacks.
The endShift() function uses bare fetch() without checking res.ok, bypassing the standard error handling pattern established in api.ts. It also calls window.open() without the noopener flag, exposing a tab-nabbing vulnerability where the opened page can access window.opener and redirect the original tab.
🛡️ Suggested hardening
export async function endShift() {
const res = await fetch('/api/shifts/close', { method: 'POST' });
+ if (!res.ok) {
+ let message = res.statusText;
+ try {
+ const body = await res.json();
+ message = body.error || body.message || message;
+ } catch {}
+ throw Object.assign(new Error(message), { status: res.status });
+ }
const contentType = res.headers.get('content-type') || '';
@@
if (contentType.includes('application/json')) {
const data = await res.json();
if (data.url) {
- window.open(data.url, '_blank');
+ window.open(data.url, '_blank', 'noopener');
}
return data;
}🤖 Prompt for AI Agents
In `@src/lib/services/shift.service.ts` around lines 14 - 37, endShift currently
skips HTTP error checking and opens report links without noopener; update
endShift to check res.ok after fetch and handle non-OK responses by parsing JSON
or text (similar to api.ts) and throwing/returning a structured error, ensuring
callers can handle failures, and when opening the returned URL use a safe opener
(e.g., window.open(data.url, '_blank', 'noopener,noreferrer') or create an
anchor with rel="noopener noreferrer" and target="_blank") to prevent
tab-nabbing; also preserve existing blob download flow (URL.createObjectURL,
a.download, cleanup) and ensure errors during blob parsing are caught and
surfaced.
1 participant
Summary
+page.sveltewith a layered architecture of 14 small, single-responsibility files (~1200 total lines)src/lib/services/) with typed fetch wrappers for each API domain (rentals, reservations, tours, store sales, shifts, guides)src/lib/sections/) as smart containers that own their domain modals and call services directly+page.svelteto a ~260-line orchestrator handling tabs, shared DeleteModal, and cross-tab wiring (e.g. start rental from reservation)Architecture
No changes to: server-side logic, API routes,
+page.server.ts, or existing modal/card components.Test plan
npm run buildpasses🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Security Improvements