[compliments] fix: duplicate query param "?" in compliments module refresh url #3967
Conversation
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
Checks if this.urlSuffix already contains a ? If it does, uses & to append the dummy parameter If it doesn't, uses ? to start a new query string
xsorifc28
force-pushed
the
fix/compliments-cache-buster-url-suffix
branch
from
07d6b8a to
dacb804
Compare
KristjanESPERANTO
changed the title
KristjanESPERANTO
approved these changes
Nov 19, 2025
Collaborator
|
Nice catch and good PR! Thanks for this fix! 😃 |
rejas
pushed a commit
that referenced
this pull request
Nov 23, 2025
…unit tests (#3969) ## Changes - Replace `indexOf()` with `startsWith()` for cleaner protocol detection - Use `URL` API for robust cache-busting parameter handling - Add HTTP response validation and improved error logging - Add JSDoc type annotations for better documentation - Remove unused `urlSuffix` instance variable - Add unit tests - Fix `.gitignore` pattern ## Motivation After merging #3967, I noticed some potential for improving reliability and user experience related to the method `loadComplimentFile`. With these changes the method now validates URLs upfront to catch configuration errors early, checks HTTP status codes to detect server issues (404/500), and provides specific error messages that help users troubleshoot problems. The complexity of the code does not really increase with the changes. On the contrary, the method should now be more intuitive to understand. ## Testing Added unit tests for `loadComplimentFile()` to validate the improvements: - HTTP error handling - Cache-busting Since E2E tests already cover the happy path, these unit tests focus on error cases and edge cases. ## Additional Fix While adding the test file, I discovered that the `.gitignore` pattern `modules` was incorrectly matching `tests/unit/modules/`, preventing test files from being tracked. Changed to `/modules` to only match the root directory.
Merged
sdetweil
added a commit
that referenced
this pull request
Jan 1, 2026
Thanks to: @Blackspirits, @Crazylegstoo, @jarnoml, @jboucly, @JHWelch, @khassel, @KristjanESPERANTO, @rejas, @sdetweil, @xsorifc28⚠️ This release needs nodejs version >=22.21.1 <23 || >=24 [Compare to previous Release v2.33.0](v2.33.0...develop) [core] Prepare Release 2.34.0 (#3998) dependency update + adjust Playwright setup + fix linter issue (#3994) demo with gif (#3995) [core] fix: allow browser globals in config files (#3992) [core] fix: restore --ozone-platform=wayland flag for reliable Wayland support (#3989) [core] auto create release notes with every push on develop (#3985) [core] chore: simplify Wayland start script (#3974) [gitignore] restore the old Git behavior for the default modules (#3968) [core] configure cspell to check default modules only and fix typos (#3955) [gitignore] restoring the old Git behavior for the CSS directory (#3954) feat(core): add server:watch script with automatic restart on file changes (#3920) [check_config] refactor: improve error handling (#3927) refactor: replace express-ipfilter with lightweight custom middleware (#3917) refactor: replace module-alias dependency with internal alias resolver (#3893) [dependencies] [chore] update dependencies and min. node version (#3986) [core] bump dependencies into december (#3982) Bump actions/checkout from 5 to 6 (#3972) Update deps, unpin parse5 (#3934) [core] Update deps and pin jsdom to v27.0.0 (#3925) chore: update dependencies (#3921) update deps, exclude node v23 (#3916) remove eslint warnings, add npm publish process to Collaboration.md (#3913) feat: add ESlint rule no-sparse-arrays for config check (#3911) chore: bump dependencies into october (#3909) [logging] logger: add calling filename as prefix on server side (#3926) [logger] Add prefixes to most Log messages (#3923) [modules/alert] Add new pt and pt-BR translations for Alert module and update global PT strings (#3965) [modules/calendar] add checksum to test whether calendar event list changed (#3988) [calendar] fix: prevent excessive fetching on client reload and refactor calendarfetcherutils.js (#3976) [calendar] refactor: migrate CalendarFetcher to ES6 class and improve error handling (#3959) [calendar] Show repeatingCountTitle only if yearDiff > 0 (#3949) [tests] suppress debug logs in CI environment + improve calendar symbol test stability (#3941) [calendar] chore: remove requiresVersion: "2.1.0" (#3932) [calendar] test: remove "Recurring event per timezone" test (#3929) [modules/compliments] [compliments] refactor: optimize loadComplimentFile method and add unit tests (#3969) fix: set compliments remote file minimum delay to 15 minutes (#3970) [compliments] fix: duplicate query param "?" in compliments module refresh url (#3967) [modules/newsfeed] [newsfeed] fix header layout issue (#3946) [modules/weather] [weatherprovider] update subclass language use override (#3914) [weather] fix wind-icon not showing in pirateweather (#3957) [weather] add error handling to weather fetch functions, including cors (#3791) remove deprecated ukmetoffice datapoint provider, cleanup .gitignore (#3952) fixes problems with daylight-saving-time in weather provider openmeteo (#3931) Fix for envcanada Provider to use updated Env Canada URL (#3919) [weather] feat: add configurable forecast date format option (#3918) [testing] testing: update "Enforce Pull-Request Rules" workflow (#3987) [core] refactor: replace XMLHttpRequest with fetch and migrate e2e tests to Playwright (#3950) [test] replace node-libgpiod with serialport in electron-rebuild workflow (#3945) [ci] Add concurrency to automated tests workflow to cancel outdated runs (#3943) [tests] migrate from jest to vitest (#3940) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Karsten Hassel <hassel@gmx.de> Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Co-authored-by: Ryan Williams <65094007+ryan-d-williams@users.noreply.github.com> Co-authored-by: Veeck <github@veeck.de> Co-authored-by: Bugsounet - Cédric <github@bugsounet.fr> Co-authored-by: Marc Landis <dirk.rettschlag@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: HeikoGr <20295490+HeikoGr@users.noreply.github.com> Co-authored-by: Pedro Lamas <pedrolamas@gmail.com> Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Magnus <34011212+MagMar94@users.noreply.github.com> Co-authored-by: Ikko Eltociear Ashimine <eltociear@gmail.com> Co-authored-by: DevIncomin <56730075+Developer-Incoming@users.noreply.github.com> Co-authored-by: Nathan <n8nyoung@gmail.com> Co-authored-by: mixasgr <mixasgr@users.noreply.github.com> Co-authored-by: Savvas Adamtziloglou <savvas-gr@greeklug.gr> Co-authored-by: Konstantinos <geraki@gmail.com> Co-authored-by: OWL4C <124401812+OWL4C@users.noreply.github.com> Co-authored-by: BugHaver <43462320+bughaver@users.noreply.github.com> Co-authored-by: BugHaver <43462320+lsaadeh@users.noreply.github.com> Co-authored-by: Koen Konst <koenspero@gmail.com> Co-authored-by: Koen Konst <c.h.konst@avisi.nl> Co-authored-by: dathbe <github@beffa.us> Co-authored-by: Marcel <m-idler@users.noreply.github.com> Co-authored-by: Kevin G. <crazylegstoo@gmail.com> Co-authored-by: Jboucly <33218155+jboucly@users.noreply.github.com> Co-authored-by: Jboucly <contact@jboucly.fr> Co-authored-by: Jarno <54169345+jarnoml@users.noreply.github.com> Co-authored-by: Jordan Welch <JordanHWelch@gmail.com> Co-authored-by: Blackspirits <blackspirits@gmail.com> Co-authored-by: Samed Ozdemir <samed@xsor.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Discovered a bug for compliments module "remote file config" fetching. If the remote file contains a
?, likehttp://remoteconfig.com?type=all, the url becomeshttp://remoteconfig.com?type=all?dummy={timestamp}, which is incorrect.This fix:
this.config.remoteFile.includesalready contains a?&to append the dummy parameter?to start a new query string