Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces shared helpers to standardize server route/service behavior (responses, error handling, payload sanitization), and refactors multiple dashboard/UI components to reduce duplication and improve consistency across feature tabs, tables, and record cards.
Changes:
- Centralize API error handling and service response shaping via new server utilities/helpers.
- Refactor multiple server services/routes to use stricter payload typing and shared helpers (plus a new import/export data-transfer service).
- Componentize repeated dashboard UI patterns (feature gating pages, tab shells, CRUD menus, skeleton/empty/error states, table utilities).
Reviewed changes
Copilot reviewed 113 out of 113 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/utils/route-handler.ts | Adds shared route error handling helpers for SvelteKit routes. |
| src/server/utils/file-route.ts | Centralizes upload filename sanitization, path resolution, and content-type mapping. |
| src/server/services/vehicleService.ts | Refactors vehicle CRUD responses and customFields (de)serialization. |
| src/server/services/service-response.helper.ts | Adds reusable helpers for success/failure ApiResponse and record existence checks. |
| src/server/services/reminderService.ts | Introduces typed reminder payloads and shared response helpers. |
| src/server/services/pollutionCertificateService.ts | Introduces typed PUCC payloads and shared response helpers. |
| src/server/services/notificationService.ts | Extracts notification formatting/sorting helpers and standardizes responses. |
| src/server/services/notificationProviderService.ts | Extracts config-merge logic and standardizes responses/record lookup. |
| src/server/services/notificationDispatchService.ts | Reuses shared webhook header builder. |
| src/server/services/notification-service.helper.ts | New helper module for notification date logic, messages, and sorting. |
| src/server/services/notification-provider-test.service.ts | New service for testing notification providers (email/webhook/gotify). |
| src/server/services/notification-provider-service.helper.ts | New helper to merge incoming provider configs with existing secrets. |
| src/server/services/notification-provider-http.helper.ts | New helper to build webhook headers (content-type + auth). |
| src/server/services/maintenanceLogService.ts | Introduces typed payloads and shared response helpers. |
| src/server/services/insuranceService.ts | Introduces typed payloads, recurrence sanitization helper, and shared responses. |
| src/server/services/fuelLogService.ts | Introduces typed payloads and shared response helpers. |
| src/server/services/domain-payload.helper.ts | New helper to clear fixed end/expiry dates for recurring payloads. |
| src/server/services/data-transfer.service.ts | New service to implement data export/import with optional encryption. |
| src/server/services/configService.ts | Standardizes config responses and record-not-found handling. |
| src/server/services/autocompleteService.ts | Standardizes autocomplete responses. |
| src/server/services/authService.ts | Standardizes auth responses and record-not-found handling. |
| src/routes/dashboard/reminders/+page.svelte | Replaces inline FeatureGate wrapper with DashboardFeaturePage. |
| src/routes/dashboard/pollution/+page.svelte | Replaces inline FeatureGate wrapper with DashboardFeaturePage. |
| src/routes/dashboard/overview/+page.svelte | Replaces inline FeatureGate wrapper with DashboardFeaturePage. |
| src/routes/dashboard/maintenance/+page.svelte | Replaces inline FeatureGate wrapper with DashboardFeaturePage. |
| src/routes/dashboard/insurance/+page.svelte | Replaces inline FeatureGate wrapper with DashboardFeaturePage. |
| src/routes/dashboard/fuel/+page.svelte | Replaces inline FeatureGate wrapper with DashboardFeaturePage. |
| src/routes/api/vehicles/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/reminders/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/reminders/[reminderId]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/pucc/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/pucc/[puccId]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/notifications/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/notifications/[notificationId]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/maintenance-logs/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/maintenance-logs/[logId]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/insurance/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/insurance/[insuranceId]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/fuel-logs/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/fuel-logs/[logId]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/vehicles/[id]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/test-email-digest/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/notifications/test-enabled-providers/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/notification-providers/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/notification-providers/[id]/test/+server.ts | Uses shared route error handling wrapper + extracts provider test logic. |
| src/routes/api/notification-providers/[id]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/files/+server.ts | Uses shared route error handling + extracts file-path/name logic. |
| src/routes/api/files/[filename]/+server.ts | Uses shared route error handling + shared filename validation/content-type logic. |
| src/routes/api/data/import/+server.ts | Uses new data-transfer service + JSON error wrapper. |
| src/routes/api/data/export/+server.ts | Uses new data-transfer service + JSON error wrapper. |
| src/routes/api/cron/reload/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/config/branding/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/config/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/config/[key]/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/autocomplete/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/auth/register/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/auth/profile/+server.ts | Uses shared route error handling wrapper. |
| src/routes/api/auth/+server.ts | Uses shared route error handling wrapper + minor logging change. |
| src/lib/types/settings.ts | Adds shared settings form option/type definitions. |
| src/lib/stores/sheet.svelte.ts | Tightens sheet store typing for component/data payloads. |
| src/lib/response.ts | Makes ApiResponse generic for better typing. |
| src/lib/helper/table.helper.ts | Adds shared table pagination/header helper utilities. |
| src/lib/helper/table-cell.helper.ts | Adds shared table cell formatting utilities. |
| src/lib/helper/settings-form.helper.ts | Adds reusable settings schema + option builders. |
| src/lib/components/layout/DashboardFeaturePage.svelte | New wrapper for gated dashboard pages with consistent disabled fallback UI. |
| src/lib/components/layout/AppTable.svelte | Refactors table state updates and extracts paging/column label logic to helpers. |
| src/lib/components/feature/vehicle/VehicleQuickActions.svelte | New component for feature-gated vehicle quick action buttons. |
| src/lib/components/feature/vehicle/VehicleList.svelte | Uses shared skeleton/empty/error state components. |
| src/lib/components/feature/vehicle/VehicleCardHeader.svelte | Extracts vehicle card header markup into a dedicated component. |
| src/lib/components/feature/vehicle/VehicleCardDetails.svelte | Extracts vehicle card details row into a dedicated component. |
| src/lib/components/feature/vehicle/VehicleCard.svelte | Simplifies VehicleCard by composing extracted subcomponents. |
| src/lib/components/feature/settings/WebhookProviderForm.svelte | Tightens typing for webhook auth credentials building. |
| src/lib/components/feature/settings/SettingsUnitsTab.svelte | Adds a dedicated units tab component for settings. |
| src/lib/components/feature/settings/SettingsSelectField.svelte | Reuses shared SettingsOption type. |
| src/lib/components/feature/settings/SettingsSection.svelte | Uses Snippet type for slot content typing. |
| src/lib/components/feature/settings/SettingsPersonalizationTab.svelte | Adds a dedicated personalization tab component for settings. |
| src/lib/components/feature/settings/SettingsFeaturesTab.svelte | Adds a dedicated features tab component for settings. |
| src/lib/components/feature/settings/SettingFormSection.svelte | Uses Snippet type for slot content typing. |
| src/lib/components/feature/settings/NotificationProvidersSettings.svelte | Breaks up provider settings UI into smaller reusable components + improves typing. |
| src/lib/components/feature/settings/NotificationProvidersEmptyState.svelte | Extracts “no providers” empty state card. |
| src/lib/components/feature/settings/NotificationProviderDialog.svelte | Extracts provider create/edit dialog into a component. |
| src/lib/components/feature/settings/NotificationProviderChannels.svelte | Extracts channel subscription + enable toggle UI into a component. |
| src/lib/components/feature/settings/NotificationDeliveryPanel.svelte | Extracts scheduled delivery panel UI into a component. |
| src/lib/components/feature/settings/EmailProviderForm.svelte | Removes any and tightens config shaping. |
| src/lib/components/feature/settings/CronInput.svelte | Adds onValueChange callback and tweaks trigger sizing. |
| src/lib/components/feature/reminder/ReminderTab.svelte | Moves to FeatureTabShell abstraction for consistent tab actions. |
| src/lib/components/feature/reminder/ReminderList.svelte | Refactors to shared record card/skeleton/state components. |
| src/lib/components/feature/reminder/ReminderContextMenu.svelte | Refactors to shared CrudActionsMenu abstraction. |
| src/lib/components/feature/pollution/PuccContextMenu.svelte | Refactors to shared CrudActionsMenu abstraction. |
| src/lib/components/feature/pollution/PollutionTab.svelte | Moves to FeatureTabShell abstraction. |
| src/lib/components/feature/pollution/PollutionCertificateList.svelte | Refactors to shared record card/skeleton/state components. |
| src/lib/components/feature/maintenance/MaintenenceLogTab.svelte | Moves to FeatureTabShell abstraction. |
| src/lib/components/feature/maintenance/MaintenanceLogList.svelte | Refactors to shared table skeleton/state components + shared table cell formatters. |
| src/lib/components/feature/maintenance/MaintenanceContextMenu.svelte | Refactors to shared CrudActionsMenu abstraction. |
| src/lib/components/feature/insurance/InsuranceTab.svelte | Moves to FeatureTabShell abstraction. |
| src/lib/components/feature/insurance/InsuranceList.svelte | Refactors to shared record card/skeleton/state components. |
| src/lib/components/feature/insurance/InsuranceContextMenu.svelte | Refactors to shared CrudActionsMenu abstraction. |
| src/lib/components/feature/fuel/FuelLogTab.svelte | Moves to FeatureTabShell abstraction. |
| src/lib/components/feature/fuel/FuelLogList.svelte | Refactors to shared table skeleton/state components + shared table cell formatters. |
| src/lib/components/feature/fuel/FuelLogContextMenu.svelte | Refactors to shared CrudActionsMenu abstraction. |
| src/lib/components/app/TableSkeleton.svelte | New reusable table loading skeleton. |
| src/lib/components/app/ResourceState.svelte | New reusable empty/error/info state presenter. |
| src/lib/components/app/RecordDetailItem.svelte | New reusable labeled icon/value detail row component. |
| src/lib/components/app/input.svelte | Removes stray debug logging. |
| src/lib/components/app/index.ts | Exports new reusable app-level components. |
| src/lib/components/app/FeatureTabShell.svelte | New shared wrapper for feature tabs using TabContainer + sheet actions. |
| src/lib/components/app/FeatureRecordCardSkeleton.svelte | New reusable record card skeleton list. |
| src/lib/components/app/FeatureRecordCard.svelte | New reusable record card layout with header/actions slots. |
| src/lib/components/app/CrudActionsMenu.svelte | New reusable CRUD dropdown + delete confirmation component. |
| src/lib/components/app/CardGridSkeleton.svelte | New reusable skeleton layout for card grids/lists. |
| sample-fuel-import.csv | Removes the sample CSV file from the repo. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+112
to
+116
| export const addVehicle = async (vehicleData: VehicleMutationPayload): Promise<ApiResponse> => { | ||
| const processedData = serializeVehiclePayload(vehicleData); | ||
| const [vehicle] = await db.insert(schema.vehicleTable).values(processedData).returning(); | ||
|
|
||
| const [vehicle] = await db | ||
| .insert(schema.vehicleTable) | ||
| .values({ ...processedData, id: undefined }) | ||
| .returning(); | ||
|
|
||
| return { | ||
| data: { | ||
| ...vehicle, | ||
| customFields: vehicle.customFields ? JSON.parse(vehicle.customFields) : null | ||
| }, | ||
| success: true, | ||
| message: 'Vehicle added successfully.' | ||
| }; | ||
| return createSuccessResponse(parseVehicleRecord(vehicle), 'Vehicle added successfully.'); |
Comment on lines
+197
to
208
| export const updateVehicle = async ( | ||
| id: string, | ||
| vehicleData: VehicleMutationPayload | ||
| ): Promise<ApiResponse> => { | ||
| await getVehicleById(id); // Validates vehicle exists | ||
|
|
||
| // Serialize customFields to JSON string | ||
| const processedData = { | ||
| ...vehicleData, | ||
| customFields: vehicleData.customFields ? JSON.stringify(vehicleData.customFields) : null | ||
| }; | ||
| const processedData = serializeVehiclePayload(vehicleData); | ||
|
|
||
| const [updatedVehicle] = await db | ||
| .update(schema.vehicleTable) | ||
| .set(processedData) | ||
| .where(eq(schema.vehicleTable.id, id)) |
Comment on lines
+24
to
35
| export const addFuelLog = async ( | ||
| vehicleId: string, | ||
| fuelLogData: FuelLogPayload | ||
| ): Promise<ApiResponse> => { | ||
| await validateVehicleExists(vehicleId); | ||
| const fuelLog = await db | ||
| .insert(schema.fuelLogTable) | ||
| .values({ | ||
| ...fuelLogData, | ||
| id: undefined, | ||
| vehicleId: vehicleId | ||
| }) | ||
| .returning(); |
Comment on lines
127
to
146
| @@ -140,11 +144,7 @@ export const updateFuelLog = async ( | |||
| }) | |||
| .where(eq(schema.fuelLogTable.id, id)) | |||
| .returning(); | |||
Comment on lines
+23
to
35
| export const addInsurance = async ( | ||
| vehicleId: string, | ||
| insuranceData: InsurancePayload | ||
| ): Promise<ApiResponse> => { | ||
| await validateVehicleExists(vehicleId); | ||
| // Sanitize: remove endDate on backend when recurrence is not fixed | ||
| if (insuranceData && insuranceData.recurrenceType !== 'none') { | ||
| insuranceData.endDate = null; | ||
| } | ||
| const sanitizedInsuranceData = clearFixedEndDate(insuranceData); | ||
| const insurance = await db | ||
| .insert(schema.insuranceTable) | ||
| .values({ | ||
| ...insuranceData, | ||
| vehicleId: vehicleId, | ||
| id: undefined | ||
| ...sanitizedInsuranceData, | ||
| vehicleId: vehicleId | ||
| }) | ||
| .returning(); |
Comment on lines
49
to
61
| if (typeof body.cost !== 'number' || body.cost <= 0) { | ||
| throw error(400, 'Cost must be a positive number'); | ||
| } | ||
|
|
||
| // Validate against schema | ||
| const validationResult = fuelSchema.safeParse({ ...body, vehicleId: id }); | ||
| if (!validationResult.success) { | ||
| const errors = validationResult.error.flatten(); | ||
| throw error(400, `Validation failed: ${JSON.stringify(errors.fieldErrors)}`); | ||
| } | ||
|
|
||
| const result = await fuelLogService.addFuelLog(id, body); | ||
| return json(result, { status: 201 }); |
Comment on lines
+7
to
+20
| export function clearFixedEndDate<T extends RecurringPayload>(payload: T): T { | ||
| if (payload.recurrenceType === 'none') { | ||
| return payload; | ||
| } | ||
|
|
||
| const clearedPayload = { ...payload }; | ||
|
|
||
| if ('endDate' in clearedPayload) { | ||
| clearedPayload.endDate = null; | ||
| } | ||
|
|
||
| if ('expiryDate' in clearedPayload) { | ||
| clearedPayload.expiryDate = null; | ||
| } |
Comment on lines
42
to
60
| if (body.recurrenceType !== 'none') { | ||
| delete body.expiryDate; | ||
| } else { | ||
| if (!body.expiryDate) { | ||
| throw error(400, 'Expiry date is required when recurrence is fixed date'); | ||
| } | ||
| const expiryDate = new Date(body.expiryDate); | ||
| if (isNaN(expiryDate.getTime())) { | ||
| throw error(400, 'Invalid expiry date format'); | ||
| } | ||
| if (expiryDate <= issueDate) { | ||
| throw error(400, 'Expiry date must be after issue date'); | ||
| } | ||
| } | ||
|
|
||
| // Set default recurrence values if not provided | ||
| body.recurrenceType = body.recurrenceType || 'none'; | ||
| body.recurrenceInterval = body.recurrenceInterval || 1; | ||
|
|
Comment on lines
73
to
76
| const updatedInsurance = await db | ||
| .update(schema.insuranceTable) | ||
| .set({ | ||
| ...(insuranceData && insuranceData.recurrenceType !== 'none' | ||
| ? { ...insuranceData, endDate: null } | ||
| : insuranceData) | ||
| }) | ||
| .set(clearFixedEndDate(insuranceData)) | ||
| .where(eq(schema.insuranceTable.id, id)) |
Comment on lines
75
to
78
| const updatedCertificate = await db | ||
| .update(schema.pollutionCertificateTable) | ||
| .set({ | ||
| ...(pollutionCertificateData && pollutionCertificateData.recurrenceType !== 'none' | ||
| ? { ...pollutionCertificateData, expiryDate: null } | ||
| : pollutionCertificateData) | ||
| }) | ||
| .set(clearFixedEndDate(pollutionCertificateData)) | ||
| .where(eq(schema.pollutionCertificateTable.id, id)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.