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

Create "Busy" blocks in a user's schedule #279

Merged
merged 19 commits into from
Mar 26, 2024

Conversation

MelissaAutumn
Copy link
Member

@MelissaAutumn MelissaAutumn commented Feb 28, 2024

Fixes #277

  • Adjust event_set_difference to event_roll_up_difference - This now rolls up any collisions as a "booked" slot.
  • Create a replica enum of slot's booking status on the frontend
  • Style "booked" bookable slots to not be bookable, clickable, or friendly looking.
  • Locale update!
  • Remove storeToRefs for a function that seems to be breaking.
  • Hide agenda view on mobile month, because we can't style it right now.
  • Add a hover effect to change cursor, and highlight the day in mobile month view.
  • Fix case where event_roll_up_difference would not roll up trailing events.
  • Add test case for events_roll_up_difference.

Some screenshots:

image image

This does have the potential to show up in the month view like:
image

Which I would hide, but we don't currently style that slot. I've also hidden the mobile-sized agenda view because we can't style that slot yet. I've added some logic to make it behave to how we use to do month views.

@MelissaAutumn MelissaAutumn added the l10n update A string has been added or needs updating label Feb 28, 2024
@MelissaAutumn MelissaAutumn self-assigned this Feb 28, 2024

available_slots.append(a)

# FIXME: Lack of coffee == duplicate code. Re-work this code so we don't need a tail append.
Copy link
Member Author

Choose a reason for hiding this comment

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

If anyone can think of a better solution pls suggest.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Which edge case does this tail append solve?

Copy link
Member Author

Choose a reason for hiding this comment

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

I should have just named it end append, sounds more fun 😄

If you've got a schedule 9 to 17, and you've got a event from 16-17 then it wouldn't have appended the last busy slot, because I added a continue if collision is found, and it's the end of the loop.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see

@@ -79,7 +79,7 @@ const call = inject('call');
const appointmentStore = useAppointmentStore();
const bookingViewStore = useBookingViewStore();
const bookingModalStore = useBookingModalStore();
const { status: appointmentStatus } = storeToRefs(appointmentStore);
const { status: appointmentStatus } = appointmentStore;
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure why but this was breaking the bookingView for me. Probably because that function isn't a ref to begin with, but I don't know how it worked before..

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's weird, indeed it shouldn't have worked before, since status is just a function. On the other hand, storeToRefs might be able to convert that, but the docs are a bit vague on that in composition mode... 🤷🏻

@MelissaAutumn MelissaAutumn changed the title Adjustments to create "Busy" blocks in a user's schedule: Create "Busy" blocks in a user's schedule Feb 28, 2024
Copy link
Collaborator

@devmount devmount left a comment

Choose a reason for hiding this comment

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

On my end, the busy slot was shown as a selectable time slot on the booking page 😅 Even after saving the schedule config again. Anything I need to do before seeing the blocker?

image

Can you reproduce this?

backend/test/unit/test_calendar_tools.py Outdated Show resolved Hide resolved
frontend/src/definitions.js Outdated Show resolved Hide resolved
frontend/src/definitions.js Outdated Show resolved Hide resolved
@MelissaAutumn
Copy link
Member Author

On my end, the busy slot was shown as a selectable time slot on the booking page 😅 Even after saving the schedule config again. Anything I need to do before seeing the blocker?

image

Can you reproduce this?

