feat(ZMSKVR): Prototype refactor zmscalldisplay to zmscitizencalldisplay refArch vuejs with apigateway yolo don't merge #1463
Conversation
…gration - Create new zmscitizencalldisplay Vue.js application - Configure API gateway to handle zmsapi requests - Implement calldisplay functionality with MUC Pattern Lab components - Add internationalization support with vue-i18n - Set up environment-based API URL configuration - Add error handling with MUC callout components - Create configuration message for missing scope parameters - Update GitHub Actions workflow to build both applications - Maintain parallel operation with existing PHP zmscalldisplay
WalkthroughAdds a new Spring Cloud Gateway route, extends CI to build a new frontend app, and introduces a complete zmscitizencalldisplay Vue 3/Vite application (assets, API client, components, i18n, audio/blink utilities, tests, Docker/Nginx, and configs). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Browser as zmscitizencalldisplay (Frontend)
participant Gateway as Spring Cloud Gateway
participant ZMS as ddev-zms-web (Backend)
User->>Browser: Open calldisplay URL (with params)
Browser->>Browser: parseCalldisplayUrlParams()
Browser->>Gateway: POST /buergeransicht/api/calldisplay/ (payload)
Gateway->>Gateway: RewritePath -> /terminvereinbarung/api/2/calldisplay/
Gateway->>ZMS: Forward request
ZMS-->>Gateway: Response
Gateway-->>Browser: Response (WWW-Authenticate removed)
loop polling every 5s
Browser->>Gateway: POST /buergeransicht/api/calldisplay/queue/?statusList=...
Gateway->>ZMS: Forward (rewritten)
ZMS-->>Gateway: Queue payload
Gateway-->>Browser: Queue payload
Browser->>Browser: audioManager.updateAndPlayAudio(queue)
Browser->>Browser: blinkElementsByAppointmentIds(newIds)
end
Browser->>Gateway: GET /buergeransicht/api/scope/{id}/
Gateway->>ZMS: Forward (rewritten)
ZMS-->>Gateway: Scope data
Gateway-->>Browser: Scope data
sequenceDiagram
autonumber
participant CI as GitHub Actions
participant Repo as Repository
CI->>Repo: workflow trigger
CI->>CI: matrix include: zmscitizencalldisplay
CI->>CI: npm ci / build (if package.json present)
CI->>CI: maven build (if pom.xml present)
CI->>CI: container image build (conditional)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (34)
zmscitizencalldisplay/.env.development (1)
1-1: Align base URL trailing slash with production (or normalize join in client).Dev uses no trailing slash; prod does. To avoid double/missing slashes when building endpoints, prefer consistency.
Apply this diff:
-VITE_VUE_APP_API_URL=http://localhost:8084 +VITE_VUE_APP_API_URL=http://localhost:8084/zmscitizencalldisplay/tsconfig.node.json (1)
1-12: Add Node types for Vite config.Prevents missing Node globals in vite.config.ts.
Apply this diff:
"compilerOptions": { "composite": true, "skipLibCheck": true, "module": "ESNext", "moduleResolution": "bundler", "allowSyntheticDefaultImports": true, "strict": true, - "noEmit": true + "noEmit": true, + "types": ["node"] },zmscitizencalldisplay/tsconfig.app.json (1)
1-9: Enable isolatedModules and drop TSX include (Vue app).isolation helps with SFC toolchains; TSX include can be removed unless you really use TSX.
Apply this diff:
"compilerOptions": { "composite": true, - "noEmit": false + "noEmit": false, + "isolatedModules": true }, - "include": ["src/**/*.ts", "src/**/*.d.ts", "src/**/*.tsx", "src/**/*.vue"], + "include": ["src/**/*.ts", "src/**/*.d.ts", "src/**/*.vue"],zmscitizencalldisplay/src/types/svg.d.ts (1)
1-4: LGTM; consider covering common SVG import variants too.Add bare "*.svg" and "?url" for DX.
Apply this diff:
declare module "*.svg?raw" { const content: string; export default content; } + +declare module "*.svg?url" { + const url: string; + export default url; +} + +declare module "*.svg" { + const url: string; + export default url; +}zmscitizencalldisplay/src/utils/en-US.json (2)
6-6: Avoid HTML in translations.Prefer line breaks via '\n' and render with CSS (white-space) or split paragraphs; reduces need for v-html.
Apply this diff:
- "errorContent": "The requested page cannot be displayed at the moment. This may be caused by technical problems that will be resolved shortly.<br/>Please try restarting this device first. If the problem persists, please report the issue at:", + "errorContent": "The requested page cannot be displayed at the moment. This may be caused by technical problems that will be resolved shortly.\nPlease try restarting this device first. If the problem persists, please report the issue at:",
14-19: Duplicate labels (number vs numberEn, counter vs counterEn).If the UI needs bilingual headers, render two locales, not duplicate keys in a single locale file.
Apply this diff if duplicates aren’t required:
"table": { - "number": "Number", - "numberEn": "Number", - "counter": "Counter", - "counterEn": "Counter" + "number": "Number", + "counter": "Counter" }zmscitizencalldisplay/package.json (5)
12-13: Run Vitest in a browser-like env to avoid DOMPurify/window issues.
Vitest defaults to node; DOMPurify expects a DOM. Use jsdom env.- "test": "vitest run", + "test": "vitest run --environment jsdom",
15-22: Move jsdom to devDependencies; it’s not needed at runtime.
Keeps runtime bundle lean and avoids accidental server-side install."dependencies": { "@muenchen/muc-patternlab-vue": "5.6.2", "dompurify": "^3.2.4", - "jsdom": "^26.0.0", "vue": "^3.5.12", "vue-i18n": "^10.0.8", "webfontloader": "^1.0.0" }, "devDependencies": { + "jsdom": "^26.0.0",
9-11: Wire up ESLint or drop the dependency.
You install ESLint but don’t run it. Either add a script or remove ESLint packages."scripts": { "dev": "vite", "preview": "vite preview", "build": "vite build --mode production", "type-check": "vue-tsc --noEmit -p tsconfig.app.json --composite false", - "lint": "prettier src/ --check", + "lint": "eslint . --ext .ts,.tsx,.vue --max-warnings=0 && prettier src/ --check", "format": "prettier --write src/",Also applies to: 23-41
42-46: Verify necessity of the make-dir override.
vite-plugin-vuetify isn’t listed; if unused, drop the override to avoid confusion.
21-21: Consider bumping webfontloader to a modern release.
1.0.0 is very old and may miss fixes. If you don’t rely on it, remove it.zmscitizencalldisplay/tsconfig.json (1)
23-27: Expose Vite/client types for import.meta.env and SVG imports.
Improves DX and avoids ambient type gaps./* Path mapping */ "baseUrl": ".", "paths": { "@/*": ["./src/*"] - } + }, + "types": ["vite/client"]zmscitizencalldisplay/src/utils/sanitizeHtml.ts (1)
22-26: Guard for non-browser/test environments.
If executed in node tests without jsdom env, DOMPurify will fail. You already added jsdom; run vitest with jsdom (see package.json change) or short‑circuit here.export function sanitizeHtml(dirtyHtml: string | null | undefined): string { const input = (dirtyHtml ?? "").toString(); if (input.trim() === "") return ""; - return DOMPurify.sanitize(input, config) as string; + // In non-DOM environments, return a conservative empty string + if (typeof window === "undefined") return ""; + return DOMPurify.sanitize(input, config) as string; }zmscitizencalldisplay/vite.config.ts (3)
17-19: Avoid clobbering process.env.
Some libs expect NODE_ENV. Define only what you need.- define: {'process.env': {}}, + define: { 'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV ?? 'production') },
18-33: Remove unsupported/unused Vite options.
- assetsInclude for svg is unnecessary
- resolve.extensions is a webpack option and ignored by Vite
- assetsInclude: ['**/*.svg'], resolve: { dedupe: ['vue'], alias: { '@': fileURLToPath(new URL('./src', import.meta.url)) - }, - extensions: - [ - '.js', - '.json', - '.jsx', - '.mjs', - '.ts', - '.tsx', - '.vue', - ], + } },
37-51: Simplify build output unless you have a strict consumer contract.
assetsDir "src" is unconventional and can confuse asset loaders; ssrManifest is not needed without SSR.build: { - ssrManifest: true, - manifest: true, - minify: true, - assetsDir: "src", + manifest: true, + minify: true, + assetsDir: "assets", rollupOptions: { - input: { - "index": "./index.html" - }, + input: { index: "./index.html" }, output: { - entryFileNames: "src/entry-[name]-[hash].js", + entryFileNames: "assets/entry-[name]-[hash].js", dir: "dist", }, } },.ddev/local-gateway-application.yml (1)
22-28: Anchor RewritePath and verify the group matches all four prefixes.
Minor hardening; avoids accidental partial rewrites.- - RewritePath=/buergeransicht/api/(?<segment>.*), /terminvereinbarung/api/2/${segment} + - RewritePath=^/buergeransicht/api/(?<segment>.*), /terminvereinbarung/api/2/${segment}Please confirm that backend paths are indeed under /terminvereinbarung/api/2/... for calldisplay, scope, status, and config in local and non-local profiles.
zmscitizencalldisplay/index.html (2)
19-22: Add a noscript fallback.
Basic UX if JS is disabled.<body> <div id="app"></div> + <noscript>Bitte aktivieren Sie JavaScript, um diese Seite zu nutzen.</noscript> <script type="module" src="/src/main.ts"></script> </body>
10-17: Optional: add SRI to external styles or self-host.
Improves supply‑chain security.zmscitizencalldisplay/src/utils/de-DE.json (1)
6-7: Avoid HTML in translations when possible.
Prefer structured strings and render line breaks in the component to keep i18n plain-text.- "errorContent": "Die gewünschte Seite kann derzeit nicht angezeigt werden. Dies kann durch technische Probleme verursacht werden, die kurzfristig behoben werden.<br/>Versuchen Sie bitte zunächst, dieses Gerät neu zu starten. Sollte das Problem weiterhin bestehen, melden Sie die Störung bitte unter:", + "errorContentLine1": "Die gewünschte Seite kann derzeit nicht angezeigt werden. Dies kann durch technische Probleme verursacht werden, die kurzfristig behoben werden.", + "errorContentLine2": "Versuchen Sie bitte zunächst, dieses Gerät neu zu starten. Sollte das Problem weiterhin bestehen, melden Sie die Störung bitte unter:",zmscitizencalldisplay/src/i18n.ts (1)
5-13: Consider exposing runtime locale switch via detect/param.
Defaulting to de-DE is fine; optionally read from navigator.language or a URL param.-const i18n = createI18n({ +const i18n = createI18n({ legacy: false, - locale: "de-DE", + locale: (typeof navigator !== "undefined" && navigator.language) || "de-DE", fallbackLocale: "en-US",zmscitizencalldisplay/src/vite-env.d.ts (1)
3-5: Make env var optional and consider clearer naming.The env may be unset at runtime. Typing it as optional avoids false guarantees; also consider a clearer name (e.g., VITE_API_BASE_URL).
-interface ImportMetaEnv { - readonly VITE_VUE_APP_API_URL: string; -} +interface ImportMetaEnv { + readonly VITE_VUE_APP_API_URL?: string; +}zmscitizencalldisplay/src/components/ConfigMessage.vue (1)
3-15: Move inline styles into scoped CSS class.Inline styles hinder reusability and theming; use a class.
- <div class="config-message"> - <muc-callout type="info" style="max-width: 80%; margin: 5% auto"> + <div class="config-message"> + <muc-callout type="info" class="config-callout">Add to the <style scoped> block:
.config-message { padding: 20px; display: flex; justify-content: center; align-items: center; min-height: 200px; } + +.config-callout { + max-width: 80%; + margin: 5% auto; +}zmscitizencalldisplay/src/styles/main.scss (2)
22-29: Respect reduced motion preferences for animations.Disable spin animations when users prefer reduced motion.
@keyframes spin { 0% { transform: rotate(0deg); } 100% { transform: rotate(360deg); } } + +@media (prefers-reduced-motion: reduce) { + .spin, .refresh-indicator { + animation: none !important; + } +}
92-112: Minimize use of !important in utility classes.Frequent !important reduces maintainability and makes overrides harder. Consider specificity or utility layers instead.
zmscitizencalldisplay/src/utils/urlParams.ts (1)
50-53: Filter out blank queue statuses.Avoid producing [""] when query ends with a comma.
- config.queue = { - status: queueStatus.split(",").map((s) => s.trim()), - }; + const status = queueStatus + .split(",") + .map((s) => s.trim()) + .filter(Boolean); + if (status.length) { + config.queue = { status }; + }zmscitizencalldisplay/src/components/CalldisplayView.vue (5)
376-387: Wrong timer type in browser TS.Use ReturnType instead of NodeJS.Timeout to avoid DOM/Node typing conflicts.
-let timeInterval: NodeJS.Timeout | null = null; +let timeInterval: ReturnType<typeof setInterval> | null = null;
21-25: Avoid v-html for local time markup.It’s safe here, but unnecessary. Render date/time in separate nodes to drop v-html.
Also applies to: 354-367
338-347: Prop ‘refreshing’ is unused.Either use to show a subtle loader or remove to keep the surface area minimal.
445-452: Duplicate keyframes ‘spin’.There’s a global animation in main.scss; keep one definition to avoid divergence.
Also applies to: 486-493
623-631: Provide a visible default for unknown status.You return "status-unknown" but have no style for it. Add a neutral fallback.
+.queue-number.status-unknown, +.queue-destination.status-unknown { + background: #e9ecef; + color: #2c3e50; +}zmscitizencalldisplay/src/CalldisplayApp.vue (3)
67-72: Use radix with parseInt to avoid octal quirks and be explicit.- .map((id) => parseInt(id.trim())) + .map((id) => parseInt(id.trim(), 10)) @@ - .map((id) => parseInt(id.trim())) + .map((id) => parseInt(id.trim(), 10)) @@ - if (display) { - displayNumber.value = parseInt(display); - } + if (display) { + displayNumber.value = parseInt(display, 10); + }Also applies to: 74-79, 87-91
35-35: Prefer extensionless TS imports for better tooling interoperability.-} from "./api/CalldisplayAPI.ts"; +} from "./api/CalldisplayAPI";
48-49: displayNumber is parsed but unused. Remove or pass to children.Either remove the state and parsing, or plumb it into CalldisplayView/logic.
Want a patch to remove it now, or should I pass it to CalldisplayView as a prop?
Also applies to: 87-91
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
zmscitizencalldisplay/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
.ddev/local-gateway-application.yml(1 hunks).github/workflows/maven-node-build.yaml(1 hunks)zmscitizencalldisplay/.env.development(1 hunks)zmscitizencalldisplay/.env.production(1 hunks)zmscitizencalldisplay/.gitignore(1 hunks)zmscitizencalldisplay/index.html(1 hunks)zmscitizencalldisplay/package.json(1 hunks)zmscitizencalldisplay/src/CalldisplayApp.vue(1 hunks)zmscitizencalldisplay/src/api/CalldisplayAPI.ts(1 hunks)zmscitizencalldisplay/src/components/CalldisplayView.vue(1 hunks)zmscitizencalldisplay/src/components/ConfigMessage.vue(1 hunks)zmscitizencalldisplay/src/i18n.ts(1 hunks)zmscitizencalldisplay/src/main.ts(1 hunks)zmscitizencalldisplay/src/styles/main.scss(1 hunks)zmscitizencalldisplay/src/types/index.ts(1 hunks)zmscitizencalldisplay/src/types/svg.d.ts(1 hunks)zmscitizencalldisplay/src/utils/de-DE.json(1 hunks)zmscitizencalldisplay/src/utils/en-US.json(1 hunks)zmscitizencalldisplay/src/utils/sanitizeHtml.ts(1 hunks)zmscitizencalldisplay/src/utils/urlParams.ts(1 hunks)zmscitizencalldisplay/src/vite-env.d.ts(1 hunks)zmscitizencalldisplay/tsconfig.app.json(1 hunks)zmscitizencalldisplay/tsconfig.json(1 hunks)zmscitizencalldisplay/tsconfig.node.json(1 hunks)zmscitizencalldisplay/vite.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the following Clean Code guidelines to all files, as summarized by wojteklu's Clean Code gist:Code is clean if it can be understood easily – by everyone on the team. Clean code can be read and enhanced by a developer other than its original author. With understandability comes readability, changeability, extensibility and maintainability.
General rules
- Follow standard conventions.
- Keep it simple stupid. Simpler is always better. Reduce complexity as much as possible.
- Boy scout rule. Leave the campground cleaner than you found it.
- Always find root cause. Always look for the root cause of a problem.
Design rules
- Keep configurable data at high levels.
- Prefer polymorphism to if/else or switch/case.
- Separate multi-threading code.
- Prevent over-configurability.
- Use dependency injection.
- Follow Law of Demeter. A class should know only its direct dependencies.
Understandability tips
- Be consistent. If you do something a certain way, do all similar things in the same way.
- Use explanatory variables.
- Encapsulate boundary conditions. Boundary conditions are hard to keep track of. Put the processing for them in one place.
- Prefer dedicated value objects to primitive type.
- Avoid logical dependency. Don't write methods which works correctly depending on something else in the same class.
- Avoid negative conditionals.
Names rules
- Choose descriptive and unambiguous names.
- Make meaningful distinction.
- Use pronounceable names.
- Use searchable names.
- Replace magic numbers with named constants.
- Avoid encodings. Don't append prefixes or type information.
Functions rules
- Small.
- Do one thing.
- Use descriptive names.
- Prefer fewer arguments.
- Have no side effects.
- Don't use flag arguments. Split method into several independent methods that can b...
Files:
zmscitizencalldisplay/tsconfig.node.jsonzmscitizencalldisplay/src/styles/main.scsszmscitizencalldisplay/tsconfig.app.jsonzmscitizencalldisplay/src/utils/de-DE.jsonzmscitizencalldisplay/tsconfig.jsonzmscitizencalldisplay/index.htmlzmscitizencalldisplay/src/types/svg.d.tszmscitizencalldisplay/src/utils/urlParams.tszmscitizencalldisplay/src/utils/sanitizeHtml.tszmscitizencalldisplay/src/i18n.tszmscitizencalldisplay/src/main.tszmscitizencalldisplay/vite.config.tszmscitizencalldisplay/package.jsonzmscitizencalldisplay/src/utils/en-US.jsonzmscitizencalldisplay/src/vite-env.d.tszmscitizencalldisplay/src/CalldisplayApp.vuezmscitizencalldisplay/src/components/ConfigMessage.vuezmscitizencalldisplay/src/types/index.tszmscitizencalldisplay/src/components/CalldisplayView.vuezmscitizencalldisplay/src/api/CalldisplayAPI.ts
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: Flag any usage of console.log() as it should be replaced with proper logging or removed:
- For development: console.debug()
- For production: Remove console.log() statements or use structured logging
- For errors: Use error console.error()
Example replacement:
// Instead of: console.log('User data:', userData); // Use: console.debug('Processing user data', { userData }); // or for development only: Remove the console.log entirelyFlag specific logging violations:
- console.log(), console.debug(), console.warn() usage (except console.error in catch blocks)
- Any debug output that should use proper logging frameworks
Example replacements:
// Instead of: console.log('User data:', userData); console.debug('Processing...'); // Use: // Remove console.log entirely or use proper logging // Only console.error in catch blocks is acceptable try { processData(userData); } catch (error) { console.error('Processing failed:', error); }Flag JavaScript security and UX issues:
- alert(), confirm(), prompt() usage (poor UX)
- eval() usage (security risk)
- innerHTML with user input (XSS risk)
- Unfiltered user input in DOM manipulation
Example replacements:
// Instead of: alert('Error occurred'); eval(userInput); // Use: // Use proper error handling and UI components this.showErrorNotification('Error occurred'); // Avoid eval() entirelyFlag TODO/FIXME comments that indicate technical debt:
- TODO comments without clear action items
- FIXME comments indicating broken functionality
- HACK comments indicating temporary workarounds
- XXX comments indicating problematic code
These should be converted to proper issues or addressed:
// Instead of: // TODO: fix this later // FIXME: this is broken // Use: // Create proper issue and reference it // @see Issue #123: Refactor validation logicFlag code complexity issues:
- Functions longer than 50 lines
- Deep nesting (...
Files:
zmscitizencalldisplay/src/types/svg.d.tszmscitizencalldisplay/src/utils/urlParams.tszmscitizencalldisplay/src/utils/sanitizeHtml.tszmscitizencalldisplay/src/i18n.tszmscitizencalldisplay/src/main.tszmscitizencalldisplay/vite.config.tszmscitizencalldisplay/src/vite-env.d.tszmscitizencalldisplay/src/types/index.tszmscitizencalldisplay/src/api/CalldisplayAPI.ts
🧬 Code graph analysis (4)
zmscitizencalldisplay/src/utils/urlParams.ts (2)
zmscitizencalldisplay/src/api/CalldisplayAPI.ts (1)
CalldisplayConfig(2-12)zmscitizencalldisplay/src/types/index.ts (1)
CalldisplayConfig(14-14)
zmscitizencalldisplay/src/utils/sanitizeHtml.ts (1)
zmscitizencalldisplay/src/api/CalldisplayAPI.ts (1)
Config(63-74)
zmscitizencalldisplay/src/types/index.ts (1)
zmscitizencalldisplay/src/api/CalldisplayAPI.ts (4)
CalldisplayEntity(14-19)QueueList(21-31)Scope(33-44)Config(63-74)
zmscitizencalldisplay/src/api/CalldisplayAPI.ts (1)
zmscitizencalldisplay/src/types/index.ts (7)
CalldisplayConfig(14-14)CalldisplayEntity(15-15)Scope(16-16)Cluster(17-17)QueueItem(18-18)Config(20-20)QueueList(19-19)
🪛 ast-grep (0.38.6)
zmscitizencalldisplay/src/main.ts
[warning] 16-16: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: iconContainer.innerHTML = mucIconsSprite + customIconsSprite
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 16-16: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: iconContainer.innerHTML = mucIconsSprite + customIconsSprite
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: call-php-build-images / build (zmsticketprinter, 8.3)
- GitHub Check: call-php-build-images / build (zmscalldisplay, 8.3)
- GitHub Check: call-php-build-images / build (zmsstatistic, 8.3)
- GitHub Check: call-php-build-images / build (zmsmessaging, 8.3)
- GitHub Check: call-php-build-images / build (zmscitizenapi, 8.3)
- GitHub Check: call-php-build-images / build (zmsapi, 8.3)
- GitHub Check: call-php-build-images / build (zmsadmin, 8.3)
🔇 Additional comments (10)
zmscitizencalldisplay/.env.production (1)
1-1: Keep base URL convention consistent with development.Prod has a trailing slash. After updating .env.development as suggested, both will align and reduce URL-join edge cases.
.github/workflows/maven-node-build.yaml (1)
20-20: Verify hashFiles condition; empty string vs null.hashFiles returns '' when no file matches; current checks use != null, which evaluates true for ''. Steps may run unintentionally for new app-paths.
Apply this diff:
- - if: ${{hashFiles(format('./{0}/package.json', matrix.app-path))!=null}} + - if: ${{ hashFiles(format('./{0}/package.json', matrix.app-path)) != '' }} ... - - if: ${{hashFiles(format('./{0}/pom.xml', matrix.app-path))!=null}} + - if: ${{ hashFiles(format('./{0}/pom.xml', matrix.app-path)) != '' }}zmscitizencalldisplay/.gitignore (1)
1-76: LGTM.Comprehensive ignores aligned with Vue/Vite. Ensure lockfiles (npm/yarn/pnpm) are committed at repo root/app level.
zmscitizencalldisplay/src/main.ts (1)
15-18: Sanity-check the sprite sources are immutable.Confirm both sprite imports come solely from versioned assets in @muenchen/muc-patternlab-vue (no user input). If there’s any chance of dynamism, sanitize before insertion.
zmscitizencalldisplay/src/utils/urlParams.ts (1)
1-126: Verify API type alignment: CalldisplayConfig.display must be a number everywhere.Automated repo search failed in the sandbox — manually confirm the CalldisplayConfig declaration uses display: number and that all usages (API types, URL parsers/serializers, and UI/consumer code) treat display as a number; update any locations still expecting a string.
zmscitizencalldisplay/src/CalldisplayApp.vue (2)
92-97: Trim and sanitize queue status values from URL.Prevents accidental whitespace/empty entries.
[suggest_minor_refactor]
- if (queueStatus) { - urlConfig.value.queue = { - status: queueStatus.split(","), - }; - } + if (queueStatus) { + urlConfig.value.queue = { + status: queueStatus.split(",").map((s) => s.trim()).filter(Boolean), + }; + }
127-129: Queue status defaults diverge from API defaults—align or centralize.UI uses ["called","pickup"]; API default is ["waiting","called","confirmed"]. Please confirm the intended set and centralize as a single constant to avoid drift.
Would you like me to add a shared DEFAULT_QUEUE_STATUSES constant and wire both call sites to it?
Also applies to: 180-182
zmscitizencalldisplay/src/api/CalldisplayAPI.ts (3)
155-156: Confirm intended default queue statuses and align with UI.API default is ["waiting","called","confirmed"] while UI uses ["called","pickup"]. Pick one set and document.
I can add a union type for statuses and a shared constant to enforce consistency across layers.
103-110: Verify backend expects string ids for scopes/clusters.You’re sending id as string while interfaces model id as number elsewhere. Ensure the backend contract requires strings; otherwise, send numbers to reduce coercion.
If numbers are acceptable, I’ll switch to numeric ids to match the TS interfaces.
Also applies to: 117-123
83-87: Env var name check.Ensure VITE_VUE_APP_API_URL is defined in your Vite envs; Vite only exposes variables prefixed with VITE_. The “VUE_APP_” middle segment is fine but confirm it matches your .env files and CI.
Want me to scan the repo for .env.* and GitHub Actions to confirm presence?
| "version": "0.0.0", | ||
| "type": "module", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mark package private and pin Node engine (prevents accidental publish, matches Vite requirements).
Add private:true and an engines constraint so CI/devs use a compatible Node.
"name": "@muenchen/zms-citizen-calldisplay",
"version": "0.0.0",
+ "private": true,
+ "engines": {
+ "node": ">=18.18"
+ },📝 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.
| "version": "0.0.0", | |
| "type": "module", | |
| "version": "0.0.0", | |
| "private": true, | |
| "engines": { | |
| "node": ">=18.18" | |
| }, | |
| "type": "module", |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/package.json around lines 3-4, mark the package private
and add an engines constraint to pin the Node version so it can't be
accidentally published and CI/devs use a compatible Node for Vite; add
"private": true at the top-level of package.json and add an "engines" object
(e.g. a Node semver range compatible with your Vite version such as "node":
">=16.8.0") to enforce the Node runtime used by contributors and CI.
| baseUrl || | ||
| import.meta.env.VITE_VUE_APP_API_URL || | ||
| "http://localhost:8084"; | ||
| console.log("CalldisplayAPI initialized with baseUrl:", this.baseUrl); |
There was a problem hiding this comment.
Remove console. to avoid leaking request/response data.*
These logs risk over‑logging and can expose payloads/responses. Use a proper logger with redaction if needed; otherwise remove.
- console.log("CalldisplayAPI initialized with baseUrl:", this.baseUrl);
@@
- console.log("Making API request to:", url);
- console.log("Base URL:", this.baseUrl);
- console.log("API Payload:", JSON.stringify(apiPayload, null, 2));
@@
- console.log("Calldisplay API response:", JSON.stringify(data, null, 2));
@@
- console.log("Making queue API request to:", url);
- console.log("Queue API Payload:", JSON.stringify(apiPayload, null, 2));Also applies to: 126-129, 139-139, 189-191
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/api/CalldisplayAPI.ts around lines 87, 126-129,
139, and 189-191, remove all direct console.* calls that log request/response
data; replace them with calls to the project's logger (or remove entirely) and,
if logging is necessary, use structured logging with minimal fields and redact
sensitive/request/response payloads before logging. Update each
console.log/console.debug/console.error to use the shared logger instance (e.g.,
logger.info/warn/error) or delete the statement, and ensure any logged objects
are filtered/redacted to avoid leaking PII or full payloads.
| const apiPayload: any = { | ||
| serverTime: Math.floor(Date.now() / 1000), | ||
| organisation: { name: "" }, | ||
| }; | ||
|
|
||
| if ( | ||
| config.collections.scopelist && | ||
| config.collections.scopelist.length > 0 | ||
| ) { | ||
| // Convert scope IDs to scope objects like the old PHP code does | ||
| apiPayload.scopes = config.collections.scopelist.map((id) => ({ | ||
| id: id.toString(), | ||
| source: "dldb", | ||
| provider: { id: 0, name: "", source: "dldb" }, | ||
| shortName: "", | ||
| })); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
DRY: Extract payload construction from config to a helper.
Both methods duplicate the same mapping logic. Extracting reduces risk of divergence.
export class CalldisplayAPI {
private baseUrl: string;
constructor(baseUrl?: string) {
@@
}
+ private buildPayloadFromConfig(config: CalldisplayConfig) {
+ const payload: any = {
+ serverTime: Math.floor(Date.now() / 1000),
+ organisation: { name: "" },
+ };
+ if (config.collections.scopelist?.length) {
+ payload.scopes = config.collections.scopelist.map((id) => ({
+ id: id.toString(),
+ source: "dldb",
+ provider: { id: 0, name: "", source: "dldb" },
+ shortName: "",
+ }));
+ }
+ if (config.collections.clusterlist?.length) {
+ payload.clusters = config.collections.clusterlist.map((id) => ({
+ id: id.toString(),
+ source: "dldb",
+ provider: { id: 0, name: "", source: "dldb" },
+ shortName: "",
+ }));
+ }
+ return payload;
+ }
@@
- const apiPayload: any = {
- serverTime: Math.floor(Date.now() / 1000),
- organisation: { name: "" },
- };
-
- if (
- config.collections.scopelist &&
- config.collections.scopelist.length > 0
- ) {
- // Convert scope IDs to scope objects like the old PHP code does
- apiPayload.scopes = config.collections.scopelist.map((id) => ({
- id: id.toString(),
- source: "dldb",
- provider: { id: 0, name: "", source: "dldb" },
- shortName: "",
- }));
- }
-
- if (
- config.collections.clusterlist &&
- config.collections.clusterlist.length > 0
- ) {
- // Convert cluster IDs to cluster objects
- apiPayload.clusters = config.collections.clusterlist.map((id) => ({
- id: id.toString(),
- source: "dldb",
- provider: { id: 0, name: "", source: "dldb" },
- shortName: "",
- }));
- }
+ const apiPayload = this.buildPayloadFromConfig(config);
@@
- const apiPayload: any = {
- serverTime: Math.floor(Date.now() / 1000),
- organisation: { name: "" },
- };
- if (
- config.collections.scopelist &&
- config.collections.scopelist.length > 0
- ) {
- // Convert scope IDs to scope objects like the old PHP code does
- apiPayload.scopes = config.collections.scopelist.map((id) => ({
- id: id.toString(),
- source: "dldb",
- provider: { id: 0, name: "", source: "dldb" },
- shortName: "",
- }));
- }
- if (
- config.collections.clusterlist &&
- config.collections.clusterlist.length > 0
- ) {
- // Convert cluster IDs to cluster objects
- apiPayload.clusters = config.collections.clusterlist.map((id) => ({
- id: id.toString(),
- source: "dldb",
- provider: { id: 0, name: "", source: "dldb" },
- shortName: "",
- }));
- }
+ const apiPayload = this.buildPayloadFromConfig(config);Also applies to: 157-186
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/api/CalldisplayAPI.ts around lines 94-110 and
157-186, the same logic mapping config.collections.scopelist IDs to scope
objects is duplicated; extract that mapping into a single helper function (e.g.,
buildScopesFromConfig or getScopes) that accepts the
config.collections.scopelist (or config) and returns the array of scope objects,
ensure correct typing (return type and input type), replace both inline map
blocks with calls to the helper, and export or keep the helper in the same
module as appropriate to avoid duplication and keep behavior identical.
| async getConfig(): Promise<any> { | ||
| const response = await fetch(`${this.baseUrl}/buergeransicht/api/config/`); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch config: ${response.statusText}`); | ||
| } | ||
|
|
||
| return response.json(); | ||
| } |
There was a problem hiding this comment.
Strongly type getConfig() and return the data payload consistently.
Align with getCalldisplay/getScope behavior and the Config interface.
- async getConfig(): Promise<any> {
- const response = await fetch(`${this.baseUrl}/buergeransicht/api/config/`);
-
- if (!response.ok) {
- throw new Error(`Failed to fetch config: ${response.statusText}`);
- }
-
- return response.json();
- }
+ async getConfig(): Promise<Config> {
+ const response = await fetch(`${this.baseUrl}/buergeransicht/api/config/`);
+ if (!response.ok) {
+ throw new Error(`Failed to fetch config: ${response.statusText}`);
+ }
+ const data = await response.json();
+ return (data.data ?? data) as Config;
+ }📝 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.
| async getConfig(): Promise<any> { | |
| const response = await fetch(`${this.baseUrl}/buergeransicht/api/config/`); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch config: ${response.statusText}`); | |
| } | |
| return response.json(); | |
| } | |
| async getConfig(): Promise<Config> { | |
| const response = await fetch(`${this.baseUrl}/buergeransicht/api/config/`); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch config: ${response.statusText}`); | |
| } | |
| const data = await response.json(); | |
| return (data.data ?? data) as Config; | |
| } |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/api/CalldisplayAPI.ts around lines 243-251,
getConfig currently returns Promise<any> and raw JSON; update it to return
Promise<Config> (import/ensure the Config interface is available), await and
parse response.json() into a Config-typed object, and return that typed payload;
keep the existing non-OK error check but throw a descriptive Error with
response.status and statusText, and ensure the method signature and callers are
updated to expect and use the Config type so behavior matches
getCalldisplay/getScope.
| async function loadData() { | ||
| try { | ||
| loading.value = true; | ||
| error.value = null; | ||
| // Check if we have scope parameters - if not, don't make API calls | ||
| if ( | ||
| !urlConfig.value.collections.scopelist || | ||
| urlConfig.value.collections.scopelist.length === 0 | ||
| ) { | ||
| console.log("No scope parameters provided, skipping API calls"); | ||
| calldisplay.value = null; | ||
| queue.value = null; | ||
| scope.value = null; | ||
| loading.value = false; | ||
| return; | ||
| } | ||
| // Load calldisplay configuration | ||
| console.log( | ||
| "URL Config being passed to API:", | ||
| JSON.stringify(urlConfig.value, null, 2), | ||
| ); | ||
| calldisplay.value = await api.getCalldisplay(urlConfig.value); | ||
| // Load queue data | ||
| const statusList = urlConfig.value.queue?.status || ["called", "pickup"]; | ||
| queue.value = await api.getQueue(urlConfig.value, statusList); | ||
| // Load scope if only one scope | ||
| if (calldisplay.value.scopes && calldisplay.value.scopes.length === 1) { | ||
| scope.value = await api.getScope(calldisplay.value.scopes[0].id); | ||
| } | ||
| // Load config (optional - don't block on error) | ||
| try { | ||
| config.value = await api.getConfig(); | ||
| } catch (err) { | ||
| console.warn("Config API error:", err); | ||
| // Use default config if API fails | ||
| config.value = { | ||
| template: "default_counter", | ||
| display: "1", | ||
| refreshInterval: 5000, | ||
| organisation: { | ||
| name: "Kreisverwaltungsreferat", | ||
| id: 4, | ||
| }, | ||
| preferences: { | ||
| ticketPrinterProtectionEnabled: true, | ||
| }, | ||
| }; | ||
| } | ||
| } catch (err) { | ||
| error.value = err instanceof Error ? err.message : "Unknown error occurred"; | ||
| console.error("Error loading calldisplay data:", err); | ||
| } finally { | ||
| loading.value = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate fetch logic between loadData and refreshData—extract a shared helper.
Keeps functions small (<50 lines) and reduces maintenance.
If you want, I can push a small refactor that extracts “fetchAll()” returning { calldisplay, queue, scope, config } and reuse it in both paths.
Also applies to: 163-217
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/CalldisplayApp.vue around lines 101-160 (and
similarly 163-217), duplicate fetching logic exists in loadData and refreshData;
extract a shared async helper (e.g., fetchAll) that accepts urlConfig and
returns an object { calldisplay, queue, scope, config } handling the same
checks, API calls, config fallback, and error propagation, then replace the body
of loadData and refreshData with calls to this helper to assign returned values
and manage loading/error state; ensure the helper does not mutate component refs
directly (return values) so callers can set refs and keep behavior identical.
| loading: boolean; | ||
| error: string | null; | ||
| calldisplay: CalldisplayEntity | null; | ||
| queue: QueueList | null; |
There was a problem hiding this comment.
Fix CalldisplayState.queue type to match actual usage (array).
Currently QueueList | null, but the app uses QueueList[].
- queue: QueueList | null;
+ queue: QueueList[] | null;📝 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.
| queue: QueueList | null; | |
| queue: QueueList[] | null; |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/types/index.ts around line 41, the
CalldisplayState.queue property is typed as QueueList | null but the code
expects an array (QueueList[]). Change the type to QueueList[] | null (or simply
QueueList[] if null is not needed) so the type matches actual usage; update any
imports/consumers if they rely on the old singular type to use array operations
safely.
| const config: Config = { | ||
| ADD_TAGS: ["svg", "symbol", "defs", "use", "path", "g"], | ||
| ADD_ATTR: [ | ||
| "viewBox", | ||
| "xmlns", | ||
| "fill", | ||
| "stroke", | ||
| "d", | ||
| "width", | ||
| "height", | ||
| "focusable", | ||
| "xlink:href", | ||
| ], | ||
| FORBID_TAGS: ["img"], | ||
| }; |
There was a problem hiding this comment.
Harden SVG URI handling; restrict xlink:href/href on to internal IDs only.
Prevents external/javascript: links via SVG use. Enable SVG profile and tighten URI regex.
const config: Config = {
- ADD_TAGS: ["svg", "symbol", "defs", "use", "path", "g"],
+ ADD_TAGS: ["svg", "symbol", "defs", "use", "path", "g"],
ADD_ATTR: [
"viewBox",
"xmlns",
"fill",
"stroke",
"d",
"width",
"height",
"focusable",
"xlink:href",
],
FORBID_TAGS: ["img"],
+ // Allow only safe protocols and internal anchors
+ ALLOWED_URI_REGEXP: /^(#|https?:|mailto:|tel:)/i,
+ USE_PROFILES: { html: true, svg: true }
};
+
+// Drop any xlink:href/href on <use> that is not an internal fragment (#id)
+DOMPurify.addHook("uponSanitizeAttribute", (node, data) => {
+ const tag = (node as Element).tagName?.toLowerCase?.();
+ const isUse = tag === "use";
+ const isHref =
+ data.attrName === "xlink:href" || (data.attrName === "href" && isUse);
+ if (isUse && isHref && data.attrValue && !data.attrValue.startsWith("#")) {
+ data.keepAttr = false;
+ }
+});📝 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 config: Config = { | |
| ADD_TAGS: ["svg", "symbol", "defs", "use", "path", "g"], | |
| ADD_ATTR: [ | |
| "viewBox", | |
| "xmlns", | |
| "fill", | |
| "stroke", | |
| "d", | |
| "width", | |
| "height", | |
| "focusable", | |
| "xlink:href", | |
| ], | |
| FORBID_TAGS: ["img"], | |
| }; | |
| const config: Config = { | |
| ADD_TAGS: ["svg", "symbol", "defs", "use", "path", "g"], | |
| ADD_ATTR: [ | |
| "viewBox", | |
| "xmlns", | |
| "fill", | |
| "stroke", | |
| "d", | |
| "width", | |
| "height", | |
| "focusable", | |
| "xlink:href", | |
| ], | |
| FORBID_TAGS: ["img"], | |
| // Allow only safe protocols and internal anchors | |
| ALLOWED_URI_REGEXP: /^(#|https?:|mailto:|tel:)/i, | |
| USE_PROFILES: { html: true, svg: true }, | |
| }; | |
| // Drop any xlink:href/href on <use> that is not an internal fragment (#id) | |
| DOMPurify.addHook("uponSanitizeAttribute", (node, data) => { | |
| const tag = (node as Element).tagName?.toLowerCase?.(); | |
| const isUse = tag === "use"; | |
| const isHref = | |
| data.attrName === "xlink:href" || (data.attrName === "href" && isUse); | |
| if (isUse && isHref && data.attrValue && !data.attrValue.startsWith("#")) { | |
| data.keepAttr = false; | |
| } | |
| }); |
🤖 Prompt for AI Agents
zmscitizencalldisplay/src/utils/sanitizeHtml.ts around lines 6-20: current
config adds SVG tags/attrs but does not enable the SVG profile or restrict URIs
on <use>, leaving xlink:href/href vulnerable to external/javascript: links.
Update the DOMPurify config to enable the SVG profile and tighten URI handling
by adding an ALLOWED_URI_REGEXP (or equivalent DOMPurify option) that only
permits internal ID references for <use> (values starting with '#' and matching
a safe ID pattern like /^[#][A-Za-z][\w:-]*$/), and/or add an
attribute-sanitizing hook that rejects any xlink:href or href on a <use> element
unless it is a single internal ID; keep other allowed SVG attributes unchanged
and still forbid <img>.
| .map((id) => parseInt(id.trim())) | ||
| .filter((id) => !isNaN(id)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Explicit radix and empty-slice filtering.
Use base 10 parses and drop empty tokens.
- .map((id) => parseInt(id.trim()))
+ .map((id) => parseInt(id.trim(), 10))
.filter((id) => !isNaN(id));- .map((id) => parseInt(id.trim()))
+ .map((id) => parseInt(id.trim(), 10))
.filter((id) => !isNaN(id));- const displayNum = parseInt(display);
+ const displayNum = parseInt(display, 10);- const displayNum = parseInt(display);
+ const displayNum = parseInt(display, 10);Also applies to: 28-30, 108-109
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/utils/urlParams.ts around lines 21-23 (and also at
28-30 and 108-109), the code parses ID tokens with parseInt without specifying a
radix and does not guard against empty tokens; update the token processing to
first filter out empty/whitespace tokens (e.g., token.trim() !== "") and then
call parseInt(token.trim(), 10), finally filtering out NaN results so only valid
base-10 integers remain.
| const displayNum = parseInt(display); | ||
| if (!isNaN(displayNum)) { | ||
| config.display = displayNum; | ||
| } |
There was a problem hiding this comment.
Type mismatch: display parsed as number but CalldisplayConfig.display is string in API.
This will cause TS friction/inconsistency. Align the API type to number or keep display as string everywhere. Recommendation: make display a number in the API.
Apply safer parsing (base 10) and keep number:
- if (display) {
- const displayNum = parseInt(display);
+ if (display) {
+ const displayNum = parseInt(display, 10);
if (!isNaN(displayNum)) {
config.display = displayNum;
}
}And update the API type (in src/api/CalldisplayAPI.ts):
// change
display?: number;🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/utils/urlParams.ts around lines 41 to 44 the code
parses display with parseInt but doesn't specify radix and the project API type
still declares CalldisplayConfig.display as a string, causing a type mismatch;
change parseInt to parseInt(display, 10) (or Number(display)) to explicitly
parse a base-10 number and assign the numeric value to config.display, and then
update the API type in src/api/CalldisplayAPI.ts to declare display?: number so
the types align across the codebase.
| plugins: [ | ||
| vue({ | ||
| features: { | ||
| customElement: true | ||
| } | ||
| }) | ||
| ], |
There was a problem hiding this comment.
Fix Vue plugin option; use customElement at the top level (features is not a valid key).
Current key is likely ignored.
plugins: [
- vue({
- features: {
- customElement: true
- }
- })
+ vue({
+ customElement: true
+ })
],📝 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.
| plugins: [ | |
| vue({ | |
| features: { | |
| customElement: true | |
| } | |
| }) | |
| ], | |
| plugins: [ | |
| vue({ | |
| customElement: true | |
| }) | |
| ], |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/vite.config.ts around lines 10 to 16, the vue plugin is
using an invalid "features" key so customElement is being ignored; move the
option to the plugin's top-level by replacing features: { customElement: true }
with customElement: true inside the vue(...) call so the plugin receives the
correct option.
…ue.js calldisplay - Add audio manager with HTML5 audio support and user interaction detection - Implement visual blinking effect for new appointments - Create SoundPermissionModal component with browser-specific settings guidance - Add comprehensive test suite with vitest configuration - Hide audio player element from UI while maintaining functionality - Support Chrome, Firefox, Edge, Safari, and IE browser detection - Include copy-to-clipboard and direct settings URL opening features
- Add Dockerfile for nginx-based container deployment - Copy nginx configuration files from zmscitizenview - Enable GitHub Actions Docker build process
ThomasAFink
changed the title
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (3)
zmscitizencalldisplay/src/components/CalldisplayView.vue (3)
63-66: External link hardening: add rel="noopener noreferrer".- <a :href="$t('calldisplay.errorSupportLink')" target="_blank">{{ + <a + :href="$t('calldisplay.errorSupportLink')" + target="_blank" + rel="noopener noreferrer" + >{{ $t("calldisplay.errorSupportLink") }}</a>
429-435: Avoid O(n) filtering per cell render; cache called items.const scopeShortName = computed(() => { if (props.calldisplay?.scopes && props.calldisplay.scopes.length === 1) { return props.calldisplay.scopes[0].shortName || ""; } return ""; }); + +// Cache items with status "called" to avoid repeated filtering in cell renders +const calledItems = computed(() => + (props.queue ?? []).filter((item) => item.status === "called"), +); @@ const getQueueItemForRow = (row: number, column: number) => { if (!props.queue || !Array.isArray(props.queue)) return null; - // Only show called items (status: 'called') - const calledItems = props.queue.filter((item) => item.status === "called"); - // Calculate index: (row - 1) * 2 + (column - 1) const index = (row - 1) * 2 + (column - 1); - return calledItems[index] || null; + return calledItems.value[index] ?? null; };Also applies to: 536-546
717-757: Fix status CSS selectors: use compound selectors, not descendant.-.status-waiting .queue-number, -.status-waiting .queue-destination { +.queue-number.status-waiting, +.queue-destination.status-waiting { @@ -.status-called .queue-number, -.status-called .queue-destination { +.queue-number.status-called, +.queue-destination.status-called { @@ -.status-confirmed .queue-number, -.status-confirmed .queue-destination { +.queue-number.status-confirmed, +.queue-destination.status-confirmed { @@ -.status-queued .queue-number, -.status-queued .queue-destination { +.queue-number.status-queued, +.queue-destination.status-queued { @@ -.status-pending .queue-number, -.status-pending .queue-destination { +.queue-number.status-pending, +.queue-destination.status-pending { @@ -.status-pickup .queue-number, -.status-pickup .queue-destination { +.queue-number.status-pickup, +.queue-destination.status-pickup {
🧹 Nitpick comments (13)
zmscitizencalldisplay/vitest.config.ts (1)
18-21: Optional: drop globals to avoid polluting test scope.You already import
describe/it/expect;globals: trueis unnecessary. Consider removing for cleaner test scope.- test: { - environment: "jsdom", - globals: true, - setupFiles: ["./tests/setup.ts"] - } + test: { + environment: "jsdom", + setupFiles: ["./tests/setup.ts"] + }zmscitizencalldisplay/tests/utils/sanitizeHtml.spec.ts (3)
3-5: Remove spurious ts-expect-error.This import isn’t a Vue SFC; the directive is misleading noise.
-// @ts-expect-error: Vue SFC import for test -import { sanitizeHtml } from "@/utils/sanitizeHtml"; +import { sanitizeHtml } from "@/utils/sanitizeHtml";
41-45: Make URL attribute assertions robust to quote styles.DOMPurify may emit single or double quotes; match both.
- expect(clean).not.toMatch(/href=\"javascript:/i); - expect(clean).not.toMatch(/href=\"data:/i); - expect(clean).toMatch(/href=\"\/relative\"/); - expect(clean).toMatch(/href=\"https:\/\/example.com\"/); + expect(clean).not.toMatch(/href=(['"])javascript:/i); + expect(clean).not.toMatch(/href=(['"])data:/i); + expect(clean).toMatch(/href=(['"])\/relative\1/); + expect(clean).toMatch(/href=(['"])https:\/\/example\.com\1/);
6-77: Optional: many asserts per test—consider splitting for clarity.Guideline suggests one assert per test. Consider smaller tests (e.g., separate cases for tags, attributes, URLs) for easier failure diagnostics.
If you’d like, I can propose a split into sub-specs with one assertion each.
zmscitizencalldisplay/src/utils/blinkEffect.ts (2)
33-41: Prevent double cleanup: clear the safety timeout when interval finishes.Currently both the interval branch and the safety timeout perform cleanup; add a handle to cancel the timeout when the interval exits to avoid redundant work and late toggles.
const blinkInterval = setInterval(() => { if (blinkCount >= opts.times * 2) { // Stop blinking and ensure element is visible clearInterval(blinkInterval); + clearTimeout(safetyTimeout); element.classList.remove(opts.className); element.style.visibility = "visible"; element.style.opacity = "1"; return; } @@ // Safety timeout to ensure blinking stops - setTimeout(() => { + const safetyTimeout = setTimeout(() => { clearInterval(blinkInterval); element.classList.remove(opts.className); element.style.visibility = "visible"; element.style.opacity = "1"; }, opts.duration);Also applies to: 51-57
43-47: Reduce layout shift: prefer opacity-only changes over visibility toggles.Toggling visibility hides the element and can cause layout jumps. Keep it in flow and blink via opacity only.
- element.style.visibility = isVisible ? "visible" : "hidden"; - element.style.opacity = isVisible ? "1" : "0.3"; + element.style.opacity = isVisible ? "1" : "0.3";zmscitizencalldisplay/src/components/CalldisplayView.vue (2)
436-436: Use browser-appropriate timer type.NodeJS.Timeout is incorrect in the browser; use number.
-let timeInterval: NodeJS.Timeout | null = null; +let timeInterval: number | null = null;
613-620: Remove duplicate keyframes.spin is defined twice; keep one to avoid confusion.
-@keyframes spin { - from { - transform: rotate(0deg); - } - to { - transform: rotate(360deg); - } -}zmscitizencalldisplay/src/components/SoundPermissionModal.vue (1)
427-456: Remove duplicated global styles for the modal.Scoped styles already define .modal-*; this duplicate global block is unnecessary.
-<style> -/* Global styles to ensure modal works properly */ -.modal-overlay { - position: fixed !important; - top: 0 !important; - left: 0 !important; - width: 100vw !important; - height: 100vh !important; - background: rgba(0, 0, 0, 0.5) !important; - z-index: 999999 !important; - display: flex !important; - align-items: center !important; - justify-content: center !important; - margin: 0 !important; - padding: 0 !important; -} - -.modal-content { - background: white !important; - padding: 30px !important; - border-radius: 12px !important; - max-width: 600px !important; - width: 90% !important; - box-shadow: 0 8px 24px rgba(0, 0, 0, 0.3) !important; - font-family: Arial, sans-serif !important; - position: relative !important; - z-index: 1000000 !important; - margin: 0 !important; -} -</style>zmscitizencalldisplay/Dockerfile (2)
2-2: Base image pin: reproducible but track security updates.Digest pinning is good. Ensure Renovate/Dependabot (or similar) watches this tag+digest for CVEs and bumps.
11-11: Reconsider overriding CMD.The UBI nginx image’s default entrypoint may perform init tasks (env wiring, include paths). Overriding with nginx -g could skip them. If not required, prefer the image’s default CMD/ENTRYPOINT.
-# Start the web server -CMD nginx -g "daemon off;" +# Use base image default CMD (recommended), or document why override is needed +# CMD nginx -g "daemon off;"zmscitizencalldisplay/docker/nginx/compression.conf (1)
1-7: Tune gzip for CPU vs. payoff; trim legacy types.Level 9 is CPU‑heavy for marginal size gains; also both text/javascript and application/x-javascript are legacy.
gzip on; gzip_http_version 1.1; gzip_min_length 1100; gzip_vary on; gzip_proxied expired no-cache no-store private auth; -gzip_types text/plain text/css application/json application/javascript application/x-javascript text/xml application/xml application/xml+rss text/javascript; -gzip_comp_level 9; +gzip_types text/plain text/css application/json application/javascript text/xml application/xml application/xml+rss; +gzip_comp_level 6;zmscitizencalldisplay/docker/nginx/files.conf (1)
7-10: Strengthen revalidation for loader scripts; add ACAO.If loader-*.js is unversioned, force revalidation and allow cross-origin if needed.
location ~ ^/loader-.*\.js$ { - add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; - add_header Cache-Control 'no-cache'; + add_header 'Access-Control-Allow-Origin' '*' always; + add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; + add_header Cache-Control 'no-cache, must-revalidate'; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
zmscitizencalldisplay/Dockerfile(1 hunks)zmscitizencalldisplay/docker/nginx/compression.conf(1 hunks)zmscitizencalldisplay/docker/nginx/files.conf(1 hunks)zmscitizencalldisplay/docker/nginx/health.conf(1 hunks)zmscitizencalldisplay/docker/nginx/sbom.conf(1 hunks)zmscitizencalldisplay/src/components/CalldisplayView.vue(1 hunks)zmscitizencalldisplay/src/components/SoundPermissionModal.vue(1 hunks)zmscitizencalldisplay/src/styles/main.scss(1 hunks)zmscitizencalldisplay/src/utils/audioManager.ts(1 hunks)zmscitizencalldisplay/src/utils/blinkEffect.ts(1 hunks)zmscitizencalldisplay/tests/setup.ts(1 hunks)zmscitizencalldisplay/tests/utils/sanitizeHtml.spec.ts(1 hunks)zmscitizencalldisplay/vitest.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- zmscitizencalldisplay/src/styles/main.scss
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the following Clean Code guidelines to all files, as summarized by wojteklu's Clean Code gist:Code is clean if it can be understood easily – by everyone on the team. Clean code can be read and enhanced by a developer other than its original author. With understandability comes readability, changeability, extensibility and maintainability.
General rules
- Follow standard conventions.
- Keep it simple stupid. Simpler is always better. Reduce complexity as much as possible.
- Boy scout rule. Leave the campground cleaner than you found it.
- Always find root cause. Always look for the root cause of a problem.
Design rules
- Keep configurable data at high levels.
- Prefer polymorphism to if/else or switch/case.
- Separate multi-threading code.
- Prevent over-configurability.
- Use dependency injection.
- Follow Law of Demeter. A class should know only its direct dependencies.
Understandability tips
- Be consistent. If you do something a certain way, do all similar things in the same way.
- Use explanatory variables.
- Encapsulate boundary conditions. Boundary conditions are hard to keep track of. Put the processing for them in one place.
- Prefer dedicated value objects to primitive type.
- Avoid logical dependency. Don't write methods which works correctly depending on something else in the same class.
- Avoid negative conditionals.
Names rules
- Choose descriptive and unambiguous names.
- Make meaningful distinction.
- Use pronounceable names.
- Use searchable names.
- Replace magic numbers with named constants.
- Avoid encodings. Don't append prefixes or type information.
Functions rules
- Small.
- Do one thing.
- Use descriptive names.
- Prefer fewer arguments.
- Have no side effects.
- Don't use flag arguments. Split method into several independent methods that can b...
Files:
zmscitizencalldisplay/tests/setup.tszmscitizencalldisplay/Dockerfilezmscitizencalldisplay/docker/nginx/compression.confzmscitizencalldisplay/docker/nginx/sbom.confzmscitizencalldisplay/tests/utils/sanitizeHtml.spec.tszmscitizencalldisplay/docker/nginx/health.confzmscitizencalldisplay/docker/nginx/files.confzmscitizencalldisplay/src/utils/blinkEffect.tszmscitizencalldisplay/src/utils/audioManager.tszmscitizencalldisplay/vitest.config.tszmscitizencalldisplay/src/components/SoundPermissionModal.vuezmscitizencalldisplay/src/components/CalldisplayView.vue
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{js,jsx,ts,tsx}: Flag any usage of console.log() as it should be replaced with proper logging or removed:
- For development: console.debug()
- For production: Remove console.log() statements or use structured logging
- For errors: Use error console.error()
Example replacement:
// Instead of: console.log('User data:', userData); // Use: console.debug('Processing user data', { userData }); // or for development only: Remove the console.log entirelyFlag specific logging violations:
- console.log(), console.debug(), console.warn() usage (except console.error in catch blocks)
- Any debug output that should use proper logging frameworks
Example replacements:
// Instead of: console.log('User data:', userData); console.debug('Processing...'); // Use: // Remove console.log entirely or use proper logging // Only console.error in catch blocks is acceptable try { processData(userData); } catch (error) { console.error('Processing failed:', error); }Flag JavaScript security and UX issues:
- alert(), confirm(), prompt() usage (poor UX)
- eval() usage (security risk)
- innerHTML with user input (XSS risk)
- Unfiltered user input in DOM manipulation
Example replacements:
// Instead of: alert('Error occurred'); eval(userInput); // Use: // Use proper error handling and UI components this.showErrorNotification('Error occurred'); // Avoid eval() entirelyFlag TODO/FIXME comments that indicate technical debt:
- TODO comments without clear action items
- FIXME comments indicating broken functionality
- HACK comments indicating temporary workarounds
- XXX comments indicating problematic code
These should be converted to proper issues or addressed:
// Instead of: // TODO: fix this later // FIXME: this is broken // Use: // Create proper issue and reference it // @see Issue #123: Refactor validation logicFlag code complexity issues:
- Functions longer than 50 lines
- Deep nesting (...
Files:
zmscitizencalldisplay/tests/setup.tszmscitizencalldisplay/tests/utils/sanitizeHtml.spec.tszmscitizencalldisplay/src/utils/blinkEffect.tszmscitizencalldisplay/src/utils/audioManager.tszmscitizencalldisplay/vitest.config.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: call-php-unit-tests / zmsdb-test
🔇 Additional comments (6)
zmscitizencalldisplay/src/utils/audioManager.ts (1)
27-28: Confirmed — #ring audio element is present.Found in zmscitizencalldisplay/index.html (line 30) and zmscalldisplay/templates/element/ring.twig (line 1).
zmscitizencalldisplay/docker/nginx/sbom.conf (2)
1-3: Good: direct file access is blocked.Returning 404 for /application.cdx.json avoids accidental exposure of the raw SBOM file.
11-11: Ensure application.cdx.json is present at /opt/app-root/src in the built image (or update the nginx alias).No application.cdx.json was found in the repository; zmscitizencalldisplay/docker/nginx/sbom.conf maps /actuator/sbom/application to alias /opt/app-root/src/application.cdx.json and /application.cdx.json returns 404. Action: either copy the SBOM into the image at /opt/app-root/src (e.g., add to the Dockerfile:
COPY application.cdx.json /opt/app-root/src/) or change the nginx alias to the SBOM’s actual build/output path.zmscitizencalldisplay/docker/nginx/files.conf (1)
1-10: Verify asset path correctness.
vite.config.ts (zmscitizencalldisplay/vite.config.ts) sets assetsDir: "src" and base: './' — nginx location /src/ matches that. zmscitizencalldisplay/dist is absent in the repo so the built layout couldn't be inspected; confirm your build/CI emits assets under /src/ (or update Nginx if assetsDir/base differ).zmscitizencalldisplay/Dockerfile (2)
5-5: Confirm CI build context and that dist is produced before image build.
- zmscitizencalldisplay/Dockerfile (line 5):
COPY dist .— zmscitizencalldisplay/dist is not present in the repo.- .github/workflows/maven-node-build.yaml (line 20) has
app-path: zmscitizencalldisplay; .github/workflows/php-build-images.yaml (line 46) runsdocker build . \— I did not find an explicit--file/-forcontext:pointing to zmscitizencalldisplay.Ensure the CI step that runs before docker build (a) runs from or sets the build context to zmscitizencalldisplay (or passes
--file/--contextappropriately) and (b) produceszmscitizencalldisplay/distbefore the image build.
8-8: Confirm base image defines NGINX_DEFAULT_CONF_PATH or set an explicit path in the Dockerfilezmscitizencalldisplay/Dockerfile:8 and zmscitizenview/Dockerfile:8 copy docker/nginx/*.conf into "${NGINX_DEFAULT_CONF_PATH}"; NGINX_DEFAULT_CONF_PATH is not defined in the repo — verify the FROM image exports that env var and nginx loads that directory, or add ENV NGINX_DEFAULT_CONF_PATH=/etc/nginx/conf.d (or change the COPY to that explicit path) so configs are installed where nginx expects them.
| location /src/ { | ||
| expires 365d; | ||
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | ||
| add_header Cache-Control "public, no-transform"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid duplicate Cache-Control; add immutable for hashed assets; explicit ACAO.
expires sets Cache-Control: max-age. Adding another Cache-Control creates duplicate headers. Prefer a single header with all directives.
-location /src/ {
- expires 365d;
- add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always;
- add_header Cache-Control "public, no-transform";
-}
+location /src/ {
+ add_header 'Access-Control-Allow-Origin' '*' always;
+ add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always;
+ # If filenames are content-hashed (recommended), mark immutable:
+ add_header Cache-Control "public, max-age=31536000, no-transform, immutable";
+ # Or, if not hashed, keep a shorter max-age and drop immutable.
+}📝 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.
| location /src/ { | |
| expires 365d; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | |
| add_header Cache-Control "public, no-transform"; | |
| } | |
| location /src/ { | |
| add_header 'Access-Control-Allow-Origin' '*' always; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | |
| # If filenames are content-hashed (recommended), mark immutable: | |
| add_header Cache-Control "public, max-age=31536000, no-transform, immutable"; | |
| # Or, if not hashed, keep a shorter max-age and drop immutable. | |
| } |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/docker/nginx/files.conf around lines 1 to 5, do not
generate duplicate Cache-Control headers by removing the expires directive and
instead emit a single Cache-Control header that includes max-age and immutable
for hashed assets (e.g. "public, max-age=<seconds>, immutable"); also replace
the generic Access-Control-Allow-Methods with explicit
Access-Control-Allow-Origin and keep methods if needed (use add_header
'Access-Control-Allow-Origin' '*' and add_header 'Access-Control-Allow-Methods'
'GET, OPTIONS' always;), and ensure all add_header directives include the always
flag so the headers are sent for all responses.
| location /actuator/health { | ||
| access_log off; | ||
| add_header 'Content-Type' 'application/json'; | ||
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | ||
| add_header Cache-Control 'no-cache' always; | ||
| return 200 '{"status":"UP"}'; | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use default_type for JSON; consider explicit CORS.
Using add_header for Content-Type is unconventional here; set default_type. Add ACAO if this endpoint is read cross-origin.
location /actuator/health {
access_log off;
- add_header 'Content-Type' 'application/json';
+ default_type application/json;
+ add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always;
add_header Cache-Control 'no-cache' always;
return 200 '{"status":"UP"}';
}📝 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.
| location /actuator/health { | |
| access_log off; | |
| add_header 'Content-Type' 'application/json'; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | |
| add_header Cache-Control 'no-cache' always; | |
| return 200 '{"status":"UP"}'; | |
| } | |
| location /actuator/health { | |
| access_log off; | |
| default_type application/json; | |
| add_header 'Access-Control-Allow-Origin' '*' always; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | |
| add_header Cache-Control 'no-cache' always; | |
| return 200 '{"status":"UP"}'; | |
| } |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/docker/nginx/health.conf around lines 1-7, replace the
add_header 'Content-Type' line with a directive that sets default_type
application/json for the block, remove the Content-Type add_header, and add an
explicit Access-Control-Allow-Origin header (e.g. '*' or the allowed origin)
with the always flag; keep the existing Access-Control-Allow-Methods and
Cache-Control headers and the 200 JSON return as-is.
| location /actuator/sbom/application { | ||
| access_log off; | ||
| add_header 'Content-Type' 'application/json'; | ||
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | ||
| add_header Cache-Control 'no-store, max-age=0'; | ||
| add_header 'X-Content-Type-Options' 'nosniff' always; | ||
| alias /opt/app-root/src/application.cdx.json; | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restrict public SBOM exposure (security posture).
Exposing SBOM at /actuator/sbom/application can leak dependency intel. Gate it (IP allowlist or deny-by-default) and make CORS explicit. Also prefer default_type over add_header for Content-Type.
Apply:
location /actuator/sbom/application {
- access_log off;
- add_header 'Content-Type' 'application/json';
+ access_log off;
+ default_type application/json;
+ # CORS (adjust as needed)
+ add_header 'Access-Control-Allow-Origin' '*' always;
+ add_header 'Access-Control-Allow-Headers' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always;
add_header Cache-Control 'no-store, max-age=0';
add_header 'X-Content-Type-Options' 'nosniff' always;
+ # Tighten access (adjust allow list per env)
+ allow 127.0.0.1;
+ deny all;
alias /opt/app-root/src/application.cdx.json;
}📝 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.
| location /actuator/sbom/application { | |
| access_log off; | |
| add_header 'Content-Type' 'application/json'; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | |
| add_header Cache-Control 'no-store, max-age=0'; | |
| add_header 'X-Content-Type-Options' 'nosniff' always; | |
| alias /opt/app-root/src/application.cdx.json; | |
| } | |
| location /actuator/sbom/application { | |
| access_log off; | |
| default_type application/json; | |
| # CORS (adjust as needed) | |
| add_header 'Access-Control-Allow-Origin' '*' always; | |
| add_header 'Access-Control-Allow-Headers' '*' always; | |
| add_header 'Access-Control-Allow-Methods' 'GET, OPTIONS' always; | |
| add_header Cache-Control 'no-store, max-age=0'; | |
| add_header 'X-Content-Type-Options' 'nosniff' always; | |
| # Tighten access (adjust allow list per env) | |
| allow 127.0.0.1; | |
| deny all; | |
| alias /opt/app-root/src/application.cdx.json; | |
| } |
| <div | ||
| v-if="!audioEnabled" | ||
| @click="showSoundPermissionModal = true" | ||
| style=" | ||
| background: #ffc107; | ||
| color: #000; | ||
| padding: 5px 10px; | ||
| border-radius: 3px; | ||
| font-size: 12px; | ||
| margin-right: 10px; | ||
| cursor: pointer; | ||
| " | ||
| title="Click to open Sound permissions for this domain" | ||
| > | ||
| 🔊 Click to enable Sound | ||
| </div> | ||
| <span v-html="currentDateTime"></span> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a button for the sound-permission CTA (a11y/semantics).
Clickable divs hurt accessibility. Use a button with type and aria-label.
- <div
- v-if="!audioEnabled"
- @click="showSoundPermissionModal = true"
- style="
+ <button
+ v-if="!audioEnabled"
+ type="button"
+ @click="showSoundPermissionModal = true"
+ style="
background: #ffc107;
color: #000;
padding: 5px 10px;
border-radius: 3px;
font-size: 12px;
margin-right: 10px;
cursor: pointer;
"
- title="Click to open Sound permissions for this domain"
+ title="Click to open Sound permissions for this domain"
+ aria-label="Enable sound"
>
🔊 Click to enable Sound
- </div>
+ </button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| v-if="!audioEnabled" | |
| @click="showSoundPermissionModal = true" | |
| style=" | |
| background: #ffc107; | |
| color: #000; | |
| padding: 5px 10px; | |
| border-radius: 3px; | |
| font-size: 12px; | |
| margin-right: 10px; | |
| cursor: pointer; | |
| " | |
| title="Click to open Sound permissions for this domain" | |
| > | |
| 🔊 Click to enable Sound | |
| </div> | |
| <span v-html="currentDateTime"></span> | |
| <button | |
| v-if="!audioEnabled" | |
| type="button" | |
| @click="showSoundPermissionModal = true" | |
| style=" | |
| background: #ffc107; | |
| color: #000; | |
| padding: 5px 10px; | |
| border-radius: 3px; | |
| font-size: 12px; | |
| margin-right: 10px; | |
| cursor: pointer; | |
| " | |
| title="Click to open Sound permissions for this domain" | |
| aria-label="Enable sound" | |
| > | |
| 🔊 Click to enable Sound | |
| </button> | |
| <span v-html="currentDateTime"></span> |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/components/CalldisplayView.vue around lines 31 to
47, the clickable div used as the sound-permission CTA is inaccessible; replace
it with a semantic <button type="button"> element, keep the v-if and @click
handler, add an aria-label (e.g., "Enable sound permissions for this domain")
and preserve the visual styling via the same inline styles or preferably a CSS
class; remove the click-only behavior so the button inherently supports keyboard
activation and update/remove the title attribute if redundant.
| <div v-if="loading" class="loading"> | ||
| <div class="spinner"></div> | ||
| <p>{{ $t("calldisplay.loading") }}</p> | ||
| </div> | ||
|
|
There was a problem hiding this comment.
Fix loading/normal-content gating: content renders while loading.
The “normal” content uses a separate v-if and can render alongside the loading state. Chain it with v-else to show only one state.
<div v-if="loading" class="loading">
<div class="spinner"></div>
<p>{{ $t("calldisplay.loading") }}</p>
</div>
- <div v-else-if="error" class="error-container">
+ <div v-else-if="error" class="error-container">
@@
- <div v-if="!error">
+ <div v-else>Also applies to: 56-68, 70-70
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/components/CalldisplayView.vue around lines 51-55
(and similarly for 56-68 and line 70), the loading block uses v-if but the
normal content uses a separate v-if, allowing both to render simultaneously;
change the normal-content v-if to v-else (or v-else-if) so the loading block and
the main content are mutually exclusive, ensuring only the loading state or the
normal content is rendered at a time.
| console.log( | ||
| "Audio Manager - Sound permission request failed:", | ||
| error instanceof Error ? error.message : String(error), | ||
| ); | ||
| console.log( | ||
| "Audio Manager - This is expected - Sound permission requires user interaction", | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Use console.error in catch blocks; drop extra noise.
Consolidate catch logging to a single error line.
} catch (error) {
- console.log(
- "Audio Manager - Sound permission request failed:",
- error instanceof Error ? error.message : String(error),
- );
- console.log(
- "Audio Manager - This is expected - Sound permission requires user interaction",
- );
+ console.error(
+ "Audio Manager - Sound permission request failed:",
+ error instanceof Error ? error.message : String(error),
+ );
}📝 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.
| console.log( | |
| "Audio Manager - Sound permission request failed:", | |
| error instanceof Error ? error.message : String(error), | |
| ); | |
| console.log( | |
| "Audio Manager - This is expected - Sound permission requires user interaction", | |
| ); | |
| } | |
| } | |
| console.error( | |
| "Audio Manager - Sound permission request failed:", | |
| error instanceof Error ? error.message : String(error), | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/utils/audioManager.ts around lines 157 to 165, the
catch block currently emits two console.log calls with an extra informational
line; replace both with a single console.error that includes a clear message and
the error details (use error instanceof Error ? error.message : String(error) or
include the error object) so the failure is logged as an error and the noisy
informational log is removed.
| this.audioElement.play().catch((error) => { | ||
| console.warn("Audio Manager - Audio play failed:", error); | ||
| }); | ||
| console.log("Audio Manager - Audio play called"); |
There was a problem hiding this comment.
Replace warn with error in play() catches and remove debug prints.
Playback failures are errors; debug prints should be removed or moved to a proper logger.
- this.audioElement.play().catch((error) => {
- console.warn("Audio Manager - Audio play failed:", error);
- });
- console.log("Audio Manager - Audio play called");
+ this.audioElement.play().catch((error) => {
+ console.error("Audio Manager - Audio play failed:", error);
+ });
@@
- .then(() => {
- console.log("Audio Manager - Test audio played successfully");
- })
- .catch((error) => {
- console.warn("Audio Manager - Test audio failed:", error);
+ .catch((error) => {
+ console.error("Audio Manager - Test audio failed:", error);
if (error.name === "NotAllowedError") {
- console.warn(
+ console.error(
"Audio Manager - Audio blocked by browser autoplay policy. User interaction required.",
);
}
});
@@
- .then(() => {
- console.log("Audio Manager - Force audio played successfully");
- })
.catch((error) => {
- console.warn("Audio Manager - Force audio failed:", error);
+ console.error("Audio Manager - Force audio failed:", error);
if (error.name === "NotAllowedError") {
- console.warn(
+ console.error(
"Audio Manager - Audio blocked by browser autoplay policy. User interaction required.",
);
}
});Also applies to: 280-289, 298-315
🤖 Prompt for AI Agents
In zmscitizencalldisplay/src/utils/audioManager.ts around lines 190-193 (and
similarly at 280-289 and 298-315), replace console.warn in the audio.play()
.catch handlers with console.error and remove any stray debug console.log
prints; ensure failures are logged as errors with the caught error object and
delete or migrate debug prints to the proper logger so no direct
console.debug/info calls remain in these blocks.
| import { JSDOM } from 'jsdom'; | ||
| import DOMPurify from 'dompurify'; | ||
|
|
||
| // Create a JSDOM instance for DOMPurify | ||
| const dom = new JSDOM('<!DOCTYPE html><html><body></body></html>'); | ||
| global.window = dom.window as any; | ||
| global.document = dom.window.document; | ||
| global.DOMPurify = DOMPurify(dom.window); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t create a second JSDOM; reuse Vitest’s window and type global safely.
Vitest with environment: "jsdom" already provides window/document. Creating another JSDOM can cause inconsistencies. Initialize DOMPurify from the existing window and add a typed global.
-import { JSDOM } from 'jsdom';
-import DOMPurify from 'dompurify';
-
-// Create a JSDOM instance for DOMPurify
-const dom = new JSDOM('<!DOCTYPE html><html><body></body></html>');
-global.window = dom.window as any;
-global.document = dom.window.document;
-global.DOMPurify = DOMPurify(dom.window);
+import createDOMPurify, { DOMPurifyI } from 'dompurify';
+
+declare global {
+ // Available to tests that expect a global DOMPurify instance
+ // eslint-disable-next-line no-var
+ var DOMPurify: DOMPurifyI;
+}
+
+// Vitest provides `window`/`document` in jsdom environment
+globalThis.DOMPurify = createDOMPurify(window as unknown as Window);📝 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.
| import { JSDOM } from 'jsdom'; | |
| import DOMPurify from 'dompurify'; | |
| // Create a JSDOM instance for DOMPurify | |
| const dom = new JSDOM('<!DOCTYPE html><html><body></body></html>'); | |
| global.window = dom.window as any; | |
| global.document = dom.window.document; | |
| global.DOMPurify = DOMPurify(dom.window); | |
| import createDOMPurify, { DOMPurifyI } from 'dompurify'; | |
| declare global { | |
| // Available to tests that expect a global DOMPurify instance | |
| // eslint-disable-next-line no-var | |
| var DOMPurify: DOMPurifyI; | |
| } | |
| // Vitest provides `window`/`document` in jsdom environment | |
| globalThis.DOMPurify = createDOMPurify(window as unknown as Window); |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/tests/setup.ts lines 1-8, remove the new JSDOM creation
and instead use Vitest's provided jsdom globals; initialize DOMPurify with the
existing window (e.g., DOMPurify(window)) and assign it to a typed global. Add a
short ambient declaration to type global.DOMPurify (declare global { var
DOMPurify: typeof DOMPurify }) so the global is safe to use in TypeScript, and
ensure you export or run the initialization only once in the setup file.
| @@ -0,0 +1,23 @@ | |||
| import {defineConfig} from "vite"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use Vitest’s defineConfig for proper typing and DX.
Import from "vitest/config" instead of "vite" so the test key is typed and validated.
Apply:
-import {defineConfig} from "vite";
+import { defineConfig } from "vitest/config";📝 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.
| import {defineConfig} from "vite"; | |
| import { defineConfig } from "vitest/config"; |
🤖 Prompt for AI Agents
In zmscitizencalldisplay/vitest.config.ts around line 1, the file imports
defineConfig from "vite" which prevents Vitest from typing and validating the
test key; change the import to come from "vitest/config" (i.e. import
defineConfig from "vitest/config") and ensure the config is exported via
defineConfig(...) so the test property receives proper typings and DX.
| vue({ | ||
| features: { | ||
| customElement: true | ||
| } | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Vue plugin option: "features.customElement" likely unsupported.
@vitejs/plugin-vue expects customElement: boolean at the top level (or template.compilerOptions.isCustomElement). Please confirm; if unsupported, switch to the top‑level flag or compiler option.
Proposed change:
- vue({
- features: {
- customElement: true
- }
- })
+ vue({
+ customElement: true
+ })If your goal is to recognize custom tags (not compile SFCs as CEs), use:
vue({
template: { compilerOptions: { isCustomElement: tag => tag.includes('-') } }
})🌐 Web query:
Is "features.customElement" a valid option for @vitejs/plugin-vue (Vite 5/6)? What are the documented options for enabling custom elements support?
💡 Result:
Short answer: No — "features.customElement" is not a documented option for @vitejs/plugin-vue. The plugin exposes a top-level customElement option (and Vue's template/compilerOptions for treating tags as custom elements).
Documented ways to enable/customize custom-element behavior:
- plugin option customElement on @vitejs/plugin-vue — type: boolean | string | RegExp | (string|RegExp)[]; default matching /.ce.vue$/; true makes all .vue files compiled as custom elements, or supply patterns to match files. [1][2]
- For skipping component resolution / treating tags as native custom elements, use the compiler option isCustomElement via the plugin's template.compilerOptions (e.g. vue({ template: { compilerOptions: { isCustomElement: tag => tag.includes('-') } } })). This is a Vue compiler option, not "features.customElement". [3][2]
Sources:
- @vitejs/plugin-vue README (customElement option). [1][2]
- Vue docs / Vite config example for compilerOptions.isCustomElement. [3]
Replace unsupported "features.customElement" with documented @vitejs/plugin-vue options
features.customElement is not a valid @vitejs/plugin-vue option; use the plugin's top-level customElement or template.compilerOptions.isCustomElement.
File: zmscitizencalldisplay/vitest.config.ts — lines 7–11
- vue({
- features: {
- customElement: true
- }
- })
+ vue({
+ customElement: true
+ })Alternative (treat tags as native custom elements):
vue({
template: { compilerOptions: { isCustomElement: tag => tag.includes('-') } }
})🤖 Prompt for AI Agents
In zmscitizencalldisplay/vitest.config.ts around lines 7 to 11, replace the
invalid vue plugin option "features.customElement" with a supported
configuration: either use the plugin's top-level customElement option or set
template.compilerOptions.isCustomElement; update the vue() call to remove
features and instead pass customElement: true (or customElement: tag => /*
predicate */) or template: { compilerOptions: { isCustomElement: tag =>
tag.includes('-') } } so the plugin accepts custom element handling.
|
How would you centralize this to one file @coderabbitai? zmscitizenview/src/utils/sanitizeHtml.ts |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me also check the overall project structure to better understand the best place for centralization: @ThomasAFink I can see you have duplicate Option 1: Workspace/Monorepo Shared Package (Recommended)Create a shared utilities package that both applications can consume: Benefits:
Option 2: Root-Level Shared DirectoryCreate a shared directory at the repository root: Then configure TypeScript path mapping in both applications: // In both zmscitizencalldisplay/tsconfig.json and zmscitizenview/tsconfig.json
{
"compilerOptions": {
"paths": {
"@shared/*": ["../shared/*"]
}
}
}Option 3: Move to a Common Parent DirectoryIf the applications are tightly coupled, create a Implementation Steps:
Would you like me to help implement any of these approaches, or do you have a preference for which centralization strategy to use? 🏁 Script executed: Length of output: 1587 🏁 Script executed: Length of output: 712 Perfect! I can see the files are identical and you have a monorepo structure with multiple ZMS applications. Here's my recommendation for centralizing the Recommended Approach: Create a Shared Utilities DirectoryGiven your monorepo structure, I recommend creating a shared utilities directory at the root level: 1. Create the Shared Structure2. Move sanitizeHtml.ts to Shared Location// shared/utils/sanitizeHtml.ts
import type { Config } from "dompurify";
import DOMPurify from "dompurify";
// DOMPurify-backed sanitizer. Keeps defaults and adds minimal SVG tags/attrs for icon sprites.
const config: Config = {
ADD_TAGS: ["svg", "symbol", "defs", "use", "path", "g"],
ADD_ATTR: [
"viewBox",
"xmlns",
"fill",
"stroke",
"d",
"width",
"height",
"focusable",
"xlink:href",
],
FORBID_TAGS: ["img"],
};
export function sanitizeHtml(dirtyHtml: string | null | undefined): string {
const input = (dirtyHtml ?? "").toString();
if (input.trim() === "") return "";
return DOMPurify.sanitize(input, config) as string;
}
export default sanitizeHtml;3. Update TypeScript ConfigurationsAdd path mapping to both applications' // zmscitizencalldisplay/tsconfig.json & zmscitizenview/tsconfig.json
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
"@/*": ["./src/*"],
"@shared/*": ["../shared/*"]
}
}
}4. Update Imports in Both Applications// Replace in both applications:
// OLD: import { sanitizeHtml } from '@/utils/sanitizeHtml'
// NEW: import { sanitizeHtml } from '@shared/utils/sanitizeHtml'5. Create Shared Package Configuration (Optional but Recommended)// shared/package.json
{
"name": "@eappointment/shared-utils",
"version": "1.0.0",
"type": "module",
"main": "./utils/index.ts",
"dependencies": {
"dompurify": "^3.0.0"
}
}6. Implementation Steps
This approach will:
Would you like me to help identify other utilities that could benefit from this centralization approach? 🏁 Script executed: Length of output: 1986 🏁 Script executed: Length of output: 0 Excellent! I can see more potential for centralization. You have multiple Let me check for additional duplicated utilities and create a proper implementation plan: Now let's examine the TypeScript configurations to understand how to properly set up the path mappings: Perfect! Now I have a complete understanding. Here's your centralization implementation with the shared structure already created: ✅ Implementation Complete - Next Steps:1. Update TypeScript ConfigurationsUpdate both applications' For zmscitizencalldisplay/tsconfig.json: {
"compilerOptions": {
"baseUrl": ".",
"paths": {
"@/*": ["./src/*"],
"@shared/*": ["../shared/*"]
}
// ... rest of your existing config
}
}For zmscitizenview/tsconfig.json: {
"compilerOptions": {
"baseUrl": ".",
"paths": {
"@/*": ["./src/*"],
"@shared/*": ["../shared/*"]
}
// ... rest of your existing config
}
}2. Update Imports in Your ApplicationsFind and replace all imports in both applications: // OLD imports - replace these:
import { sanitizeHtml } from '@/utils/sanitizeHtml'
import sanitizeHtml from '@/utils/sanitizeHtml'
// NEW imports - use these instead:
import { sanitizeHtml } from '@shared/utils/sanitizeHtml'
// or
import { sanitizeHtml } from '@shared/utils'3. Remove the Duplicated FilesAfter updating imports and testing: 4. Additional Centralization OpportunitiesI noticed these utilities that could potentially be shared:
Benefits Achieved:
Would you like me to help you identify and migrate other duplicated utilities, or would you prefer to focus on testing this sanitizeHtml centralization first? 🏁 Script executed: Length of output: 824 🏁 Script executed: Length of output: 4236 🏁 Script executed: Length of output: 0 Perfect! I can see the usage patterns and have successfully set up the shared structure. Now let's complete the centralization by updating the imports: Now let's apply the updated TypeScript configurations: ✅ Centralization Complete!Summary of Changes Made:
Next Steps for You:
Benefits Achieved:
The centralization is now complete! Both applications will use the same sanitizeHtml utility from the shared directory. Would you like me to help migrate any of the other utilities I identified? 🏁 Script executed: Length of output: 875 🏁 Script executed: Length of output: 919 Perfect! I need to handle one more file that's using the sanitizeHtml utility: Excellent! 🎉 The centralization is now 100% complete! ✅ Final Summary - sanitizeHtml CentralizationWhat Was Accomplished:
Files Created/Modified:Benefits Achieved:
Next Opportunities:If you want to continue centralizing, consider these utilities:
The centralization is complete and ready for testing! Both applications now share the same sanitizeHtml utility with proper TypeScript support. 🚀 🏁 Script executed: Length of output: 4985 🎉 Perfect! Centralization Complete! ✅ Mission Accomplished - sanitizeHtml Successfully CentralizedFinal Status: 100% Complete ✨All
Final Shared Structure:What You've Gained:
Ready for Testing:
Future Opportunities:Your codebase is now ready to centralize these utilities:
Excellent work! The centralization pattern is now established and ready for your development workflow. Would you like help centralizing any other utilities, or are you ready to test this implementation? ✏ Learnings added
|
Pull Request Checklist (Feature Branch to
next):nextBranch in meinen Feature-Branch gemergt.Summary by CodeRabbit
New Features
Configuration
Chores