Skip to content

Comments

🐛 Fix: allday events not displaying if start-end date is start of week (Attempt 2)#453

Closed
that-one-arab wants to merge 5 commits intomainfrom
bug_fix_allday_events_start_of_week_not_displaying
Closed

🐛 Fix: allday events not displaying if start-end date is start of week (Attempt 2)#453
that-one-arab wants to merge 5 commits intomainfrom
bug_fix_allday_events_start_of_week_not_displaying

Conversation

@that-one-arab
Copy link
Contributor

Description

Follow up to #440
Closes #428

This PR solves the issue by aiming at solving it from the frontend side.

It resolves it by sending an additional HTTP request specifically for all day events. This works because the additional HTTP request is sent under date filters that are different than the get events HTTP request (uses YYYY-MM-DD format rather than ISO 8601)

Sadly, this introduces an additional HTTP request to be sent, which would degrade performance. This is the only way to resolve this issue solely from the frontend side that I know of.

This could be further optimized by implementing a backend side solution to send 2 database requests in 1 HTTP request, rather than sending 2 database requests in 2 HTTP requests, but since the goal is to implement a frontend solution only, I omitted doing that.

This helps with cases where the incoming date filter is malformed or invalid.

One common case is where a timezone string is sent over HTTP. The timezone string contains a "+" character which is a special character and is replaced with a space, this character needs to be encoded before sent over HTTP. If we receive a non-encoded timezone date string, we need to catch it here, or we would have issues with filtering.
Since we now validate incoming dates in the `getReadAllFilter`, this change thankfully caught a failing test, test is now fixed.
Updated the event API to encode the start and end date parameters in the request URL, ensuring proper handling of special characters and preventing potential issues with malformed date strings.
… into dedicated helper function

Since this dispatch call is reused in 2 different places, this helps in keeping the logic consistent.
@that-one-arab that-one-arab requested a review from tyler-dane May 25, 2025 16:08
@tyler-dane
Copy link
Contributor

I appreciate the persistent, but this unfortunately also isn't going to work. The problem is that we're already taking a big performance hit due to #456. After testing this branch, I found the UX to be too slow, and I'm afraid the users will think the same and drop.

The bug this attempts to fix is not ideal, but it does save the event to the user's google calendar and our Compass DB. So, it results in some annoyance if a user makes an all day event on Sunday. But introducing this PR without fixing #456 would guarantee a worse UX for all users whenever they change weeks.

Thinking through the root cause a little more, I agree with what you said in #440 about this being related to timezones. Google Calendar API includes a timezone property for every event. The best path forward is probably not to change the date format for all-day events, but to also add a timezone to our event model and use it in our query to catch events that are at the week's edge.

We've spent enough time on this issue for now, so let's revisit after finishing #456 and figuring out how to change the event model on the backend.

@tyler-dane tyler-dane closed this May 26, 2025
@tyler-dane tyler-dane deleted the bug_fix_allday_events_start_of_week_not_displaying branch September 25, 2025 00:26
Sign up for free to 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.

Creating an all day event at the start of the week does not display it after refreshing the page

2 participants