I cannot, but can you check your response to see if the booking_status for that event is 3? And can you try clearing your cache (it shouldn't be affected because I think vite hashes js/css but just in case!)

@MelissaAutumn MelissaAutumn force-pushed the main branch 2 times, most recently from 0e75e8b to 73d348b Compare March 1, 2024 18:09
@devmount
Copy link
Collaborator

devmount commented Mar 4, 2024

I cannot, but can you check your response to see if the booking_status for that event is 3? And can you try clearing your cache (it shouldn't be affected because I think vite hashes js/css but just in case!)

Ok maybe it really was a caching issue, after logging in again I cannot reproduce that anymore 😇 Case closed. If I see that again, I'll create an issue.

Copy link
Collaborator

@devmount devmount left a comment

Choose a reason for hiding this comment

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

I'm so sorry, but now I found some issues again 🙈

  1. I got the first issue with blockers styled as selectable events again. But as soon as I wanted to inspect that, the blockers showed up again! Sneaky little bug. I get back to that as soon as I have some debugging information on this.
  2. There seems to be some issues with calculating the start blockers now. I have a blocker shown on the booking page where none is supposed to be:

My Google Calendar:
image

My booking page:
image
image

The events array with the two blocker objects opened:
image

Any ideas? I'll play around with the blocker calculation. I feel we should do some intense testing on that method again and comment every little step since that seems kind of complicated.

@MelissaAutumn
Copy link
Member Author

Thanks for finding that! Whats your scheduled booking hours? And do you have any other calendars connected to Appointment? It uses all calendars to determine blockers (but only the schedule calendar to book on.)

@MelissaAutumn
Copy link
Member Author

I think I've found the bug while working on a separate issue. Will poke into it more.

@MelissaAutumn
Copy link
Member Author

The mystery blocker at 8am could be a stray booked slot. I'll add some functionality to restrict this logic to run only between the schedule's available booking time.

@MelissaAutumn MelissaAutumn force-pushed the features/277-fill-in-busy-time-ga branch from 237bd6e to f66d3f3 Compare March 5, 2024 21:29
MelissaAutumn and others added 4 commits March 5, 2024 13:31
* Adjust event_set_difference to event_roll_up_difference - This now rolls up any collisions as a "booked" slot.
* Create a replica enum of slot's booking status on the frontend
* Style "booked" bookable slots to not be bookable, clickable, or friendly looking.
* Locale update!
* Remove storeToRefs for a function that seems to be breaking.
* Hide agenda view on mobile month, because we can't style it right now.
* Add a hover effect to change cursor, and highlight the day in mobile month view.
* Fix case where event_roll_up_difference would not roll up trailing events.
* Add test case for events_roll_up_difference.
@MelissaAutumn MelissaAutumn force-pushed the features/277-fill-in-busy-time-ga branch from f66d3f3 to 483a0a8 Compare March 5, 2024 21:32
@devmount
Copy link
Collaborator

Thanks for finding that! Whats your scheduled booking hours? And do you have any other calendars connected to Appointment? It uses all calendars to determine blockers (but only the schedule calendar to book on.)

Sorry for the late reply. This is my schedule config:

image

And I only have one calendar, the google calendar, connected.

@MelissaAutumn MelissaAutumn force-pushed the features/277-fill-in-busy-time-ga branch from 751c93a to 9e6affe Compare March 18, 2024 22:37
@MelissaAutumn
Copy link
Member Author

MelissaAutumn commented Mar 18, 2024

PR is up to the current main code.

Additional issues found:
All day events just leave the day blank
image

I have a busy block from 1am to 9??
image

Confirmed bug: When there's a blocker at end of day, it will roll over to the next day before the next available slot. Adding test, and will fix once it's broken in test.

@MelissaAutumn
Copy link
Member Author

...okay there's some issues with some of our test libraries that prevents me from creating a digestable unit test. Will work on that, but I've pushed up a fix for the issue.

@devmount can you test the latest push?

@MelissaAutumn MelissaAutumn requested a review from devmount March 20, 2024 15:47
Copy link
Collaborator

@devmount devmount left a comment

Choose a reason for hiding this comment

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

No event glitches anymore, great work @MelissaAutumn!

This is how it looks on my end now:

image

I'm still not sure if I really like those "busy" blocks, I found it kind of cleaner to only show selectable events 🤷🏻 And is it correct, that busy blocks show at the beginning of the day, but not at the end?

@MelissaAutumn
Copy link
Member Author

Not showing at the end is also a bug...back to the busy mines I go! But yeah, they do look a little tough, but I'm sure we can soften the styling a bit. Maybe make them smaller than the bookable slots. 🤔

I'll play around with it early next week.

@devmount
Copy link
Collaborator

It's fine, really! I do see the value of having the whole working hours area filled with either selectable slots or blockers. Thank for your work on this! 💪🏻 👏🏻

@MelissaAutumn
Copy link
Member Author

Oh no worries! It should at least look a little less heavy. 😄

@MelissaAutumn
Copy link
Member Author

cc @devmount for one last check. I re-wrote the roll up function, might be a little slower but it's simpler-ish now. Please test to see if it still works as expected. (Looks fine to me, and the tests.)

Here's the updated styles to be a little less "in your face".
image
image

Copy link
Collaborator

@devmount devmount left a comment

Choose a reason for hiding this comment

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

Nice work Mel! Edge event properly show up now on my end too 👏🏻 Also I like the less prominent blocker entries 👍🏻

@MelissaAutumn MelissaAutumn merged commit 33cc02d into main Mar 26, 2024
2 checks passed
@MelissaAutumn MelissaAutumn deleted the features/277-fill-in-busy-time-ga branch March 26, 2024 15:18
jdbass pushed a commit that referenced this pull request May 17, 2024
* Adjustments to create "Busy" blocks in a user's schedule:

* Adjust event_set_difference to event_roll_up_difference - This now rolls up any collisions as a "booked" slot.
* Create a replica enum of slot's booking status on the frontend
* Style "booked" bookable slots to not be bookable, clickable, or friendly looking.
* Locale update!
* Remove storeToRefs for a function that seems to be breaking.
* Hide agenda view on mobile month, because we can't style it right now.
* Add a hover effect to change cursor, and highlight the day in mobile month view.
* Fix case where event_roll_up_difference would not roll up trailing events.
* Add test case for events_roll_up_difference.

* Fix pytest error with filenames.

* Add enum jsdoc properties to our defines.

* 🌐 complete German translation

* Add schedule store and pass the slot_duration info into qalendar.

* Add slot_duration to AppointmentOut schema, and hook it up for public view.

* Fix the egg.

* Fix booking redis cache clearing on booking code and fix test.

* Fix busy blocks sometimes being created before the schedule starts.

* Test blockers

* Fix missing .start

* Merge duplicate definition

* Disable `tailwindcss/no-custom-classname`

* Adjust styling of booked slots

* Refactor events_roll_up_difference:
* Make two lists
* Merge them together
* Sort
* Might be a little less performant, but it's simpler.

* Remove references to booking view.

---------

Co-authored-by: Andreas Müller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
l10n update A string has been added or needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fill in empty gaps for a user's busy slots on their GA.
2 participants