Skip to content

Fixes issue 4243: Ensure while loop in calendar module does not get stuck when no entries - #4244

Merged
khassel merged 2 commits into
MagicMirrorOrg:developfrom
deBasMan21:patch-1
Sep 9, 2026
Merged

Fixes issue 4243: Ensure while loop in calendar module does not get stuck when no entries#4244
khassel merged 2 commits into
MagicMirrorOrg:developfrom
deBasMan21:patch-1

Conversation

@deBasMan21

@deBasMan21 deBasMan21 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes: #4243

@KristjanESPERANTO

KristjanESPERANTO commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Thanks. But please base your pull requests against the develop branch.

@khassel
khassel force-pushed the patch-1 branch 5 times, most recently from abb9177 to 7a4901b Compare September 4, 2026 22:40
@khassel
khassel changed the base branch from master to develop September 4, 2026 22:40
@khassel

khassel commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

@KristjanESPERANTO I was able to switch this to develop

@KristjanESPERANTO

KristjanESPERANTO commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

@deBasMan21 I haven’t looked deeply into this yet, but could moving daysCollected++ outside the if affect how limitDays is intended to work?

@sdetweil

sdetweil commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Yes, that was why it was counted inside the loop

@deBasMan21

deBasMan21 commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@khassel thanks for switching it to develop!

@KristjanESPERANTO It should actually not really affect the limitDays functionality. It was counted inside the loop for n amount of days to collect calendar items from. However when a day has no items it would keep being stuck in the loop so no more items would be added. With this change the calendar items for the next n days would still be collected, but when there is a day without any items nothing would break. So when you set limitDays to 5 for example, it would still only collect the items from the next 5 days. I hope i explained this clear enough, but otherwise let me know ofcourse!

KristjanESPERANTO
KristjanESPERANTO approved these changes Sep 7, 2026
View reviewed changes

@KristjanESPERANTO KristjanESPERANTO left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, that makes sense to me now. Would you also like to add a regression test for this? No problem if not :)

@deBasMan21

deBasMan21 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

I would love to, but currently im a bit short on time so maybe i will add one later on.

@KristjanESPERANTO

KristjanESPERANTO commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

I wrote a test. Without your fix, it fails; with your fix, it passes. So, as far as I'm concerned, the PR is ready to be merged :)

