Skip to content

Only pass string addresses to ICS downloads - #184

Merged
edalzell merged 2 commits into
mainfrom
183-ics-location-array
Aug 18, 2026
Merged

Only pass string addresses to ICS downloads#184
edalzell merged 2 commits into
mainfrom
183-ics-location-array

Conversation

@adnankussair

@adnankussairadnankussair commented Aug 17, 2026

Copy link
Copy Markdown
Member

Summary

Test plan

  • ICS download with string address still writes LOCATION:
  • ICS download with string location and no address still writes LOCATION:
  • ICS download with group/array location and no address returns 200 and omits LOCATION: (single, recurring, multi-day)

Note

Low Risk
Localized ICS export fix with type guards and regression tests; no auth, persistence, or API contract changes beyond avoiding errors on array locations.

Overview
Fixes ICS 500 errors when location is a group/array (e.g. Prime location fieldset, empty location: {}, or coords-only) instead of a plain string.

Introduces icsAddress() on the base Event type so Spatie’s iCal builder only receives a non-empty string—still preferring address, then falling back to string location. Single-day, recurring, and multi-day ICS builders all route through this helper instead of passing raw location values.

eventUrl() is aligned to read location via get('location') and skip non-string values before URL checks.

New controller tests cover grouped location without LOCATION: in the file, string location fallback, and the same cases for recurring and multi-day exports.

Reviewed by Cursor Bugbot for commit cf61c0c. Bugbot is set up for automated code reviews on this repo. Configure here.

Skip group/array location fields so Spatie no longer 500s when address is empty.
Co-authored-by: Cursor <cursoragent@cursor.com>
Comment threadsrc/Types/RecurringEvent.php Outdated
Co-authored-by: Cursor <cursoragent@cursor.com>
@edalzell
edalzell marked this pull request as draft August 17, 2026 22:03
@adnankussairadnankussair mentioned this pull request Aug 17, 2026
1 task
@adnankussair

Copy link
Copy Markdown
MemberAuthor

Follow-up: the RruleRRule casing cleanup Erin flagged is now in a separate draft PR: #185 (formatting-only, no behavior change).

@edalzell

Copy link
Copy Markdown
Member

@adnankussair thinking through this a bit more, not sure this is the correct fix at all because Events doesn't actually define an address or location field. The location field comes from Prime.

@adnankussair

adnankussair commented Aug 17, 2026

Copy link
Copy Markdown
MemberAuthor

@edalzell Agreed that Prime owns the location fieldset and computed address, that is where the group/array shape comes from.

The 500 still happens in Events though. The ICS builder falls back to raw location when address is empty, and Spatie requires a string. Prime already returns a string address when location.details is set; the bug is in Events passing the group array on fallback.

Events is also a standalone package with tests for plain string location on non-Prime installs, so we cannot drop that fallback, we just need to ensure only strings reach Spatie.

Hence no Prime change required to fix the 500.

@edalzell

Copy link
Copy Markdown
Member

Yo cursor, back off, I wanna talk w/ @adnankussair

@edalzell
edalzell marked this pull request as ready for review August 18, 2026 19:49
@edalzell
edalzell merged commit c5fa026 into mainAug 18, 2026
26 checks passed
@edalzell
edalzell deleted the 183-ics-location-array branch August 18, 2026 19:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ICS download 500s when location is a group/array

2 participants

@adnankussair@edalzell