feat(ZMS-2936 MPDZBS): new zmscitizenview initial setup webcomponent #900
Conversation
…-2936-initial-setup-webcomponent-cicd
…omponent-cicd ZMS-2936: CI/CD
WalkthroughThis pull request introduces extensive new functionality and configuration for an appointment management system. It adds a CI/CD GitHub Actions workflow and Docker/Nginx deployment settings, along with a full suite of Vue components for booking appointments. New TypeScript interfaces, classes, and global type declarations support API integrations, dynamic module loading, localization, and state management. Additional configuration files for formatting, linting, testing, and build tooling ensure a robust and modular project setup. Changes
Sequence Diagram(s)sequenceDiagram
participant G as GitHub
participant U as Ubuntu Runner
participant R as Repository
participant N as Node Build
participant M as Maven Build
participant D as Docker
participant Reg as Docker Registry
G->>U: Trigger on push event
U->>R: Checkout specific commit
R-->>U: Provide source code
U->>N: Execute Node build (if package.json exists)
U->>M: Execute Maven build (if pom.xml exists)
U->>D: Build Docker image with defined parameters
D->>Reg: Push Docker image using secrets
sequenceDiagram
participant U as User
participant AV as AppointmentView
participant API as ZMSAppointmentAPI
participant BE as Backend
U->>AV: Initiates appointment booking
AV->>API: Request available services and providers
API->>BE: Fetch service & calendar data
BE-->>API: Return service data
API-->>AV: Provide appointment options
U->>AV: Select service and timeslot
AV->>API: Reserve/confirm appointment
API->>BE: Process reservation
BE-->>API: Confirm appointment
API-->>AV: Return confirmation
AV-->>U: Display appointment summary
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (1)
zmscitizenview/src/types/ServiceImpl.ts (1)
1-38: 🛠️ Refactor suggestionImprove type safety and add validation.
The implementation has the following issues:
- The
combinableproperty type could be more specific thanstring[][]- Missing validation for required fields and array contents
Consider adding validation and more specific types:
export class ServiceImpl implements Service { // ... existing properties ... constructor( id: string, name: string, maxQuantity: number, combinable: Array<Array<string>> | undefined, providers: OfficeImpl[] | undefined, subServices: SubService[] | undefined, count: number | undefined ) { if (!id || !name || maxQuantity < 0) { throw new Error('Invalid service parameters'); } this.id = id; this.name = name.trim(); this.maxQuantity = maxQuantity; this.combinable = combinable?.map(arr => [...arr]); // Create defensive copies this.providers = providers?.map(p => ({ ...p })); this.subServices = subServices?.map(s => ({ ...s })); this.count = count; } }
🧹 Nitpick comments (43)
zmscitizenview/src/types/StepperTypes.ts (1)
1-5: LGTM! Consider adding documentation and making icon optional.The interface is well-structured and follows TypeScript naming conventions. Consider these improvements:
+/** + * Represents an item in a stepper component that shows progress through multiple steps. + */ export interface StepperItem { + /** Unique identifier for the step */ id: string; + /** Display text for the step */ label: string; + /** Icon name or path (optional) */ - icon: string; + icon?: string; }zmscitizenview/src/api/models/Provider.ts (1)
1-19: Enhance JSDoc documentation with meaningful descriptions.The interface is well-structured, but the JSDoc comments lack meaningful descriptions of the properties and their purpose. Consider adding descriptive comments to improve code maintainability.
Apply this diff to enhance the documentation:
/** - * + * Represents a provider entity in the appointment system. * @export * @interface Provider */ export interface Provider { /** - * + * Unique identifier for the provider. * @type {string} * @memberof Provider */ id: string; /** - * + * Source system or origin of the provider. * @type {string} * @memberof Provider */ source: string; }zmscitizenview/src/api/models/AvailableDaysDTO.ts (1)
1-19: Enhance JSDoc documentation and consider type refinements.The interface is well-structured, but consider the following improvements:
- Add meaningful descriptions to JSDoc comments.
- Consider using a more specific type for
availableDaysto indicate the expected date format.Apply this diff to enhance the documentation and types:
/** - * + * Data transfer object for available appointment days. * @export * @interface AvailableDaysDTO */ export interface AvailableDaysDTO { /** - * + * List of available days in ISO 8601 date format (YYYY-MM-DD). * @type {Array<string>} * @memberof AvailableDaysDTO */ - availableDays: string[]; + availableDays: `${number}-${number}-${number}`[]; /** - * + * Unix timestamp of the last modification. * @type {number} * @memberof AvailableDaysDTO */ lastModified: number; }zmscitizenview/src/types/AppointmentHashTypes.ts (1)
1-7: Add JSDoc documentation to improve code maintainability.The interface lacks documentation. Consider adding JSDoc comments to describe the purpose of the interface and its properties.
Apply this diff to add documentation:
+/** + * Represents a hash for appointment identification and authentication. + * @interface AppointmentHash + */ export interface AppointmentHash { + /** + * Unique identifier for the appointment. + */ id: string; + /** + * Authentication key for accessing the appointment. + */ authKey: string; + /** + * Optional scope information for the appointment. + */ scope?: Scope; }zmscitizenview/docker/nginx/health.conf (1)
1-7: Consider enhancing the health check configuration.The health check configuration is good but could be improved in the following ways:
- Consider enabling access logging with a reduced format for monitoring and debugging.
- Add rate limiting to prevent DoS attacks.
- Consider adding more comprehensive health status information.
Apply this diff to enhance the configuration:
location /actuator/health { - access_log off; + # Use minimal logging format for monitoring + access_log /var/log/nginx/health.log combined buffer=512k flush=1m; + + # Rate limiting + limit_req zone=health burst=5; + add_header 'Content-Type' 'application/json'; add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; add_header Cache-Control 'no-cache'; - return 200 '{"status":"UP"}'; + return 200 '{"status":"UP","timestamp":$msec}'; }zmscitizenview/src/api/models/Relation.ts (2)
1-12: Enhance JSDoc documentation for theofficeIdproperty.The current JSDoc comments only specify the type. Consider adding descriptions to explain the purpose and any constraints of the property.
/** - * - * @type {string} + * Unique identifier for the office. + * @type {string} - Must be a valid office ID from the system * @memberof Relation */
13-24: Consider adding type constraints for IDs and slots.The properties could benefit from more specific types or validation:
- IDs could use a specific format or pattern
- Slots should be non-negative
- serviceId: string; + serviceId: `service-${string}`; // Template literal type for specific format /** * * @type {number} * @memberof Relation */ - slots: number; + slots: number & { __brand: 'NonNegative' }; // Branded type for validationConsider adding runtime validation in the consuming code:
function validateRelation(relation: Relation): void { if (relation.slots < 0) { throw new Error('Slots must be non-negative'); } if (!relation.officeId.match(/^[a-zA-Z0-9-]+$/)) { throw new Error('Invalid office ID format'); } if (!relation.serviceId.match(/^[a-zA-Z0-9-]+$/)) { throw new Error('Invalid service ID format'); } }zmscitizenview/src/vite-env.d.ts (1)
3-7: Improve type safety in Vue component declaration.The current type declaration uses empty objects and
any, which reduces type safety.declare module "*.vue" { import type { DefineComponent } from "vue"; - const component: DefineComponent<{}, {}, any>; + const component: DefineComponent< + Record<string, unknown>, // Props type + Record<string, unknown>, // RawBindings type + Record<string, unknown> // Data type + >; export default component; }🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 5-5: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
zmscitizenview/src/types/SubService.ts (1)
3-13: Add JSDoc documentation and consider making properties readonly.The class and its properties lack documentation, and mutable properties could lead to unintended state changes.
+/** + * Represents a sub-service within the appointment system. + */ export class SubService { - id: string; + /** Unique identifier for the sub-service */ + readonly id: string; - name: string; + /** Display name of the sub-service */ + readonly name: string; - maxQuantity: number; + /** Maximum number of appointments allowed */ + readonly maxQuantity: number; - providers: OfficeImpl[]; + /** List of offices providing this sub-service */ + readonly providers: readonly OfficeImpl[]; - count: number; + /** Current number of appointments */ + readonly count: number;zmscitizenview/src/api/models/AvailableTimeSlotsDTO.ts (1)
1-19: Enhance type safety and documentation for timestamps.The interface could benefit from more specific types and better documentation.
/** - * + * Data transfer object for available appointment time slots. * @export * @interface AvailableTimeSlotsDTO */ export interface AvailableTimeSlotsDTO { /** - * - * @type {Array<number>} + * Array of Unix timestamps (in milliseconds) representing available appointment slots. + * @type {Array<number>} - Must be valid future timestamps * @memberof AvailableTimeSlotsDTO */ - appointmentTimestamps: number[]; + appointmentTimestamps: Array<number & { __brand: 'UnixTimestamp' }>; /** - * - * @type {number} + * Unix timestamp (in milliseconds) of when this data was last modified. + * @type {number} - Must be a valid timestamp * @memberof AvailableTimeSlotsDTO */ - lastModified: number; + lastModified: number & { __brand: 'UnixTimestamp' }; }Consider adding runtime validation:
function validateTimestamp(timestamp: number): void { const now = Date.now(); if (timestamp < now) { throw new Error('Timestamp must be in the future'); } if (!Number.isInteger(timestamp)) { throw new Error('Timestamp must be an integer'); } }zmscitizenview/src/api/models/ErrorDTO.ts (1)
1-25: Consider enhancing error handling with additional properties and validation.The interface provides basic error handling, but could be improved with:
- Optional
detailsproperty for additional error context- Enum or union type for
errorCodeto ensure consistent error codes- Validation for
lastModifiedtimestamp formatConsider this enhancement:
export interface ErrorDTO { errorCode: string; errorMessage: string; lastModified: number; + details?: Record<string, unknown>; } +// Define allowed error codes +export enum ErrorCode { + NOT_FOUND = 'NOT_FOUND', + INVALID_INPUT = 'INVALID_INPUT', + SERVER_ERROR = 'SERVER_ERROR', + // Add more as needed +}zmscitizenview/public/wrapper.js (1)
1-15: Add lifecycle methods and error handling.The custom element implementation could be improved with:
- Lifecycle methods for cleanup
- Error handling for attribute operations
Consider this enhancement:
customElements.define('zms-appointment', class extends HTMLElement { constructor() { super(); this.attachShadow({ mode: 'open' }); const i18nHost = document.createElement("i18n-host"); const wrapped = document.createElement("zms-appointment-wrapped"); - for (let attr of this.attributes) { - wrapped.setAttribute(attr.name, attr.value); + try { + Array.from(this.attributes).forEach(attr => { + wrapped.setAttribute(attr.name, attr.value); + }); + } catch (error) { + console.error('Failed to copy attributes:', error); } i18nHost.appendChild(wrapped); this.shadowRoot.appendChild(i18nHost); } + + disconnectedCallback() { + // Cleanup when element is removed + this.shadowRoot.textContent = ''; + } } );zmscitizenview/src/api/models/OfficesAndServicesDTO.ts (1)
1-29: Consider using readonly arrays to prevent mutations.The interface could be improved by making the arrays readonly to prevent accidental mutations of the DTO properties.
Consider this enhancement:
export interface OfficesAndServicesDTO { - offices: Office[]; + readonly offices: readonly Office[]; - services: Service[]; + readonly services: readonly Service[]; - relations: Relation[]; + readonly relations: readonly Relation[]; }zmscitizenview/src/api/models/Service.ts (1)
1-31: Add validation constraints and readonly modifiers.The interface could be improved with:
- String validation for id and name
- Number constraints for maxQuantity
- Readonly modifier for combinable array
Consider these enhancements:
+// Add validation types +type NonEmptyString = string & { __brand: 'NonEmptyString' }; +type PositiveNumber = number & { __brand: 'PositiveNumber' }; export interface Service { - id: string; + id: NonEmptyString; - name: string; + name: NonEmptyString; - maxQuantity: number; + maxQuantity: PositiveNumber; - combinable?: string[][]; + readonly combinable?: readonly (readonly string[])[]; } +// Add validation functions +function validateNonEmptyString(value: string): NonEmptyString { + if (!value.trim()) throw new Error('String cannot be empty'); + return value as NonEmptyString; +} +function validatePositiveNumber(value: number): PositiveNumber { + if (value <= 0) throw new Error('Number must be positive'); + return value as PositiveNumber; +}zmscitizenview/src/api/models/AppointmentDTO.ts (1)
4-82: Enhance JSDoc documentation and fix type annotation mismatch.
- The JSDoc comments should include meaningful descriptions for each property to improve code documentation.
- There's a type annotation mismatch in the comment for
subRequestCounts:
- Comment shows
@type {any[]}- Implementation shows
SubRequestCount[]Apply this diff to fix the type annotation and add meaningful descriptions:
/** + * Data Transfer Object for appointment information * * @export * @interface AppointmentDTO */ export interface AppointmentDTO { /** + * Unique identifier for the appointment process * * @type {string} * @memberof AppointmentDTO */ processId: string; /** + * Unix timestamp of the appointment * * @type {number} * @memberof AppointmentDTO */ timestamp: number; /** + * Authentication key for the appointment * * @type {string} * @memberof AppointmentDTO */ authKey: string; /** + * Family name of the person making the appointment * * @type {string} * @memberof AppointmentDTO */ familyName: string; /** + * Email address for appointment confirmation * * @type {string} * @memberof AppointmentDTO */ email: string; /** + * Optional phone number for contact * * @type {string} * @memberof AppointmentDTO */ telephone?: string; /** + * Optional custom text field for additional information * * @type {string} * @memberof AppointmentDTO */ customTextfield?: string; /** + * Identifier of the office where the appointment is scheduled * * @type {string} * @memberof AppointmentDTO */ officeId: string; /** + * Scope of the appointment * * @type {Scope} * @memberof AppointmentDTO */ scope: Scope; /** + * Array of sub-request counts for the appointment * - * @type {any[]} + * @type {SubRequestCount[]} * @memberof AppointmentDTO */ subRequestCounts: SubRequestCount[]; /** + * Identifier of the service being scheduled * * @type {string} * @memberof AppointmentDTO */ serviceId: string; /** + * Number of service instances requested * * @type {number} * @memberof AppointmentDTO */ serviceCount: number; }zmscitizenview/processes/post-build.js (2)
4-25: Fix typo in documentation comment.There's a typo in the documentation:
- "componente" should be "component"
Apply this diff to fix the typo:
/** * Why this? * - * When we build our custom web componente, vite automatically adds a + * When we build our custom web component, vite automatically adds a * "cache busting" mechanic to our minified JS-File.
27-31: Add error handling for file operations.The implementation should handle potential errors when reading the manifest or when the expected file property is missing.
Apply this diff to add error handling:
// read filename from manifest.json +if (!manifest['index.html']) { + throw new Error('Missing index.html entry in manifest.json'); +} const filename = manifest['index.html'].file; // generate loaderJs with the app script's filename -generateLoaderJs(filename); +try { + generateLoaderJs(filename); +} catch (error) { + console.error('Failed to generate loader.js:', error); + process.exit(1); +}zmscitizenview/src/main.ts (2)
1-6: Enhance file documentation.The file comment could be more descriptive about the custom elements being defined.
Apply this diff to improve the documentation:
/** * main.ts * - * Bootstraps Vuetify and other plugins then mounts the App` + * Bootstraps Vue and defines custom elements: + * - i18n-host: Provides internationalization context + * - zms-appointment-wrapped: Handles appointment functionality */
7-16: Add error handling for custom element registration.The implementation should handle potential errors when defining custom elements, especially if they're already defined.
Apply this diff to add error handling:
import { defineCustomElement } from "vue"; import I18nHost from "@/i18n-host.ce.vue"; import ZMSAppointmentElement from "@/zms-appointment.ce.vue"; -const I18nHostElement = defineCustomElement(I18nHost); -customElements.define("i18n-host", I18nHostElement); +try { + const I18nHostElement = defineCustomElement(I18nHost); + if (!customElements.get("i18n-host")) { + customElements.define("i18n-host", I18nHostElement); + } +} catch (error) { + console.error("Failed to register i18n-host:", error); +} -const zmsAppointmentWebcomponent = defineCustomElement(ZMSAppointmentElement); -customElements.define("zms-appointment-wrapped", zmsAppointmentWebcomponent); +try { + const zmsAppointmentWebcomponent = defineCustomElement(ZMSAppointmentElement); + if (!customElements.get("zms-appointment-wrapped")) { + customElements.define("zms-appointment-wrapped", zmsAppointmentWebcomponent); + } +} catch (error) { + console.error("Failed to register zms-appointment-wrapped:", error); +}zmscitizenview/src/i18n-host.ce.vue (1)
5-25: Consider extracting i18n configuration and making locale configurable.
- The i18n configuration could be moved to a separate file for better maintainability.
- The default locale could be made configurable through props.
Create a new file
i18n.config.ts:import deDE from "./utils/de-DE.json"; import enUS from "./utils/en-US.json"; export const createI18nConfig = (locale = "de-DE") => ({ legacy: false, locale, messages: { "de-DE": deDE, "en-US": enUS, }, });Then update the component:
<script setup lang="ts"> -import { provide } from "vue"; +import { provide, defineProps } from "vue"; import { createI18n, I18nInjectionKey } from "vue-i18n"; -import deDE from "./utils/de-DE.json"; -import enUS from "./utils/en-US.json"; +import { createI18nConfig } from "./i18n.config"; -const i18n = createI18n({ - legacy: false, - locale: "de-DE", - messages: { - "de-DE": deDE, - "en-US": enUS, - }, -}); +const props = defineProps<{ + defaultLocale?: string; +}>(); + +const i18n = createI18n(createI18nConfig(props.defaultLocale)); /** * provide i18n instance with `I18nInjectionKey` for other web components */ provide(I18nInjectionKey, i18n); </script>zmscitizenview/eslint.config.js (1)
26-26: Consider enabling TypeScript'sno-explicit-anyruleDisabling
@typescript-eslint/no-explicit-anycould lead to type safety issues. Consider enabling it to maintain better type safety across the codebase.- "@typescript-eslint/no-explicit-any": "off", + "@typescript-eslint/no-explicit-any": "warn",zmscitizenview/processes/lib/loaderjsGenerator.js (1)
17-19: Use consistent path resolutionFor consistency and safety, use
path.resolvefor all file paths, including the template path.- const vitepressLoaderJsTemplate = fs.readFileSync(`${__dirname}/vitepress-loader.js.template`, {encoding: 'utf-8'}); + const vitepressTemplatePath = path.resolve(__dirname, 'vitepress-loader.js.template'); + const vitepressLoaderJsTemplate = fs.readFileSync(vitepressTemplatePath, {encoding: 'utf-8'});zmscitizenview/src/components/Appointment/CustomerInfo.vue (2)
99-100: Extract validation patterns to constants fileEmail and telephone patterns should be moved to a separate constants file for reusability and easier maintenance.
Create a new file
src/constants/validation.ts:export const VALIDATION_PATTERNS = { EMAIL: /^[^\s@]+@[^\s@]+\.[^\s@]+$/, TELEPHONE: /^\+?\d[\d\s]*$/ };
2-7: Enhance accessibility with ARIA attributesThe heading could benefit from additional ARIA attributes for better screen reader support.
<h2 class="m-component-form__title" tabindex="0" + role="heading" + aria-level="2" > {{ t("contactDetails") }} </h2>.github/workflows/maven-node-build.yaml (1)
14-15: Consider expanding matrix strategy for future scalability.The matrix currently includes only one path. Consider adding a comment explaining how to add more paths or moving the paths to a configuration file for better maintainability.
- include: # hier müssen die Pfade angegeben werden + include: # Add application paths below. Each path should contain either package.json or pom.xml - app-path: zmscitizenview + # Example: + # - app-path: another-appzmscitizenview/src/types/ProvideInjectTypes.ts (2)
8-11: Consider adding error handling to updateSelectedService.The method signature doesn't handle potential errors. Consider adding a return type to indicate success/failure.
- updateSelectedService: (newService: ServiceImpl) => void; + updateSelectedService: (newService: ServiceImpl) => Promise<boolean>;
13-16: Add update method for consistency with SelectedServiceProvider.The
SelectedTimeslotProviderinterface should include update methods for consistency withSelectedServiceProvider.export interface SelectedTimeslotProvider { selectedProvider: Ref<OfficeImpl | undefined>; selectedTimeslot: Ref<number>; + updateSelectedProvider: (newProvider: OfficeImpl) => Promise<boolean>; + updateSelectedTimeslot: (newTimeslot: number) => Promise<boolean>; }zmscitizenview/src/api/models/Scope.ts (1)
3-69: Improve JSDoc documentation for better clarity.The JSDoc comments are present but lack meaningful descriptions. Consider adding detailed descriptions for each property.
/** - * - * @export - * @interface Scope + * Represents the scope configuration for an appointment system. + * @interface Scope + * @description Defines the configuration and feature flags for a specific scope + * within the appointment system. */ export interface Scope { /** - * - * @type {string} + * Unique identifier for the scope. + * @type {string} * @memberof Scope */ id: string; // Add similar improvements for other propertieszmscitizenview/src/api/models/Office.ts (1)
69-69: Consider using number type for maxSlotsPerAppointment.The
maxSlotsPerAppointmentis typed as string but represents a numeric value. Consider changing it to number for better type safety and to avoid potential type coercion issues.- maxSlotsPerAppointment?: string; + maxSlotsPerAppointment?: number;zmscitizenview/src/types/OfficeImpl.ts (2)
26-26: Add JSDoc comments for the slots property.The
slotsproperty is missing documentation. Consider adding JSDoc comments to maintain consistency with other properties.+ /** + * Number of available slots for this office + * @type {number} + * @memberof OfficeImpl + */ slots?: number;
5-53: Consider implementing builder pattern for better object creation.The constructor has many parameters which can make object creation error-prone. Consider implementing the builder pattern for a more flexible and maintainable approach.
Here's how you could implement it:
export class OfficeImpl implements Office { private constructor( public readonly id: string, public readonly name: string, public readonly address: Address, public readonly showAlternativeLocations: boolean, public readonly displayNameAlternatives: string[], public readonly organization: string, public readonly organizationUnit?: string, public readonly slotTimeInMinutes: number, public readonly scope?: Scope, public readonly maxSlotsPerAppointment?: string, public readonly slots?: number ) {} static Builder = class { private id!: string; private name!: string; private address!: Address; private showAlternativeLocations!: boolean; private displayNameAlternatives: string[] = []; private organization!: string; private organizationUnit?: string; private slotTimeInMinutes!: number; private scope?: Scope; private maxSlotsPerAppointment?: string; private slots?: number; setId(id: string): this { this.id = id; return this; } // Add similar methods for other properties build(): OfficeImpl { if (!this.id || !this.name || !this.address || !this.organization || !this.slotTimeInMinutes) { throw new Error('Missing required properties'); } return new OfficeImpl( this.id, this.name, this.address, this.showAlternativeLocations, this.displayNameAlternatives, this.organization, this.organizationUnit, this.slotTimeInMinutes, this.scope, this.maxSlotsPerAppointment, this.slots ); } }; }Usage example:
const office = new OfficeImpl.Builder() .setId('123') .setName('Main Office') .setAddress(address) .setShowAlternativeLocations(true) .setOrganization('Org') .setSlotTimeInMinutes(30) .build();zmscitizenview/src/components/Appointment/CalendarView.vue (2)
13-15: Typo in variable name “proverider”.
It appears that “proverider” should be spelled “provider.” This may cause confusion for future maintainers.- v-for="proverider in selectableProviders" + v-for="provider in selectableProviders"
349-366: Consider adding explicit error-handling for API calls.
Although you seterror.value = truewhen unexpected data shape is returned, no.catch()block or error message is shown for network failures or server errors. Handling these errors with a.catch()ortry/catchmight improve reliability and user feedback.zmscitizenview/src/zms-appointment.ce.vue (1)
24-25: Minor naming inconsistency forcustomIconsSprit.
Consider renaming tocustomIconsSpritefor clarity.-import customIconsSprit from "@muenchen/muc-patternlab-vue/assets/icons/custom-icons.svg?raw"; +import customIconsSprite from "@muenchen/muc-patternlab-vue/assets/icons/custom-icons.svg?raw";zmscitizenview/src/components/Appointment/SubserviceListItem.vue (1)
45-49: Rename “checkPlusEndabled” for clarity.
The computed property namecheckPlusEndabledmight be a slight misspelling. ConsidercheckPlusEnabledor a clearer semantic name.-const checkPlusEndabled = computed( +const checkPlusEnabled = computed(zmscitizenview/src/components/Appointment/ServiceFinder.vue (1)
224-224: Use strict equality checks
Using==can lead to unintended type coercions. Prefer strict equality (===) to avoid subtle bugs:- if (relation.serviceId == serviceId) { + if (relation.serviceId === serviceId) {zmscitizenview/src/components/Appointment/AppointmentView.vue (5)
4-13: Consider extracting the condition into a computed property for clarity.The combined condition in
v-if="!confirmAppointmentHash && !appointmentNotFoundError && currentView < 4"is correct but somewhat long. Refactoring it into a computed property can improve readability and maintainability.
50-59: Clarify or remove the stale comment regardingtooManyAppointmentsWithSameMailError.Line 58 includes a note: "Delete
tooManyAppointmentsWithSameMailErrorif contact is transferred from backend call offices-and-services." This suggests a future cleanup or an unfinalized requirement. Consider removing or replacing it with a clear TODO if you still need additional changes before removing the error state.
265-267: Fix the minor spelling issue inrebookOrCanelDialog.This ref name appears to be missing an "n" in "Cancel." Recommended fix:
- const rebookOrCanelDialog = ref<boolean>(false); + const rebookOrCancelDialog = ref<boolean>(false);
300-311: Add checks to guard against invalid step transitions.While these step control methods are succinct, consider adding bounds checks to prevent accidentally decreasing below step 0 or increasing beyond the maximum. This helps ensure no unintended behavior if this component is extended or reused.
495-596: Consider adding unified error handling for all fetch operations inonMounted().The
onMounted()hook fetches multiple endpoints using.then()but lacks.catch()blocks orresponse.okchecks. A single network failure (e.g., server down) might lead to silent rejections. Adding consistent error handling ensures more robust and maintainable code.zmscitizenview/src/api/ZMSAppointmentAPI.ts (2)
28-43: Checkresponse.okand handle non-200 statuses when callingfetchServicesAndProviders.The current implementation calls
.json()without verifyingresponse.ok. If the response is 4xx/5xx, parsing will not fail outright, but you may lose track of errors. Consider adding a simple check:return fetch(apiUrl).then((response) => { + if (!response.ok) { + throw new Error(\`Request failed with status \${response.status}\`); + } return response.json(); });
28-238: Adopt consistent error handling across all fetch functions.All API calls in this file (e.g.,
fetchAvailableDays,reserveAppointment,updateAppointment, etc.) share a pattern of calling.then((response) => response.json())without.catch()or checkingresponse.ok. Consider a centralized error-handling approach withtry/catchor a post-response check to prevent unhandled rejections and ensure uniform error behavior.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
zmscitizenview/package-lock.jsonis excluded by!**/package-lock.jsonzmscitizenview/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (61)
.github/workflows/maven-node-build.yaml(1 hunks)zmscitizenview/.browserslistrc(1 hunks)zmscitizenview/.editorconfig(1 hunks)zmscitizenview/.env.production(1 hunks)zmscitizenview/.gitignore(1 hunks)zmscitizenview/.prettierignore(1 hunks)zmscitizenview/.prettierrc.json(1 hunks)zmscitizenview/Dockerfile(1 hunks)zmscitizenview/docker/nginx/compression.conf(1 hunks)zmscitizenview/docker/nginx/health.conf(1 hunks)zmscitizenview/eslint.config.js(1 hunks)zmscitizenview/index.html(1 hunks)zmscitizenview/package.json(1 hunks)zmscitizenview/processes/lib/loader.js.template(1 hunks)zmscitizenview/processes/lib/loaderjsGenerator.js(1 hunks)zmscitizenview/processes/lib/vitepress-loader.js.template(1 hunks)zmscitizenview/processes/post-build.js(1 hunks)zmscitizenview/public/wrapper.js(1 hunks)zmscitizenview/src/api/ZMSAppointmentAPI.ts(1 hunks)zmscitizenview/src/api/models/Address.ts(1 hunks)zmscitizenview/src/api/models/AppointmentDTO.ts(1 hunks)zmscitizenview/src/api/models/AvailableDaysDTO.ts(1 hunks)zmscitizenview/src/api/models/AvailableTimeSlotsDTO.ts(1 hunks)zmscitizenview/src/api/models/ErrorDTO.ts(1 hunks)zmscitizenview/src/api/models/Office.ts(1 hunks)zmscitizenview/src/api/models/OfficesAndServicesDTO.ts(1 hunks)zmscitizenview/src/api/models/Provider.ts(1 hunks)zmscitizenview/src/api/models/Relation.ts(1 hunks)zmscitizenview/src/api/models/Scope.ts(1 hunks)zmscitizenview/src/api/models/Service.ts(1 hunks)zmscitizenview/src/api/models/SubRequestCount.ts(1 hunks)zmscitizenview/src/components/Appointment/AppointmentSummary.vue(1 hunks)zmscitizenview/src/components/Appointment/AppointmentView.vue(1 hunks)zmscitizenview/src/components/Appointment/CalendarView.vue(1 hunks)zmscitizenview/src/components/Appointment/ClockSvg.vue(1 hunks)zmscitizenview/src/components/Appointment/CustomerInfo.vue(1 hunks)zmscitizenview/src/components/Appointment/ServiceFinder.vue(1 hunks)zmscitizenview/src/components/Appointment/SubserviceListItem.vue(1 hunks)zmscitizenview/src/i18n-host.ce.vue(1 hunks)zmscitizenview/src/main.ts(1 hunks)zmscitizenview/src/shims-tsx.d.ts(1 hunks)zmscitizenview/src/shims-vue.d.ts(1 hunks)zmscitizenview/src/types/AppointmentHashTypes.ts(1 hunks)zmscitizenview/src/types/AppointmentImpl.ts(1 hunks)zmscitizenview/src/types/CustomerData.ts(1 hunks)zmscitizenview/src/types/OfficeImpl.ts(1 hunks)zmscitizenview/src/types/ProvideInjectTypes.ts(1 hunks)zmscitizenview/src/types/ServiceImpl.ts(1 hunks)zmscitizenview/src/types/StepperTypes.ts(1 hunks)zmscitizenview/src/types/SubService.ts(1 hunks)zmscitizenview/src/utils/Constants.ts(1 hunks)zmscitizenview/src/utils/de-DE.json(1 hunks)zmscitizenview/src/utils/en-US.json(1 hunks)zmscitizenview/src/vite-env.d.ts(1 hunks)zmscitizenview/src/zms-appointment.ce.vue(1 hunks)zmscitizenview/tests/unit/example.spec.ts(1 hunks)zmscitizenview/tsconfig.json(1 hunks)zmscitizenview/tsconfig.node.json(1 hunks)zmscitizenview/vite.config.ts(1 hunks)zmscitizenview/vitest.config.ts(1 hunks)zmscitizenview/vue.config.js(1 hunks)
✅ Files skipped from review due to trivial changes (20)
- zmscitizenview/processes/lib/vitepress-loader.js.template
- zmscitizenview/vue.config.js
- zmscitizenview/tsconfig.node.json
- zmscitizenview/src/shims-vue.d.ts
- zmscitizenview/.prettierignore
- zmscitizenview/.browserslistrc
- zmscitizenview/vitest.config.ts
- zmscitizenview/src/api/models/SubRequestCount.ts
- zmscitizenview/src/api/models/Address.ts
- zmscitizenview/src/utils/de-DE.json
- zmscitizenview/.editorconfig
- zmscitizenview/src/components/Appointment/ClockSvg.vue
- zmscitizenview/.prettierrc.json
- zmscitizenview/Dockerfile
- zmscitizenview/package.json
- zmscitizenview/.gitignore
- zmscitizenview/src/shims-tsx.d.ts
- zmscitizenview/tsconfig.json
- zmscitizenview/.env.production
- zmscitizenview/docker/nginx/compression.conf
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,jsx,ts,tsx}`: Flag any usage of console.log() as i...
**/*.{js,jsx,ts,tsx}: Flag any usage of console.log() as it should be replaced with proper logging:
- For development: Use proper debug tools or logging libraries
- For production: Remove console.log() statements or use structured logging
- For errors: Use error tracking services (e.g., Sentry)
- For debugging: Consider using debug libraries that can be enabled/disabled
Example replacement:
// Instead of: console.log('User data:', userData); // Use: logger.debug('Processing user data', { userData }); // or for development only: if (process.env.NODE_ENV === 'development') { debug('User data:', userData); }
zmscitizenview/src/types/StepperTypes.tszmscitizenview/src/api/models/Provider.tszmscitizenview/src/api/models/Relation.tszmscitizenview/src/api/models/AvailableDaysDTO.tszmscitizenview/src/types/AppointmentHashTypes.tszmscitizenview/tests/unit/example.spec.tszmscitizenview/src/types/AppointmentImpl.tszmscitizenview/processes/lib/loaderjsGenerator.jszmscitizenview/src/types/ServiceImpl.tszmscitizenview/src/vite-env.d.tszmscitizenview/src/api/models/AvailableTimeSlotsDTO.tszmscitizenview/eslint.config.jszmscitizenview/src/types/SubService.tszmscitizenview/public/wrapper.jszmscitizenview/processes/post-build.jszmscitizenview/src/api/models/Service.tszmscitizenview/src/main.tszmscitizenview/src/types/CustomerData.tszmscitizenview/src/api/models/AppointmentDTO.tszmscitizenview/vite.config.tszmscitizenview/src/api/models/Office.tszmscitizenview/src/api/models/ErrorDTO.tszmscitizenview/src/api/models/OfficesAndServicesDTO.tszmscitizenview/src/api/models/Scope.tszmscitizenview/src/types/OfficeImpl.tszmscitizenview/src/utils/Constants.tszmscitizenview/src/types/ProvideInjectTypes.tszmscitizenview/src/api/ZMSAppointmentAPI.ts
🪛 Biome (1.9.4)
zmscitizenview/src/vite-env.d.ts
[error] 5-5: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 5-5: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: call-owasp-security-checks / security-scan (zmsclient, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdb, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsslim, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsentities, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdldb, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsticketprinter, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsstatistic, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsmessaging, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmscitizenapi, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmscalldisplay, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsapi, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsadmin, 8.0)
- GitHub Check: call-php-unit-tests / zmsdb-test
- GitHub Check: call-php-unit-tests / zmsapi-test
🔇 Additional comments (19)
zmscitizenview/tests/unit/example.spec.ts (1)
1-2: LGTM! Clean import structure.The import statement is well-organized and includes the necessary testing utilities from Vitest.
zmscitizenview/src/api/models/AppointmentDTO.ts (1)
1-2: LGTM!The imports are correctly defined and use proper relative paths.
zmscitizenview/processes/post-build.js (1)
1-2: LGTM!The imports are correctly defined and use the new
withsyntax for JSON imports.zmscitizenview/src/i18n-host.ce.vue (1)
1-3: LGTM!The template is correctly defined with a slot for child components.
zmscitizenview/vite.config.ts (1)
17-17:❓ Verification inconclusive
Review environment variables handling
Setting
process.envto an empty object might prevent access to important environment variables. Consider defining specific environment variables needed by the application.
🏁 Script executed:
#!/bin/bash # Check for .env files and their variables fd -t f "^\.env" -x cat {} \; | grep -v '^#' | grep -v '^$'Length of output: 272
Attention: Environment variables handling in vite.config.ts
The current configuration:
define: {'process.env': {}},sets
process.envto an empty object. This approach might disable access to environment variables that your application actually needs. Since our initial search for.envfiles didn’t yield usable output (due to hidden file filtering), please verify whether your project relies on any environment variables that should be explicitly defined.To ensure a thorough check, please run the script below—which includes the hidden flag—to search for any
.envfiles in your repository:#!/bin/bash # Search for hidden .env files and display non-comment, non-empty lines fd -H -t f '^\.env' -x cat {} \; | grep -v '^#' | grep -v '^$'If any environment variables are found and are required by your application, update the configuration to define only the necessary variables rather than replacing
process.envwith an empty object..github/workflows/maven-node-build.yaml (1)
17-17: Good security practice using pinned action version!Using a specific commit hash for the checkout action is a secure approach as it prevents supply chain attacks.
zmscitizenview/src/utils/en-US.json (1)
22-22:❓ Verification inconclusive
Consider sanitizing HTML in translation strings.
The translation string contains raw HTML (
<a>tags). Ensure that your i18n setup properly sanitizes HTML to prevent XSS attacks when rendering these translations.
🏁 Script executed:
#!/bin/bash # Check if vue-i18n is configured with HTML sanitization rg -A 5 "createI18n|i18n.*setup" src/Length of output: 84
HTML Sanitization Verification Needed for Translation String
The translation string in
zmscitizenview/src/utils/en-US.jsoncontains raw HTML that may introduce XSS risks. The initial verification script was run against the wrong folder (src/), which resulted in an error. Please confirm that your Vue-i18n configuration (likely withinzmscitizenview/) is set up to sanitize HTML content before rendering. Specifically, ensure that:
- The i18n setup is correctly configured (e.g., in a file where
createI18nor similar setup functions are used) to sanitize any HTML included in translation strings.- Any render functions or directives (e.g., usage of
v-html) that process these translation strings integrate proper sanitization measures.After verifying these points, if the configuration does not enforce sanitization, please update it to apply HTML sanitizing to translation strings.
zmscitizenview/index.html (1)
11-17:❓ Verification inconclusive
Verify the fixed version of Muenchen.de assets.
The stylesheets are linked to a specific version (1.0.7) of Muenchen.de assets. Ensure this is the latest stable version and check for any security advisories.
🌐 Web query:
What is the latest version of Muenchen.de assets (muenchende-style.css)?💡 Result:
Based on the provided search results, there is no direct mention of "muenchende-style.css" or its versioning in any of the sources related to Munich's official websites or assets. The search results primarily discuss:
- WordPress theme CSS versioning issues[1][5][9]
- Munich tourist attractions and city infrastructure[2][6][8][10]
- CSS framework updates (Simple.css v2)[3]
- Frontend development challenges[4][7]
For official Munich websites like muenchen.de, while there are references to design elements and infrastructure updates[2][6][8], none of the results contain specific information about their CSS file versions or asset management systems.
To obtain the latest version of muenchende-style.css, you would need to:
- Inspect the website directly using browser developer tools
- Check HTTP response headers for cache/version info
- Contact Munich's webmaster through their official channels[6][10]
The most recent CSS-related update mentioned in the results is for Simple.css v2 released in January 2022[3], but this appears unrelated to Munich's official assets.
Citations:
- 1: https://theme.co/forum/t/controlling-the-versioning-on-my-style-css-within-my-child-theme/60256
- 2: https://www.muenchen.de/en/sights/attractions/new-town-hall
- 3: https://simplecss.org/simple-css-version-2
- 4: https://forum.freecodecamp.org/t/project3-problems-with-flex-within-style-order/463889
- 5: https://generatepress.com/forums/topic/is-there-a-possibility-to-undo-all-of-the-changes-done-to-my-style-css/
- 6: https://www.muenchen.de/en/home
- 7: https://johnjago.com/hugo-css-mystery/
- 8: https://www.muenchen.de/en/sights/attractions/viktualienmarkt-top-sight-munich
- 9: https://community.cloudflare.com/t/styles-css-showing-previous-versions-on-live-end/392023
- 10: https://www.residenz-muenchen.de/englisch/tourist/index.htm
Action Required: Confirm Muenchen.de Asset Version & Security
The current asset links in
zmscitizenview/index.htmlspecify version 1.0.7 for bothmuenchende-fontfaces.cssandmuenchende-style.css. Our web query did not reveal any updated version information or documented security advisories for these assets. Please:
- Manually verify (via browser dev tools or direct inquiry) that 1.0.7 is indeed the latest stable version.
- Check for any security advisories related to these assets.
- Update the asset links if a newer, secure version is available.
zmscitizenview/src/components/Appointment/CalendarView.vue (1)
468-519: Prevent potential index out-of-bounds access when no offices are found.
You callshowSelectionForProvider(offices[0])without checking ifofficesis empty. This could cause a runtime error. Consider a fallback scenario (e.g., whenoffices.length === 0).+ if (offices.length === 0) { + error.value = true; + return; + }zmscitizenview/src/zms-appointment.ce.vue (1)
1-21: Overall component structure looks good.
No significant logical or security concerns identified. Prop usage and template structure are clear and well-organized.Also applies to: 23-64
zmscitizenview/src/components/Appointment/SubserviceListItem.vue (1)
1-13: Implementation is concise and correct.
Themuc-counterintegration andv-modelusage are handled well. Emitting the "change" event upon updates is a good pattern for parent component communication.zmscitizenview/src/components/Appointment/AppointmentSummary.vue (3)
13-14: Potential null reference forselectedService
IfselectedServiceis not guaranteed to be defined, referencingselectedService.countdirectly here may cause runtime errors or unexpected behavior. Consider wrapping this paragraph with av-if="selectedService"or use optional chaining (e.g.selectedService?.count) to avoid null references.
96-96: EnsureselectedProvider.scopeis present
selectedProvider.scope.customTextfieldLabelis referenced without checking ifselectedProvider.scopeexists in this block. IfselectedProvideror itsscopeproperty is undefined, this may throw an error. Consider adding additional safe checks or an appropriatev-ifguard.
218-342: Overall structure is clear and maintainable
The component effectively manages the appointment summary, including checkboxes for privacy policy and communication preferences. The computed properties and methods are well-organized, and usage of the Composition API is consistent. No major concerns were found beyond the noted null-reference checks.zmscitizenview/src/components/Appointment/ServiceFinder.vue (1)
103-350: Implementation looks solid
The service selection and slot management logic is well structured. Computed properties and watchers correctly update the state based on user interactions. No significant performance or security issues detected.zmscitizenview/src/utils/Constants.ts (2)
23-25: Provide fallback forgetServiceBaseURL()
IfVITE_VUE_APP_SERVICE_BASE_URLis missing, this function may returnundefinedand cause future API calls to fail. Consider returning a default value or throwing an error if it's not set.
40-49: Handle absent environment variable ingetAPIBaseURL()
If bothbaseUrlandVITE_VUE_APP_API_URLare not provided, falling back tonew URL(import.meta.url).originmay be unintended. Validate this fallback or provide an explicit default.zmscitizenview/src/components/Appointment/AppointmentView.vue (1)
478-493: Confirm thatfoundOfficealways exists.In
getProviders(), code assumesfoundOfficeis present. If.filter()returns an empty array, accessing[0]may result inundefined. Consider adding a check or handle the case where no matching office is found.zmscitizenview/src/api/ZMSAppointmentAPI.ts (1)
1-19: All imports and constants appear consistent.
No issues are apparent with the initial setup of imports and constant variables for your APIs and references.
ThomasAFink
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (9)
zmscitizenview/src/components/Appointment/AppointmentView.vue (9)
442-455: 🛠️ Refactor suggestionAdd error handling for the cancelAppointment promise.
This method needs a
.catch()handler to manage API call failures or network errors, ensuring proper error state management.- cancelAppointment(appointment.value, props.baseUrl ?? undefined).then( + cancelAppointment(appointment.value, props.baseUrl ?? undefined) + .then( (data) => { if ((data as AppointmentDTO).processId != undefined) { cancelAppointmentSuccess.value = true; } else { cancelAppointmentError.value = true; } increaseCurrentView(); } - ); + ) + .catch((error) => { + console.error("Error canceling appointment:", error); + cancelAppointmentError.value = true; + increaseCurrentView(); + });
519-528: 🛠️ Refactor suggestionAdd error handling for the confirmAppointment promise.
The call to
confirmAppointmentneeds a.catch()block to handle potential API failures or network errors, ensuring proper error state management.- confirmAppointment(appointmentData, props.baseUrl ?? undefined).then( + confirmAppointment(appointmentData, props.baseUrl ?? undefined) + .then( (data) => { if ((data as AppointmentDTO).processId != undefined) { confirmAppointmentSuccess.value = true; } else { confirmAppointmentError.value = true; } } - ); + ) + .catch((error) => { + console.error("Error confirming appointment:", error); + confirmAppointmentError.value = true; + });
339-363: 🛠️ Refactor suggestionHandle network or runtime errors in the setRebookData promise.
The
setRebookDatamethod lacks a.catch()handler for theupdateAppointmentAPI call, which could lead to unhandled promise rejections if network errors occur.- updateAppointment(appointment.value, props.baseUrl ?? undefined).then( + updateAppointment(appointment.value, props.baseUrl ?? undefined) + .then( (data) => { if ((data as AppointmentDTO).processId != undefined) { appointment.value = data as AppointmentDTO; } else { if ( (data as ErrorDTO).errorCode === "tooManyAppointmentsWithSameMail" ) { tooManyAppointmentsWithSameMailError.value = true; } else { updateAppointmentError.value = true; } } currentView.value = 3; } - ); + ) + .catch((error) => { + console.error("Error updating appointment during rebooking:", error); + updateAppointmentError.value = true; + currentView.value = 3; + });
430-434: 🛠️ Refactor suggestionAdd error handling for cancelAppointment in nextBookAppointment.
The call to
cancelAppointmentinside thenextBookAppointmentmethod doesn't handle potential errors, which could lead to silent failures.- cancelAppointment( - rebookedAppointment.value, - props.baseUrl ?? undefined - ); + cancelAppointment( + rebookedAppointment.value, + props.baseUrl ?? undefined + ).catch(error => { + console.error("Error canceling appointment during rebooking:", error); + // Consider whether to show an error message to the user + });
423-440: 🛠️ Refactor suggestionAdd error handling for promise rejections.
The
nextBookAppointmentmethod lacks a.catch()block to handle potential API failures or network errors, which could lead to unhandled promise rejections.- preconfirmAppointment(appointment.value, props.baseUrl ?? undefined).then( + preconfirmAppointment(appointment.value, props.baseUrl ?? undefined) + .then( (data) => { if ((data as AppointmentDTO).processId != undefined) { appointment.value = data as AppointmentDTO; if (isRebooking.value && rebookedAppointment.value) { cancelAppointment( rebookedAppointment.value, props.baseUrl ?? undefined ); } increaseCurrentView(); } } - ); + ) + .catch((error) => { + console.error("Error preconfirming appointment:", error); + confirmAppointmentError.value = true; + increaseCurrentView(); + });
376-377: 🛠️ Refactor suggestionAdd error handling for nested cancelAppointment call.
The call to
cancelAppointmentinside thenextReserveAppointmentmethod doesn't handle potential errors, which could lead to silent failures.- cancelAppointment(appointment.value, props.baseUrl ?? undefined); + cancelAppointment(appointment.value, props.baseUrl ?? undefined) + .catch(error => { + console.error("Error canceling appointment during reservation:", error); + // Consider whether to show an error message to the user + });
392-421: 🛠️ Refactor suggestionAdd error handling for API request failures.
The
nextUpdateAppointmentmethod lacks proper error handling for network failures or unexpected errors. If the API call fails, there's no.catch()handler to manage the rejection.- updateAppointment(appointment.value, props.baseUrl ?? undefined).then( + updateAppointment(appointment.value, props.baseUrl ?? undefined) + .then( (data) => { if ((data as AppointmentDTO).processId != undefined) { appointment.value = data as AppointmentDTO; } else { if ( (data as ErrorDTO).errorCode === "tooManyAppointmentsWithSameMail" ) { tooManyAppointmentsWithSameMailError.value = true; } else { updateAppointmentError.value = true; } } increaseCurrentView(); } - ); + ) + .catch((error) => { + console.error("Error updating appointment:", error); + updateAppointmentError.value = true; + increaseCurrentView(); + });
555-600: 🛠️ Refactor suggestionAdd error handling for the fetchAppointment promise.
The call to
fetchAppointmentneeds a.catch()block to handle potential API failures or network errors.- fetchAppointment(appointmentData, props.baseUrl ?? undefined).then( + fetchAppointment(appointmentData, props.baseUrl ?? undefined) + .then( (data) => { // ... existing code ... } - ); + ) + .catch((error) => { + console.error("Error fetching appointment:", error); + appointmentNotFoundError.value = true; + });
532-602: 🛠️ Refactor suggestionAdd error handling for the fetchServicesAndProviders promise.
The call to
fetchServicesAndProvidersneeds a.catch()block to handle potential API failures or network errors.- fetchServicesAndProviders( - props.serviceId ?? undefined, - props.locationId ?? undefined, - props.baseUrl ?? undefined - ).then((data) => { + fetchServicesAndProviders( + props.serviceId ?? undefined, + props.locationId ?? undefined, + props.baseUrl ?? undefined + ) + .then((data) => { services.value = data.services; relations.value = data.relations; offices.value = data.offices; // ... rest of the code ... - }); + }) + .catch((error) => { + console.error("Error fetching services and providers:", error); + appointmentNotFoundError.value = true; + });
🧹 Nitpick comments (5)
zmscitizenview/src/components/Appointment/AppointmentView.vue (5)
485-501: Optimize the getProviders method for better performance.The current implementation iterates through arrays multiple times with nested loops, which could be inefficient for large datasets. Consider using map/filter with a more efficient lookup approach.
const getProviders = (serviceId: string, providers: string[] | null) => { - const officesAtService = new Array<OfficeImpl>(); - relations.value.forEach((relation) => { - if (relation.serviceId == serviceId) { - const foundOffice: OfficeImpl = offices.value.filter((office) => { - return office.id == relation.officeId; - })[0]; - - if (!providers || providers.includes(foundOffice.id.toString())) { - foundOffice.slots = relation.slots; - officesAtService.push(foundOffice); - } - } - }); + // Create a map for faster office lookup + const officeMap = new Map(offices.value.map(office => [office.id, {...office}])); + + // Filter relations and map to offices in one pass + const officesAtService = relations.value + .filter(relation => relation.serviceId === serviceId) + .map(relation => { + const foundOffice = officeMap.get(relation.officeId); + if (foundOffice && (!providers || providers.includes(foundOffice.id.toString()))) { + foundOffice.slots = relation.slots; + return foundOffice; + } + return null; + }) + .filter(Boolean) as OfficeImpl[]; return officesAtService; };
17-17: Replace magic numbers with named constants for better readability.The code uses numeric literals (0, 1, 2, 3, 4) for view states, which reduces readability and makes maintenance more difficult. Consider defining named constants for these values.
+// Add these constants at the top of the script section +const VIEW_SERVICE_SELECTION = 0; +const VIEW_CALENDAR = 1; +const VIEW_CUSTOMER_INFO = 2; +const VIEW_APPOINTMENT_SUMMARY = 3; +const VIEW_CONFIRMATION = 4; // Then use these constants throughout the code, for example: - <div v-if="currentView === 0 && !appointmentHash"> + <div v-if="currentView === VIEW_SERVICE_SELECTION && !appointmentHash">Also applies to: 27-27, 50-50, 57-57, 102-102
397-402: Simplify null checks with nullish coalescing operator.The code uses ternary operators for null/undefined checks, which could be simplified using the nullish coalescing operator (??) for better readability.
- appointment.value.telephone = customerData.value.telephoneNumber - ? customerData.value.telephoneNumber - : undefined; - appointment.value.customTextfield = customerData.value.customTextfield - ? customerData.value.customTextfield - : undefined; + appointment.value.telephone = customerData.value.telephoneNumber ?? undefined; + appointment.value.customTextfield = customerData.value.customTextfield ?? undefined;
217-217: Improve type safety for translation function.The
tprop is typed asany, which loses type safety. Consider using a more specific type for the translation function to improve type checking and documentation.- t: any; + t: (key: string) => string;
479-483: Consider user preferences for scroll behavior.The
goToTopfunction usesbehavior: "instant"which might create a jarring experience for users. Consider using smooth scrolling for a better user experience, or make it configurable based on user preferences.- window.scrollTo({ top: 0, behavior: "instant" }); + window.scrollTo({ top: 0, behavior: "smooth" });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
zmscitizenview/src/components/Appointment/AppointmentView.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-php-code-quality / module-code-quality (zmsstatistic, 8.0)
- GitHub Check: call-php-code-quality / module-code-quality (zmsslim, 8.0)
🔇 Additional comments (1)
zmscitizenview/src/components/Appointment/AppointmentView.vue (1)
365-390: Handle network or runtime errors in promise chains.This function relies on
.then()without a dedicated.catch(). If the network request fails or the server is unreachable, the returned promise might reject silently, causing unhandled promise errors. Consider adoptingtry/catchwithasync/awaitor adding.catch()to handle request failures consistently.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (5)
zmscitizenview/src/components/Appointment/ServiceFinder.vue (1)
315-348:⚠️ Potential issueAdd error handling to the API call.
The
fetchServicesAndProvidersAPI call doesn't have error handling, which could lead to unhandled promise rejections if the network request fails.fetchServicesAndProviders( props.preselectedServiceId ?? undefined, props.preselectedOfficeId ?? undefined, props.baseUrl ?? undefined - ).then((data) => { - services.value = data.services; - relations.value = data.relations; - offices.value = data.offices; - - if (props.preselectedServiceId) { - service.value = services.value.find( - (service) => service.id == props.preselectedServiceId - ); - } - }); + ).then((data) => { + services.value = data.services; + relations.value = data.relations; + offices.value = data.offices; + + if (props.preselectedServiceId) { + service.value = services.value.find( + (service) => service.id == props.preselectedServiceId + ); + } + }).catch((error) => { + console.error("Failed to fetch services and providers:", error); + // Consider adding user-friendly error handling/display + });zmscitizenview/src/components/Appointment/AppointmentView.vue (4)
365-390:⚠️ Potential issueAdd error handling to API calls.
The
nextReserveAppointmentfunction makes API calls without proper error handling, which could lead to unhandled promise rejections if the network request fails.const nextReserveAppointment = () => { rebookOrCanelDialog.value = false; reserveAppointment( selectedTimeslot.value, Array.from(selectedServiceMap.value.keys()), Array.from(selectedServiceMap.value.values()), selectedProvider.value.id, props.baseUrl ?? undefined - ).then((data) => { - if ((data as AppointmentDTO).processId != undefined) { - if (appointment.value && !isRebooking.value) { - cancelAppointment(appointment.value, props.baseUrl ?? undefined); - } - appointment.value = data as AppointmentDTO; - if (isRebooking.value) { - setRebookData(); - } else { - increaseCurrentView(); - } - } else { - if ((data as ErrorDTO).errorCode === "appointmentNotAvailable") { - appointmentNotAvailableError.value = true; - } - } - }); + ).then((data) => { + if ((data as AppointmentDTO).processId != undefined) { + if (appointment.value && !isRebooking.value) { + cancelAppointment(appointment.value, props.baseUrl ?? undefined).catch(error => { + console.error("Failed to cancel appointment:", error); + }); + } + appointment.value = data as AppointmentDTO; + if (isRebooking.value) { + setRebookData(); + } else { + increaseCurrentView(); + } + } else { + if ((data as ErrorDTO).errorCode === "appointmentNotAvailable") { + appointmentNotAvailableError.value = true; + } + } + }).catch(error => { + console.error("Failed to reserve appointment:", error); + // Consider adding user-friendly error handling/display + }); };
518-535:⚠️ Potential issueAdd error handling for confirmAppointment API call.
The
confirmAppointmentAPI call doesn't have proper error handling, which could lead to unhandled errors if the request fails.if (props.confirmAppointmentHash) { const appointmentData = parseAppointmentHash(props.confirmAppointmentHash); if (!appointmentData) { confirmAppointmentError.value = true; return; } confirmAppointment(appointmentData, props.baseUrl ?? undefined).then( (data) => { if ((data as AppointmentDTO).processId != undefined) { confirmAppointmentSuccess.value = true; } else { confirmAppointmentError.value = true; } } - ); + ).catch(error => { + console.error("Failed to confirm appointment:", error); + confirmAppointmentError.value = true; + }); }
392-421:⚠️ Potential issueAdd error handling to nextUpdateAppointment method.
The updateAppointment call in nextUpdateAppointment lacks error handling, which could lead to unhandled promise rejections.
const nextUpdateAppointment = () => { if (appointment.value) { appointment.value.familyName = customerData.value.firstName + " " + customerData.value.lastName; appointment.value.email = customerData.value.mailAddress; appointment.value.telephone = customerData.value.telephoneNumber ? customerData.value.telephoneNumber : undefined; appointment.value.customTextfield = customerData.value.customTextfield ? customerData.value.customTextfield : undefined; updateAppointment(appointment.value, props.baseUrl ?? undefined).then( (data) => { if ((data as AppointmentDTO).processId != undefined) { appointment.value = data as AppointmentDTO; } else { if ( (data as ErrorDTO).errorCode === "tooManyAppointmentsWithSameMail" ) { tooManyAppointmentsWithSameMailError.value = true; } else { updateAppointmentError.value = true; } } increaseCurrentView(); } - ); + ).catch(error => { + console.error("Failed to update appointment:", error); + updateAppointmentError.value = true; + increaseCurrentView(); + }); } };
339-363:⚠️ Potential issueAdd error handling to setRebookData method.
Similar to other API calls, the updateAppointment call in setRebookData lacks error handling.
const setRebookData = () => { if (appointment.value && rebookedAppointment.value) { appointment.value.familyName = rebookedAppointment.value.familyName; appointment.value.email = rebookedAppointment.value.email; appointment.value.telephone = rebookedAppointment.value.telephone; appointment.value.customTextfield = rebookedAppointment.value.customTextfield; updateAppointment(appointment.value, props.baseUrl ?? undefined).then( (data) => { if ((data as AppointmentDTO).processId != undefined) { appointment.value = data as AppointmentDTO; } else { if ( (data as ErrorDTO).errorCode === "tooManyAppointmentsWithSameMail" ) { tooManyAppointmentsWithSameMailError.value = true; } else { updateAppointmentError.value = true; } } currentView.value = 3; } - ); + ).catch(error => { + console.error("Failed to update appointment:", error); + updateAppointmentError.value = true; + currentView.value = 3; + }); } };
🧹 Nitpick comments (4)
zmscitizenview/src/components/Appointment/AppointmentSummary.vue (4)
275-278: Improve type safety by replacinganywith a proper type.The
formatTimefunction usesanyfor the time parameter, which reduces type safety. Consider using a more specific type likenumbersince it appears to be a Unix timestamp.-const formatTime = (time: any) => { +const formatTime = (time: number) => { const date = new Date(time * 1000); return formatterDate.format(date) + ", " + formatterTime.format(date); };
299-330: Refactor the complexestimatedDurationfunction for better maintainability.The
estimatedDurationfunction contains nested conditionals and multiple calculations. Refactoring it into smaller, focused functions would improve readability and maintainability.+/** + * Calculate time required for a single service based on provider slots + */ +const calculateServiceTime = (service, serviceProvider) => { + if (serviceProvider && serviceProvider.slots && service && service.count) { + return service.count * serviceProvider.slots * serviceProvider.slotTimeInMinutes; + } + return 0; +}; + +/** + * Calculate time required for a subservice based on provider slots + */ +const calculateSubserviceTime = (subservice, subserviceProvider) => { + if (subserviceProvider && subservice.count && subserviceProvider.slots) { + return subservice.count * subserviceProvider.slots * subserviceProvider.slotTimeInMinutes; + } + return 0; +}; /** * This function determines the expected duration of the appointment. * The provider is queried for the service and each subservice because the slots for the respective service are stored in this provider. */ const estimatedDuration = () => { let time = 0; const serviceProvider = selectedService.value?.providers?.find( (provider) => provider.id == selectedProvider.value?.id ); - if ( - serviceProvider && - serviceProvider.slots && - selectedService.value && - selectedService.value.count - ) { - time = - selectedService.value.count * - serviceProvider.slots * - serviceProvider.slotTimeInMinutes; - } + time += calculateServiceTime(selectedService.value, serviceProvider); if (selectedService.value?.subServices) { selectedService.value.subServices.forEach((subservice) => { const subserviceProvider = subservice.providers?.find( (provider) => provider.id == selectedProvider.value?.id ); - if (subserviceProvider && subservice.count && subserviceProvider.slots) { - time += - subservice.count * - subserviceProvider.slots * - subserviceProvider.slotTimeInMinutes; - } + time += calculateSubserviceTime(subservice, subserviceProvider); }); } return time; };
285-287: Simplify computed property with direct return.This computed property can be simplified for better readability.
-const validForm = computed( - () => privacyPolicy.value && electronicCommunication.value -); +const validForm = computed(() => privacyPolicy.value && electronicCommunication.value);
92-100: Improve accessibility of custom textfield section.The
tabindex="0"on the div containing both a strong element and a paragraph makes for confusing accessibility structure. It's better to structure this differently.- <div - v-if="appointment.customTextfield" - tabindex="0" - > - <strong>{{ selectedProvider.scope.customTextfieldLabel }}</strong - ><br /> - <p>{{ appointment.customTextfield }}</p> - <br /> - </div> + <div v-if="appointment.customTextfield"> + <p tabindex="0"> + <strong>{{ selectedProvider.scope.customTextfieldLabel }}</strong><br /> + {{ appointment.customTextfield }} + </p> + <br /> + </div>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
zmscitizenview/src/components/Appointment/AppointmentSummary.vue(1 hunks)zmscitizenview/src/components/Appointment/AppointmentView.vue(1 hunks)zmscitizenview/src/components/Appointment/CalendarView.vue(1 hunks)zmscitizenview/src/components/Appointment/CustomerInfo.vue(1 hunks)zmscitizenview/src/components/Appointment/ServiceFinder.vue(1 hunks)zmscitizenview/src/utils/en-US.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- zmscitizenview/src/components/Appointment/CustomerInfo.vue
- zmscitizenview/src/utils/en-US.json
- zmscitizenview/src/components/Appointment/CalendarView.vue
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: call-php-unit-tests / module-test (zmsentities, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsdldb, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmscitizenapi, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsclient, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmscalldisplay, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdb, 8.0)
- GitHub Check: call-php-unit-tests / module-test (zmsadmin, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsslim, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsentities, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsdldb, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsticketprinter, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsstatistic, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsmessaging, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmscitizenapi, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmscalldisplay, 8.0)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: call-owasp-security-checks / security-scan (zmsapi, 8.0)
- GitHub Check: call-owasp-security-checks / security-scan (zmsadmin, 8.0)
| const { selectedService } = inject<SelectedServiceProvider>( | ||
| "selectedServiceProvider" | ||
| ) as SelectedServiceProvider; | ||
|
|
||
| const { selectedProvider } = inject<SelectedTimeslotProvider>( | ||
| "selectedTimeslot" | ||
| ) as SelectedTimeslotProvider; | ||
|
|
||
| const { appointment } = inject<SelectedAppointmentProvider>( | ||
| "appointment" | ||
| ) as SelectedAppointmentProvider; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null checks for injected dependencies.
The component injects dependencies without checking if they're properly initialized. This could lead to runtime errors if any of these injections fail.
-const { selectedService } = inject<SelectedServiceProvider>(
- "selectedServiceProvider"
-) as SelectedServiceProvider;
+const selectedServiceProvider = inject<SelectedServiceProvider>("selectedServiceProvider");
+const { selectedService } = selectedServiceProvider || { selectedService: ref() };
-const { selectedProvider } = inject<SelectedTimeslotProvider>(
- "selectedTimeslot"
-) as SelectedTimeslotProvider;
+const selectedTimeslotProvider = inject<SelectedTimeslotProvider>("selectedTimeslot");
+const { selectedProvider } = selectedTimeslotProvider || { selectedProvider: ref() };
-const { appointment } = inject<SelectedAppointmentProvider>(
- "appointment"
-) as SelectedAppointmentProvider;
+const appointmentProvider = inject<SelectedAppointmentProvider>("appointment");
+const { appointment } = appointmentProvider || { appointment: ref() };📝 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 { selectedService } = inject<SelectedServiceProvider>( | |
| "selectedServiceProvider" | |
| ) as SelectedServiceProvider; | |
| const { selectedProvider } = inject<SelectedTimeslotProvider>( | |
| "selectedTimeslot" | |
| ) as SelectedTimeslotProvider; | |
| const { appointment } = inject<SelectedAppointmentProvider>( | |
| "appointment" | |
| ) as SelectedAppointmentProvider; | |
| const selectedServiceProvider = inject<SelectedServiceProvider>("selectedServiceProvider"); | |
| const { selectedService } = selectedServiceProvider || { selectedService: ref() }; | |
| const selectedTimeslotProvider = inject<SelectedTimeslotProvider>("selectedTimeslot"); | |
| const { selectedProvider } = selectedTimeslotProvider || { selectedProvider: ref() }; | |
| const appointmentProvider = inject<SelectedAppointmentProvider>("appointment"); | |
| const { appointment } = appointmentProvider || { appointment: ref() }; |
| const estimatedDuration = computed(() => { | ||
| return service.value.providers | ||
| ? service.value.providers[0].slotTimeInMinutes * currentSlots.value | ||
| : 0; | ||
| }); |
There was a problem hiding this comment.
Guard against undefined providers in the estimatedDuration calculation.
The estimatedDuration computed property assumes providers[0] exists, which could cause errors if the providers array is empty.
const estimatedDuration = computed(() => {
- return service.value.providers
- ? service.value.providers[0].slotTimeInMinutes * currentSlots.value
- : 0;
+ if (!service.value?.providers || service.value.providers.length === 0) {
+ return 0;
+ }
+ return service.value.providers[0].slotTimeInMinutes * currentSlots.value;
});📝 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 estimatedDuration = computed(() => { | |
| return service.value.providers | |
| ? service.value.providers[0].slotTimeInMinutes * currentSlots.value | |
| : 0; | |
| }); | |
| const estimatedDuration = computed(() => { | |
| if (!service.value?.providers || service.value.providers.length === 0) { | |
| return 0; | |
| } | |
| return service.value.providers[0].slotTimeInMinutes * currentSlots.value; | |
| }); |
| const getMinSlotOfProvider = (provider: OfficeImpl[]) => { | ||
| let minSlot = MAX_SLOTS; | ||
| provider.forEach((provider) => { | ||
| if (provider.slots) { | ||
| minSlot = Math.min(minSlot, provider.slots); | ||
| } | ||
| }); | ||
| return minSlot; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for the case where no providers are available.
The getMinSlotOfProvider function could return an unreasonable value if the provider array is empty.
const getMinSlotOfProvider = (provider: OfficeImpl[]) => {
+ if (!provider || provider.length === 0) {
+ return 0;
+ }
let minSlot = MAX_SLOTS;
provider.forEach((provider) => {
if (provider.slots) {
minSlot = Math.min(minSlot, provider.slots);
}
});
return minSlot;
};📝 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 getMinSlotOfProvider = (provider: OfficeImpl[]) => { | |
| let minSlot = MAX_SLOTS; | |
| provider.forEach((provider) => { | |
| if (provider.slots) { | |
| minSlot = Math.min(minSlot, provider.slots); | |
| } | |
| }); | |
| return minSlot; | |
| }; | |
| const getMinSlotOfProvider = (provider: OfficeImpl[]) => { | |
| if (!provider || provider.length === 0) { | |
| return 0; | |
| } | |
| let minSlot = MAX_SLOTS; | |
| provider.forEach((provider) => { | |
| if (provider.slots) { | |
| minSlot = Math.min(minSlot, provider.slots); | |
| } | |
| }); | |
| return minSlot; | |
| }; |
| const setServiceData = (selectedService: ServiceImpl) => { | ||
| service.value.providers = getProviders(selectedService.id, null); | ||
| service.value.count = 1; | ||
| currentSlots.value = getMinSlotOfProvider(service.value.providers); | ||
|
|
||
| if (selectedService.combinable) { | ||
| const combinable = selectedService.combinable; | ||
| if (typeof combinable[parseInt(selectedService.id)] !== "undefined") { | ||
| delete combinable[parseInt(selectedService.id)]; | ||
| } | ||
|
|
||
| service.value.subServices = Object.entries(combinable) | ||
| .map(([subServiceId, providers]) => { | ||
| const subService = services.value.filter( | ||
| (subService) => parseInt(subService.id) == parseInt(subServiceId) | ||
| ); | ||
| if (subService && subService.length === 1) { | ||
| return { | ||
| id: parseInt(subServiceId), | ||
| name: subService[0].name, | ||
| maxQuantity: subService[0].maxQuantity, | ||
| providers: getProviders(subServiceId, providers), | ||
| count: 0, | ||
| }; | ||
| } | ||
| }) | ||
| .filter((subService) => { | ||
| if (subService === undefined) return false; | ||
| if (props.preselectedOfficeId) { | ||
| return subService.providers.some( | ||
| (provider) => provider.id == props.preselectedOfficeId | ||
| ); | ||
| } | ||
| return true; | ||
| }); | ||
| } | ||
|
|
||
| const maxSlotsOfProvider = getMaxSlotsPerAppointementOfProvider( | ||
| service.value.providers | ||
| ); | ||
| maxSlotsPerAppointment.value = | ||
| maxSlotsOfProvider > 0 | ||
| ? Math.min(maxSlotsOfProvider, MAX_SLOTS) | ||
| : MAX_SLOTS; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor complex service data processing function.
The setServiceData function is complex with multiple responsibilities: updating providers, calculating slots, processing combinable services, and determining max slots. Consider breaking it down into smaller, focused functions.
+/**
+ * Process subservices for a selected service
+ */
+const processSubservices = (selectedService: ServiceImpl, preselectedOfficeId?: string) => {
+ if (!selectedService.combinable) {
+ return [];
+ }
+
+ const combinable = {...selectedService.combinable};
+ if (typeof combinable[parseInt(selectedService.id)] !== "undefined") {
+ delete combinable[parseInt(selectedService.id)];
+ }
+
+ return Object.entries(combinable)
+ .map(([subServiceId, providers]) => {
+ const subService = services.value.filter(
+ (subService) => parseInt(subService.id) == parseInt(subServiceId)
+ );
+ if (subService && subService.length === 1) {
+ return {
+ id: parseInt(subServiceId),
+ name: subService[0].name,
+ maxQuantity: subService[0].maxQuantity,
+ providers: getProviders(subServiceId, providers),
+ count: 0,
+ };
+ }
+ })
+ .filter((subService) => {
+ if (subService === undefined) return false;
+ if (preselectedOfficeId) {
+ return subService.providers.some(
+ (provider) => provider.id == preselectedOfficeId
+ );
+ }
+ return true;
+ });
+};
const setServiceData = (selectedService: ServiceImpl) => {
service.value.providers = getProviders(selectedService.id, null);
service.value.count = 1;
currentSlots.value = getMinSlotOfProvider(service.value.providers);
- if (selectedService.combinable) {
- const combinable = selectedService.combinable;
- if (typeof combinable[parseInt(selectedService.id)] !== "undefined") {
- delete combinable[parseInt(selectedService.id)];
- }
-
- service.value.subServices = Object.entries(combinable)
- .map(([subServiceId, providers]) => {
- const subService = services.value.filter(
- (subService) => parseInt(subService.id) == parseInt(subServiceId)
- );
- if (subService && subService.length === 1) {
- return {
- id: parseInt(subServiceId),
- name: subService[0].name,
- maxQuantity: subService[0].maxQuantity,
- providers: getProviders(subServiceId, providers),
- count: 0,
- };
- }
- })
- .filter((subService) => {
- if (subService === undefined) return false;
- if (props.preselectedOfficeId) {
- return subService.providers.some(
- (provider) => provider.id == props.preselectedOfficeId
- );
- }
- return true;
- });
- }
+ service.value.subServices = processSubservices(selectedService, props.preselectedOfficeId);
const maxSlotsOfProvider = getMaxSlotsPerAppointementOfProvider(
service.value.providers
);
maxSlotsPerAppointment.value =
maxSlotsOfProvider > 0
? Math.min(maxSlotsOfProvider, MAX_SLOTS)
: MAX_SLOTS;
};📝 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 setServiceData = (selectedService: ServiceImpl) => { | |
| service.value.providers = getProviders(selectedService.id, null); | |
| service.value.count = 1; | |
| currentSlots.value = getMinSlotOfProvider(service.value.providers); | |
| if (selectedService.combinable) { | |
| const combinable = selectedService.combinable; | |
| if (typeof combinable[parseInt(selectedService.id)] !== "undefined") { | |
| delete combinable[parseInt(selectedService.id)]; | |
| } | |
| service.value.subServices = Object.entries(combinable) | |
| .map(([subServiceId, providers]) => { | |
| const subService = services.value.filter( | |
| (subService) => parseInt(subService.id) == parseInt(subServiceId) | |
| ); | |
| if (subService && subService.length === 1) { | |
| return { | |
| id: parseInt(subServiceId), | |
| name: subService[0].name, | |
| maxQuantity: subService[0].maxQuantity, | |
| providers: getProviders(subServiceId, providers), | |
| count: 0, | |
| }; | |
| } | |
| }) | |
| .filter((subService) => { | |
| if (subService === undefined) return false; | |
| if (props.preselectedOfficeId) { | |
| return subService.providers.some( | |
| (provider) => provider.id == props.preselectedOfficeId | |
| ); | |
| } | |
| return true; | |
| }); | |
| } | |
| const maxSlotsOfProvider = getMaxSlotsPerAppointementOfProvider( | |
| service.value.providers | |
| ); | |
| maxSlotsPerAppointment.value = | |
| maxSlotsOfProvider > 0 | |
| ? Math.min(maxSlotsOfProvider, MAX_SLOTS) | |
| : MAX_SLOTS; | |
| }; | |
| /** | |
| * Process subservices for a selected service | |
| */ | |
| const processSubservices = (selectedService: ServiceImpl, preselectedOfficeId?: string) => { | |
| if (!selectedService.combinable) { | |
| return []; | |
| } | |
| const combinable = { ...selectedService.combinable }; | |
| if (typeof combinable[parseInt(selectedService.id)] !== "undefined") { | |
| delete combinable[parseInt(selectedService.id)]; | |
| } | |
| return Object.entries(combinable) | |
| .map(([subServiceId, providers]) => { | |
| const subService = services.value.filter( | |
| (subService) => parseInt(subService.id) == parseInt(subServiceId) | |
| ); | |
| if (subService && subService.length === 1) { | |
| return { | |
| id: parseInt(subServiceId), | |
| name: subService[0].name, | |
| maxQuantity: subService[0].maxQuantity, | |
| providers: getProviders(subServiceId, providers), | |
| count: 0, | |
| }; | |
| } | |
| }) | |
| .filter((subService) => { | |
| if (subService === undefined) return false; | |
| if (preselectedOfficeId) { | |
| return subService.providers.some( | |
| (provider) => provider.id == preselectedOfficeId | |
| ); | |
| } | |
| return true; | |
| }); | |
| }; | |
| const setServiceData = (selectedService: ServiceImpl) => { | |
| service.value.providers = getProviders(selectedService.id, null); | |
| service.value.count = 1; | |
| currentSlots.value = getMinSlotOfProvider(service.value.providers); | |
| service.value.subServices = processSubservices(selectedService, props.preselectedOfficeId); | |
| const maxSlotsOfProvider = getMaxSlotsPerAppointementOfProvider( | |
| service.value.providers | |
| ); | |
| maxSlotsPerAppointment.value = | |
| maxSlotsOfProvider > 0 | |
| ? Math.min(maxSlotsOfProvider, MAX_SLOTS) | |
| : MAX_SLOTS; | |
| }; |
| const foundOffice: OfficeImpl = offices.value.filter((office) => { | ||
| return office.id == relation.officeId; | ||
| })[0]; | ||
|
|
||
| if ( | ||
| !providers || | ||
| providers.find((prov) => prov == foundOffice.id.toString()) | ||
| ) { | ||
| foundOffice.slots = relation.slots; | ||
| officesAtService.push(foundOffice); | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle empty array access safely.
When filtering offices, the code directly accesses index [0] without checking if the array is empty, which could lead to undefined errors.
const getProviders = (serviceId: string, providers: string[] | null) => {
const officesAtService = new Array<OfficeImpl>();
relations.value.forEach((relation) => {
if (relation.serviceId == serviceId) {
- const foundOffice: OfficeImpl = offices.value.filter((office) => {
+ const filteredOffices = offices.value.filter((office) => {
return office.id == relation.officeId;
- })[0];
+ });
+
+ if (filteredOffices.length === 0) {
+ return;
+ }
+
+ const foundOffice: OfficeImpl = filteredOffices[0];
if (
!providers ||
providers.find((prov) => prov == foundOffice.id.toString())📝 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 foundOffice: OfficeImpl = offices.value.filter((office) => { | |
| return office.id == relation.officeId; | |
| })[0]; | |
| if ( | |
| !providers || | |
| providers.find((prov) => prov == foundOffice.id.toString()) | |
| ) { | |
| foundOffice.slots = relation.slots; | |
| officesAtService.push(foundOffice); | |
| } | |
| } | |
| const getProviders = (serviceId: string, providers: string[] | null) => { | |
| const officesAtService = new Array<OfficeImpl>(); | |
| relations.value.forEach((relation) => { | |
| if (relation.serviceId == serviceId) { | |
| const filteredOffices = offices.value.filter((office) => { | |
| return office.id == relation.officeId; | |
| }); | |
| if (filteredOffices.length === 0) { | |
| return; | |
| } | |
| const foundOffice: OfficeImpl = filteredOffices[0]; | |
| if ( | |
| !providers || | |
| providers.find((prov) => prov == foundOffice.id.toString()) | |
| ) { | |
| foundOffice.slots = relation.slots; | |
| officesAtService.push(foundOffice); | |
| } | |
| } | |
| }); | |
| return officesAtService; | |
| }; |
| const getProviders = (serviceId: string, providers: string[] | null) => { | ||
| const officesAtService = new Array<OfficeImpl>(); | ||
| relations.value.forEach((relation) => { | ||
| if (relation.serviceId == serviceId) { | ||
| const foundOffice: OfficeImpl = offices.value.filter((office) => { | ||
| return office.id == relation.officeId; | ||
| })[0]; | ||
|
|
||
| if (!providers || providers.includes(foundOffice.id.toString())) { | ||
| foundOffice.slots = relation.slots; | ||
| officesAtService.push(foundOffice); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| return officesAtService; | ||
| }; |
There was a problem hiding this comment.
Add validation for array access in getProviders.
The getProviders function has a similar issue as in ServiceFinder, accessing array index [0] without checking if the filtered array is empty.
const getProviders = (serviceId: string, providers: string[] | null) => {
const officesAtService = new Array<OfficeImpl>();
relations.value.forEach((relation) => {
if (relation.serviceId == serviceId) {
- const foundOffice: OfficeImpl = offices.value.filter((office) => {
+ const filteredOffices = offices.value.filter((office) => {
return office.id == relation.officeId;
- })[0];
+ });
+
+ if (filteredOffices.length === 0) {
+ return;
+ }
+
+ const foundOffice: OfficeImpl = filteredOffices[0];
if (!providers || providers.includes(foundOffice.id.toString())) {
foundOffice.slots = relation.slots;
officesAtService.push(foundOffice);
}
}
});
return officesAtService;
};📝 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 getProviders = (serviceId: string, providers: string[] | null) => { | |
| const officesAtService = new Array<OfficeImpl>(); | |
| relations.value.forEach((relation) => { | |
| if (relation.serviceId == serviceId) { | |
| const foundOffice: OfficeImpl = offices.value.filter((office) => { | |
| return office.id == relation.officeId; | |
| })[0]; | |
| if (!providers || providers.includes(foundOffice.id.toString())) { | |
| foundOffice.slots = relation.slots; | |
| officesAtService.push(foundOffice); | |
| } | |
| } | |
| }); | |
| return officesAtService; | |
| }; | |
| const getProviders = (serviceId: string, providers: string[] | null) => { | |
| const officesAtService = new Array<OfficeImpl>(); | |
| relations.value.forEach((relation) => { | |
| if (relation.serviceId == serviceId) { | |
| const filteredOffices = offices.value.filter((office) => { | |
| return office.id == relation.officeId; | |
| }); | |
| if (filteredOffices.length === 0) { | |
| return; | |
| } | |
| const foundOffice: OfficeImpl = filteredOffices[0]; | |
| if (!providers || providers.includes(foundOffice.id.toString())) { | |
| foundOffice.slots = relation.slots; | |
| officesAtService.push(foundOffice); | |
| } | |
| } | |
| }); | |
| return officesAtService; | |
| }; |
| fetchAppointment(appointmentData, props.baseUrl ?? undefined).then( | ||
| (data) => { | ||
| if ((data as AppointmentDTO).processId != undefined) { | ||
| appointment.value = data as AppointmentDTO; | ||
| selectedService.value = services.value.find( | ||
| (service) => service.id == appointment.value.serviceId | ||
| ); | ||
| if (selectedService.value) { | ||
| selectedService.value.count = appointment.value.serviceCount; | ||
| selectedService.value.providers = getProviders( | ||
| selectedService.value.id, | ||
| null | ||
| ); | ||
|
|
||
| preselectedLocationId.value = appointment.value.officeId; | ||
| selectedProvider.value = offices.value.find( | ||
| (office) => office.id == appointment.value?.officeId | ||
| ); | ||
|
|
||
| if (appointment.value.subRequestCounts.length > 0) { | ||
| appointment.value.subRequestCounts.forEach( | ||
| (subRequestCount) => { | ||
| const subRequest: Service = services.value.find( | ||
| (service) => service.id == subRequestCount.id | ||
| ); | ||
| const subService = new SubService( | ||
| subRequest.id, | ||
| subRequest.name, | ||
| subRequest.maxQuantity, | ||
| getProviders(subRequest.id, null), | ||
| subRequestCount.count | ||
| ); | ||
| if (!selectedService.value.subServices) { | ||
| selectedService.value.subServices = []; | ||
| } | ||
| selectedService.value.subServices.push(subService); | ||
| } | ||
| ); | ||
| } | ||
| currentView.value = 3; | ||
| } | ||
| } else { | ||
| appointmentNotFoundError.value = true; | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and refactor nested logic in the fetchAppointment call.
The fetchAppointment API call contains deeply nested logic without proper error handling. Consider refactoring it for better maintainability.
fetchAppointment(appointmentData, props.baseUrl ?? undefined).then(
(data) => {
if ((data as AppointmentDTO).processId != undefined) {
appointment.value = data as AppointmentDTO;
- selectedService.value = services.value.find(
- (service) => service.id == appointment.value.serviceId
- );
- if (selectedService.value) {
- selectedService.value.count = appointment.value.serviceCount;
- selectedService.value.providers = getProviders(
- selectedService.value.id,
- null
- );
-
- preselectedLocationId.value = appointment.value.officeId;
- selectedProvider.value = offices.value.find(
- (office) => office.id == appointment.value?.officeId
- );
-
- if (appointment.value.subRequestCounts.length > 0) {
- appointment.value.subRequestCounts.forEach(
- (subRequestCount) => {
- const subRequest: Service = services.value.find(
- (service) => service.id == subRequestCount.id
- );
- const subService = new SubService(
- subRequest.id,
- subRequest.name,
- subRequest.maxQuantity,
- getProviders(subRequest.id, null),
- subRequestCount.count
- );
- if (!selectedService.value.subServices) {
- selectedService.value.subServices = [];
- }
- selectedService.value.subServices.push(subService);
- }
- );
- }
- currentView.value = 3;
- }
+ setupAppointmentData(appointment.value);
+ currentView.value = 3;
} else {
appointmentNotFoundError.value = true;
}
}
- );
+ ).catch(error => {
+ console.error("Failed to fetch appointment:", error);
+ appointmentNotFoundError.value = true;
+ });
+/**
+ * Set up appointment data after successful fetch
+ */
+const setupAppointmentData = (appointmentData: AppointmentDTO) => {
+ selectedService.value = services.value.find(
+ (service) => service.id == appointmentData.serviceId
+ );
+
+ if (!selectedService.value) {
+ return;
+ }
+
+ selectedService.value.count = appointmentData.serviceCount;
+ selectedService.value.providers = getProviders(
+ selectedService.value.id,
+ null
+ );
+
+ preselectedLocationId.value = appointmentData.officeId;
+ selectedProvider.value = offices.value.find(
+ (office) => office.id == appointmentData?.officeId
+ );
+
+ if (appointmentData.subRequestCounts.length > 0) {
+ setupSubServices(appointmentData.subRequestCounts);
+ }
+};
+
+/**
+ * Set up sub-services for an appointment
+ */
+const setupSubServices = (subRequestCounts) => {
+ if (!selectedService.value) return;
+
+ if (!selectedService.value.subServices) {
+ selectedService.value.subServices = [];
+ }
+
+ subRequestCounts.forEach((subRequestCount) => {
+ const subRequest: Service = services.value.find(
+ (service) => service.id == subRequestCount.id
+ );
+
+ if (!subRequest) return;
+
+ const subService = new SubService(
+ subRequest.id,
+ subRequest.name,
+ subRequest.maxQuantity,
+ getProviders(subRequest.id, null),
+ subRequestCount.count
+ );
+
+ selectedService.value.subServices.push(subService);
+ });
+};📝 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.
| fetchAppointment(appointmentData, props.baseUrl ?? undefined).then( | |
| (data) => { | |
| if ((data as AppointmentDTO).processId != undefined) { | |
| appointment.value = data as AppointmentDTO; | |
| selectedService.value = services.value.find( | |
| (service) => service.id == appointment.value.serviceId | |
| ); | |
| if (selectedService.value) { | |
| selectedService.value.count = appointment.value.serviceCount; | |
| selectedService.value.providers = getProviders( | |
| selectedService.value.id, | |
| null | |
| ); | |
| preselectedLocationId.value = appointment.value.officeId; | |
| selectedProvider.value = offices.value.find( | |
| (office) => office.id == appointment.value?.officeId | |
| ); | |
| if (appointment.value.subRequestCounts.length > 0) { | |
| appointment.value.subRequestCounts.forEach( | |
| (subRequestCount) => { | |
| const subRequest: Service = services.value.find( | |
| (service) => service.id == subRequestCount.id | |
| ); | |
| const subService = new SubService( | |
| subRequest.id, | |
| subRequest.name, | |
| subRequest.maxQuantity, | |
| getProviders(subRequest.id, null), | |
| subRequestCount.count | |
| ); | |
| if (!selectedService.value.subServices) { | |
| selectedService.value.subServices = []; | |
| } | |
| selectedService.value.subServices.push(subService); | |
| } | |
| ); | |
| } | |
| currentView.value = 3; | |
| } | |
| } else { | |
| appointmentNotFoundError.value = true; | |
| } | |
| } | |
| ); | |
| fetchAppointment(appointmentData, props.baseUrl ?? undefined).then( | |
| (data) => { | |
| if ((data as AppointmentDTO).processId != undefined) { | |
| appointment.value = data as AppointmentDTO; | |
| setupAppointmentData(appointment.value); | |
| currentView.value = 3; | |
| } else { | |
| appointmentNotFoundError.value = true; | |
| } | |
| } | |
| ).catch(error => { | |
| console.error("Failed to fetch appointment:", error); | |
| appointmentNotFoundError.value = true; | |
| }); | |
| /** | |
| * Set up appointment data after successful fetch | |
| */ | |
| const setupAppointmentData = (appointmentData: AppointmentDTO) => { | |
| selectedService.value = services.value.find( | |
| (service) => service.id == appointmentData.serviceId | |
| ); | |
| if (!selectedService.value) { | |
| return; | |
| } | |
| selectedService.value.count = appointmentData.serviceCount; | |
| selectedService.value.providers = getProviders( | |
| selectedService.value.id, | |
| null | |
| ); | |
| preselectedLocationId.value = appointmentData.officeId; | |
| selectedProvider.value = offices.value.find( | |
| (office) => office.id == appointmentData?.officeId | |
| ); | |
| if (appointmentData.subRequestCounts.length > 0) { | |
| setupSubServices(appointmentData.subRequestCounts); | |
| } | |
| }; | |
| /** | |
| * Set up sub-services for an appointment | |
| */ | |
| const setupSubServices = (subRequestCounts) => { | |
| if (!selectedService.value) return; | |
| if (!selectedService.value.subServices) { | |
| selectedService.value.subServices = []; | |
| } | |
| subRequestCounts.forEach((subRequestCount) => { | |
| const subRequest: Service = services.value.find( | |
| (service) => service.id == subRequestCount.id | |
| ); | |
| if (!subRequest) return; | |
| const subService = new SubService( | |
| subRequest.id, | |
| subRequest.name, | |
| subRequest.maxQuantity, | |
| getProviders(subRequest.id, null), | |
| subRequestCount.count | |
| ); | |
| selectedService.value.subServices.push(subService); | |
| }); | |
| }; |
Pull Request Checklist (Feature Branch to
next):nextBranch in meinen Feature-Branch gemergt.Summary by CodeRabbit
New Features
Chores