clean(ZMSKVR): CodeQL fixes for ASI, unused bindings, and dead code#2134
Open
ThomasAFink wants to merge 6 commits intomainfrom
Open
clean(ZMSKVR): CodeQL fixes for ASI, unused bindings, and dead code#2134ThomasAFink wants to merge 6 commits intomainfrom
ThomasAFink wants to merge 6 commits intomainfrom
Conversation
- Add explicit semicolons where ASI was relied on (vitest afterAll, ticketprinter form loop). - Remove unused imports and variables across zmsadmin, zmscitizenview, and zmsticketprinter. - Wire graph timetable conflict layer; mark selected availability row with aria-selected. - Drop redundant test config and simplify apiStatusService downtime test.
Contributor
| Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughSmall-scale code cleanup and minor behavior clarifications across multiple projects: removed unused imports/variables, tightened conditionals and boolean coercion, adjusted event handlers and callback branching, and standardized statement terminators; no public APIs or major control flows were introduced. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zmsticketprinter/js/index.js (1)
6-6:⚠️ Potential issue | 🟡 MinorRemove console.log() statements and replace with proper logging or delete.
The imported jQuery version (4.0.0) is correctly specified and all used APIs (
.each(),.on(),.prop(),.addClass(),.closest(),.submit()) remain fully compatible with jQuery 4.0.0. However, the code contains three console.log() statements (lines 24, 27, 35) that should be removed per the coding guidelines. For development purposes, replace withconsole.debug()if logging is needed; for production, remove entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmsticketprinter/js/index.js` at line 6, Remove the three console.log(...) calls in zmsticketprinter/js/index.js (the ones at the previous review lines 24, 27 and 35) and either delete them for production or replace them with console.debug(...) if you need debug-level output during development; search for the exact "console.log" invocations inside the event handlers (e.g., the change/submit handlers and any functions referenced by .each(), .on(), or .submit()) and update those calls accordingly so no console.log remains.
🧹 Nitpick comments (3)
zmsadmin/js/page/availabilityDay/layouts/tableBody.js (1)
87-88: Consider using strict equality (===) for comparisons.The comparisons use loose equality (
==), which can produce unexpected results with type coercion. Ifidvalues may be strings or numbers, consider normalizing types explicitly:♻️ Suggested refactor for explicit type handling
const isSelected = Boolean(data && ( - (data.id && availability.id == data.id) || - (data.tempId && availability.tempId == data.tempId) + (data.id && String(availability.id) === String(data.id)) || + (data.tempId && String(availability.tempId) === String(data.tempId)) ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmsadmin/js/page/availabilityDay/layouts/tableBody.js` around lines 87 - 88, The comparisons use loose equality (==) between data.id/availability.id and data.tempId/availability.tempId which can cause type-coercion bugs; update the checks to use strict equality (===) and, if ids may be different types (string vs number), normalize both sides to the same type before comparing (e.g., convert both availability.id and data.id to string or number) so the (data.id && availability.id === data.id) and (data.tempId && availability.tempId === data.tempId) style checks are robust and type-safe.zmsadmin/js/page/availabilityDay/form/datepicker.js (1)
12-12: Consider removing commented-out import.This commented-out import for
formatTimestampDateshould be removed entirely rather than left commented. The same applies to other commented-out code blocks in this file (lines 239-250, 272-274, 327).As per coding guidelines: "Don't comment out code. Just remove."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmsadmin/js/page/availabilityDay/form/datepicker.js` at line 12, Remove the leftover commented-out import for formatTimestampDate and delete the other commented-out code blocks in this file (the stashed blocks around the datepicker logic and unused helpers) rather than leaving them commented; specifically remove the commented import "formatTimestampDate" and any commented sections related to date formatting/unused helper code so the module only contains active code, then run the linter/formatter to ensure no trailing whitespace or unused-import warnings remain.zmsticketprinter/js/index.js (1)
23-28: Remove or replaceconsole.log()statements.These
console.log()calls should be removed or replaced according to the coding guidelines. For development-only debugging, useconsole.debug(). For production code, these should be removed entirely.♻️ Proposed fix
if ($('.digitaluhr').length > 0) { - console.log('Found digitaluhr elements, initializing DigitalTime...'); $('.digitaluhr').each(function() { new DigitalTime(this); }); -} else { - console.log('No digitaluhr elements found.'); }And remove line 35:
-console.log("Welcome to the ZMS Ticketprinter interface...");As per coding guidelines: "Flag any usage of console.log() as it should be replaced with proper logging or removed."
Also applies to: 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zmsticketprinter/js/index.js` around lines 23 - 28, Replace the development console.log calls around the DigitalTime initialization with the appropriate action: either remove them entirely for production or change them to console.debug() for dev-only logging; specifically update the two console.log occurrences that announce finding/initializing or not finding elements around the DigitalTime setup so no console.log remains and use console.debug(...) if you still want debug output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Outside diff comments: In `@zmsticketprinter/js/index.js`: - Line 6: Remove the three console.log(...) calls in zmsticketprinter/js/index.js (the ones at the previous review lines 24, 27 and 35) and either delete them for production or replace them with console.debug(...) if you need debug-level output during development; search for the exact "console.log" invocations inside the event handlers (e.g., the change/submit handlers and any functions referenced by .each(), .on(), or .submit()) and update those calls accordingly so no console.log remains. --- Nitpick comments: In `@zmsadmin/js/page/availabilityDay/form/datepicker.js`: - Line 12: Remove the leftover commented-out import for formatTimestampDate and delete the other commented-out code blocks in this file (the stashed blocks around the datepicker logic and unused helpers) rather than leaving them commented; specifically remove the commented import "formatTimestampDate" and any commented sections related to date formatting/unused helper code so the module only contains active code, then run the linter/formatter to ensure no trailing whitespace or unused-import warnings remain. In `@zmsadmin/js/page/availabilityDay/layouts/tableBody.js`: - Around line 87-88: The comparisons use loose equality (==) between data.id/availability.id and data.tempId/availability.tempId which can cause type-coercion bugs; update the checks to use strict equality (===) and, if ids may be different types (string vs number), normalize both sides to the same type before comparing (e.g., convert both availability.id and data.id to string or number) so the (data.id && availability.id === data.id) and (data.tempId && availability.tempId === data.tempId) style checks are robust and type-safe. In `@zmsticketprinter/js/index.js`: - Around line 23-28: Replace the development console.log calls around the DigitalTime initialization with the appropriate action: either remove them entirely for production or change them to console.debug() for dev-only logging; specifically update the two console.log occurrences that announce finding/initializing or not finding elements around the DigitalTime setup so no console.log remains and use console.debug(...) if you still want debug output. ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bea8b92e-b43e-472b-a409-7f4798f34b15
📒 Files selected for processing (19)
zmsadmin/js/page/availabilityDay/form/datepicker.jszmsadmin/js/page/availabilityDay/form/validate.jszmsadmin/js/page/availabilityDay/index.jszmsadmin/js/page/availabilityDay/layouts/graphBody.jszmsadmin/js/page/availabilityDay/layouts/tableBody.jszmsadmin/js/page/availabilityDay/timetable/graphview.jszmsadmin/tests/browser/lib/config.jszmsadmin/tests/browser/source.jszmscitizenview/src/utils/calculateEstimatedDuration.tszmscitizenview/tests/unit/Appointment/AppointmentSelection/CalendarView.spec.tszmscitizenview/tests/unit/Appointment/AppointmentSelection/ListView.spec.tszmscitizenview/tests/unit/Appointment/AppointmentView.spec.tszmscitizenview/tests/unit/Appointment/ServiceFinder/SubserviceListItem.spec.tszmscitizenview/tests/unit/AppointmentDetail/AppointmentDetailView.spec.tszmscitizenview/tests/unit/AppointmentOverview/AppointmentOverviewView.spec.tszmscitizenview/tests/unit/AppointmentOverview/AppointmentSliderView.spec.tszmscitizenview/tests/unit/utils/apiStatusService.spec.tszmsticketprinter/js/index.jszmsticketprinter/js/page/main.js
💤 Files with no reviewable changes (8)
- zmsadmin/js/page/availabilityDay/timetable/graphview.js
- zmsticketprinter/js/page/main.js
- zmsadmin/tests/browser/lib/config.js
- zmsadmin/tests/browser/source.js
- zmscitizenview/src/utils/calculateEstimatedDuration.ts
- zmscitizenview/tests/unit/utils/apiStatusService.spec.ts
- zmsadmin/js/page/availabilityDay/form/validate.js
- zmsadmin/js/page/availabilityDay/index.js
- Ticketprinter: call printDialog on DOM ready with correct this binding. - zmsadmin/zmsstatistic baseview: replace useless ternary no-ops in lightbox/dialog callbacks. - Workstation: initialize reloadTimer to null instead of no-op expression. - Emergency: ESC handler uses arrow function and this.minimize().
- datepicker: drop redundant startDate/endDate updates in updateState when name/date set. - build_swagger: readApiVersion returns VERSION via readFileContent (sync), not async callback. - ListView: remove unused newDayPart initializer; assign only in branches.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Inline comments: In `@zmsadmin/js/block/emergency/index.js`: - Line 36: Remove the stray console.log('ESC') from the Escape handler to avoid debug output in production UI code; locate the Escape handler (the key/event handler that logs "ESC") and either delete the console.log call or replace it with the approved structured logging API used in the project, ensuring no raw console.log remains in that handler. In `@zmsadmin/js/lib/baseview.js`: - Around line 155-160: The background-click branch inside the lightbox callback only calls abortCallback() and neglects to restore focus to returnTarget like the normal abort path; update that branch (the anonymous callback passed to lightbox where callbackAsBackgroundAction, callback, and abortCallback are checked) to also call returnTarget.focus() (or the existing focus-restoration helper used elsewhere) after invoking abortCallback(), ensuring lightboxContentElement/destroyLightbox flows restore focus consistently for background dismissals. In `@zmsticketprinter/js/page/process.js`: - Around line 9-12: Remove the stray console.log on the ready path by deleting or replacing the console.log('Print data and redirect to home url after presetted time') call; ensure the ready handler still invokes this.printDialog() (the printDialog function) but uses the project's approved logging mechanism if a log is required (e.g., replace with the centralized logger or remove entirely) so no console.log/console.debug/console.warn remain in this execution path. ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 49038aa6-1a92-4e82-910c-4c1d4b6dae99
📒 Files selected for processing (5)
zmsadmin/js/block/emergency/index.jszmsadmin/js/lib/baseview.jszmsadmin/js/page/workstation/index.jszmsstatistic/js/lib/baseview.jszmsticketprinter/js/page/process.js
Update zmsadmin/js/block/emergency/index.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- baseview: runUserAbortCallback + focusReturnTarget; restore returnTarget focus when closing lightbox via background (not only dialog abort). - ticketprinter process: remove constructor and beforePrint console.log per logging guidelines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist (Feature Branch to
next):nextBranch in meinen Feature-Branch gemergt.Summary by CodeRabbit
Refactor
Bug Fixes
Chores