@khassel
khassel merged commit 1761bcc into MagicMirrorOrg:develop Sep 9, 2026
13 checks passed
@KristjanESPERANTO KristjanESPERANTO mentioned this pull request Oct 1, 2026
Merged
KristjanESPERANTO added a commit that referenced this pull request Oct 1, 2026
@KristjanESPERANTO @Crazylegstoo
veeck @rejas @khassel @jboucly Jboucly @dependabot @jarnoml @sdetweil @JHWelch @Blackspirits @xsorifc28 @in-voker @angeldeejay @cgillinger @sonnyb9 @egeekial @claude @MikeBishop @ago1776 @pascalpfammatter @Antra @deBasMan21 @MannXo @alkank @i-xul @flightlesstux @jamalkamaladdin
Release 2.38.0 (#4286)
f138f5b 
## Release Notes
Thanks to: @ago1776, @alkank, @Antra, @Blackspirits, @deBasMan21,
@flightlesstux, @i-xul, @jamalkamaladdin, @khassel, @KristjanESPERANTO,
@MannXo, @pascalpfammatter, @rejas
> ⚠️ This release needs nodejs version >=22.22.2 <23 || >=24

[Compare to previous Release
v2.37.0](v2.37.0...develop)


### [core]
- Prepare Release 2.38.0 (#4285)
- fix: let Electron select the Wayland display (#4268)
- fix(http): improve access error messages (#4281)
- fix: always redact client configuration (#4275)
- refactor: migrate browser scripts to ESM (#4272)
- refactor: convert animateCSS to ES module (#4266)
- use filter instead of find to get all allowed secrets when multiple
instances of a module are active (#4265)
- refactor: convert notificationFx to ES module (#4263)
- refactor: enforce function expression style (#4262)
- fix: validate client IP behind trusted proxies (#4261)
- prefer arrow over function (#4252)
- refactor(utils): simplify configuration validation and error handling
(#4240)
- fix(server): validate request origins (#4234)
- remove codeql warning (#4231)
- refactor(socket): use direct ESM import in main (#4225)
- refactor(socket): load Socket.IO client via ESM (#4224)
- style: simplify Electron startup checks (#4213)
- refactor: consolidate electron bootstrap into async function (#4210)
- refactor(module): simplify and fix configMerge (#4203)
- refactor: convert Translator to ES module (#4202)
- docs: add CodeQL review steps and restructure after-release checklist
(#4200)
- refactor: centralize server port resolution (#4198)
- refactor: use const instead of let for variable declarations (#4196)
- refactor: replace Loader object with named exports (#4195)
- set next release dev number
- refactor(eslint): enforce prefer-arrow-callback rule (#4251)
- refactor(http_fetcher): make dynamic URL handling explicit (#4282)
- refactor: use explicit global config (#4233)
- expand logic of hideConfigSecrets, add more tests (#4229)
- refactor: make browser startup more explicit (#4214)

### [dependencies]
- update dependencies (#4276)
- update dependencies incl. electron to v44 (#4250)
- chore: update eslint incl. plugins + review rules (#4236)
- Bump actions/stale from 10 to 11 (#4215)
- update dependencies (#4221)
- Bump actions/setup-node from 6 to 7 (#4204)
- chore: update dependencies (#4201)
- Bump electron from 42.5.2 to 43.0.0 (#4192)

### [modules/alert]
- Move alert translations into common translations (#4284)

### [modules/calendar]
- Fixes issue 4243: Ensure while loop in calendar module does not get
stuck when no entries (#4244)
- fix(calendar): escape HTML in event title and location (#4260)
- calendar: allow custom events to override symbol class (#4257)
- Fix calendar crash when yearmatchgroup regex does not match (#4239)
- fix(calendar): keep yearly events in DTSTART's month when BYMONTH is
missing (#4222)
- fix(calendar): correct multi-day slice day counts (#4232)
- chore: update dependencies + adapt calendar tests (#4211)
- fix(calendar): sliced multi-day events start at 00:00, not 23:59
(#4208)

### [modules/newsfeed]
- newsfeed: don't use cors url in njk template (#4279)
- refactor(newsfeed): extract feed item normalization into feeditem.js
(#4271)
- refactor(newsfeed): replace feedme lib with feedparser (#4269)
- newsfeed: fix allowedBasicHtmlTags (#4258)
- newsfeed: update checkArticleUrl (#4256)

### [modules/updatenotification]
- feat(updatenotification): implement trusted module configuration for
update commands (#4259)

### [modules/weather]
- feat: add FMI weather provider (#4278)
- feat(weather): enable CSS ordering for forecast and hourly columns
(#4118)
- fix(weather): respect initial load delay (#4254)
- fix(weather): restrict provider loading to the providers directory
(#4235)
- refactor(weather): extract WeatherProvider base class (#4228)

### [testing]
- ci: fail lint on warnings (#4230)
- add test if install works for minimal node version (#4197)

### [translation]
- feat(i18n): add Azerbaijani translation (#4283)
- feat(i18n): complete Turkish translations (#4277)
- Improve Portuguese translations (#4274)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Kevin G. <crazylegstoo@gmail.com>
Co-authored-by: veeck <gitkraken@veeck.de>
Co-authored-by: Veeck <github@veeck.de>
Co-authored-by: Karsten Hassel <hassel@gmx.de>
Co-authored-by: Jboucly <33218155+jboucly@users.noreply.github.com>
Co-authored-by: Jboucly <contact@jboucly.fr>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jarno <54169345+jarnoml@users.noreply.github.com>
Co-authored-by: sam detweiler <sdetweil@gmail.com>
Co-authored-by: Jordan Welch <JordanHWelch@gmail.com>
Co-authored-by: Blackspirits <blackspirits@gmail.com>
Co-authored-by: Samed Ozdemir <samed@xsor.io>
Co-authored-by: in-voker <58696565+in-voker@users.noreply.github.com>
Co-authored-by: Andrés Vanegas Jiménez <142350+angeldeejay@users.noreply.github.com>
Co-authored-by: cgillinger <christian.gillinger@gmail.com>
Co-authored-by: Sonny B <43247590+sonnyb9@users.noreply.github.com>
Co-authored-by: sonnyb9 <sonnyb9@users.noreply.github.com>
Co-authored-by: Morgan McBee <egeekial@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Mike Bishop <mbishop@evequefou.be>
Co-authored-by: ago1776 <andi.goepfert@gmx.de>
Co-authored-by: pascalpfammatter <pascal.pfammatter@mirific.ch>
Co-authored-by: Pascal Pfammatter <304897782+pascalpfammatter@users.noreply.github.com>
Co-authored-by: Anders Demant van der Weide <antra@antra.dk>
Co-authored-by: Bas Buijsen <72604903+deBasMan21@users.noreply.github.com>
Co-authored-by: Parman Mohammadalizadeh <prmma23@gmail.com>
Co-authored-by: alkank <alkankizilca@gmail.com>
Co-authored-by: Henkka <121262594+i-xul@users.noreply.github.com>
Co-authored-by: Ercan Ermis <18646235+flightlesstux@users.noreply.github.com>
Co-authored-by: Jamal Kamaladdinoglu <jamalkamaladdin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@khassel khassel khassel approved these changes
@KristjanESPERANTO KristjanESPERANTO KristjanESPERANTO approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] All items not loading when calendar loading fails

4 participants

@deBasMan21 @KristjanESPERANTO @khassel @sdetweil