Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove local slot caching #7071

Closed
wants to merge 1 commit into from
Closed

fix: remove local slot caching #7071

wants to merge 1 commit into from

Conversation

ShaneMaglangit
Copy link
Contributor

@ShaneMaglangit ShaneMaglangit commented Feb 13, 2023

What does this PR do?

Fixes #6932
Partially fix: #6622

Revert caching of slots that was introduced from: PR 3118

This caching behavior causes unexpected issues client side. Aside from the issues mentioned earlier, this also prevent real-time revalidation of slots based on limits as they are reached (I'm working on another feat requests to add total booking time limits, this gave me a lot of headache tbh).

Why remove this?

  • This cache does not provide any perf improvement whether for rendering or reducing requests send to the server.
  • This was designed to persist slots when switching months based on PR 3118.
    • However, moving between months fetches new data everytime, so that "cached" slots gets discarded anyway (cached contents does not have a reliable invalidation logic, causing bugs).

This feature was added by @zomars, would be interested if there's other reasons why this was added and if we should refrain from removing this.

Not sure if this would be eligible for the full bounty, but hey /claim

Environment: Production

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

  • Test for Booking not working properly from a different timezone #6932: Changing timezone must not include slots from previous timezone (also affects dynamic time)
    • Open a booking page where the user has limited availability (e.g. Monday - Friday).
    • Change timezone that would cause the slots to extend to weekends.
    • Go back to the previous timezone.
    • Unlike before, the slots for the weekends aren't retained.
  • Test for [CAL-863] No available users found error on confirming booking for an open slot #6622: Users can still click select time slots even if fully booked.
    • Create an event with a limit of 1 booking per day.
    • Open the booking page for this event in two tabs.
    • Book a meeting to the event using Tab A.
    • Old Behavior: User can still book for the same day. Slot won't show as full unless page is refreshed.
    • New Behavior: Switch to Tab B. Slot should now show as fully booked.

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works

@vercel
Copy link

vercel bot commented Feb 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
ui ⬜️ Ignored (Inspect) Feb 13, 2023 at 3:18PM (UTC)

@vercel
Copy link

vercel bot commented Feb 13, 2023

@ShaneMaglangit is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@zomars
Copy link
Member

zomars commented Feb 13, 2023

Thank you for your contribution 🙏 @ShaneMaglangit We're aware of this issue and it is already being taken care of in this PR.

@ShaneMaglangit
Copy link
Contributor Author

Thank you for your contribution pray @ShaneMaglangit We're aware of this issue and it is already being taken care of in this PR.

Glad to know this is already being worked on. I'll be closing this PR in favor of the other one. Thank you!

@ShaneMaglangit ShaneMaglangit deleted the fix/remove-slot-caching branch February 16, 2023 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants