Full support for multi-day events

kimadactyl commented 2 years ago

User story

As someone who hosts multi day events, I want to be able to upload things spanning multiple days, so that they show up properly on each day they are active

Acceptance criteria

[ ] Event index view shows events on every day they are active
[ ] Event index view shows it the duration as the whole event length (i.e. "3 days" not "24 hours")
[x] Events under 24 hours show up as an hour and minute count (same behaviour as before)
[ ] Events that are 24 or more hours show up as a rounded up whole number of days (25 hours is 2 days, 48 hours is 2 days)
[ ] Events like clubnights that cross a day boundary but are less than 24h show up as a number of hours properly (so they are in 2 diff days, but still have a normal amount of hours)
[x] Event show view shows the day range and start and end time for the whole event

Implementation notes & questions

Implementation plan

The find_by_day scope is amended to include multi-day events
I can think of two ways to amend the UI.

We remove the "1 day" bit and have the time listing be something like 12th July 12:00 - 14th July 13:00. Then for an ordinary event where the end date is on the same day we have it just say 12th July 12:00 - 13:00
We alter the "1 day" bit to show the correct number of days

kimadactyl commented 2 years ago

This is sort of silenty failing right now, for example this event is 2 day but is showing up as 8-11pm

https://gmsc.placecal-staging.org/events/313526 https://www.eventbrite.co.uk/e/fake-test-event-1-tickets-376363733057

ivankocienski commented 2 years ago

This seems to touch a number of areas

[ ] show event event component
[ ] events index event_list component
[ ] action filter filter_events

To support multi day events the find method will have to be updated so we can do overlaps:

[ ] event contained entirely within search range
[ ] event starts in search range but ends after search range
[ ] event starts before search range but stops within search range
[ ] event starts before search range and ends after search range

katjam commented 2 years ago

I guess it's complicated because times on days may differ for different kinds of events?

Is time always required - or can an event be listed with no time specified. An multi-day events, can different days have different times? If an event is all day do we list it as midnight -> midnight?

What is the simplest interface that works in all cases? And requires least amount of exceptions / code to be "correct" and easy to read

Keep times as is (assuming people have to specify one time per event)
Keep timer section but call it "All day" rather than "1 day"
Display a range of dates like 25 July - 27 July in the date section

If times can be different per day then suggest these might not be the same event and have 2 entries instead? Or add multiple rows to that listing so you could have:

14:00 - 16:00 | 2 hours | 25 July 14:00 - 18:00 | 4 hours | 28 July

lexiwitch commented 2 years ago

Yeah, currently it's the case that we get back:

The start time and date of the event
The end time and date of the event
If the event is recurring and how often it recurs

That makes this kind of awkward to parse out multiday events on our end. I can think of (but unfortunately dont have the energy to list) about four different kinds of events that could fit into the format that we get and we don't really have a clean way of distinguishing them. Like -- the event I tested this against was listed incorrectly by the events organizer. It's supposed to occur only twice a month -- once on the start datetime, and once on the end datetime. But they put in an event range to Eventbrite, and the end time and start time are mixed in with the start date and end date, so IMO it is a) not our problem to fix and, even if it was, b) outside of the scope of this ticket.

Currently the display has been amended to show something like:

start time - end time | X day(s) | start date - end date

because that's the simplest way that we can deal with this without like a UI overhaul and mocking up alternative ways to show this data -- I'm down for that but that should be part of a different ticket.

lexiwitch commented 2 years ago

With respect to the acceptance criteria as it stands:

Events show up on every day they are active This is now true
The duration shows something sensible - a 24 hour event is not "1 day" I agree with this but I'm not sure how to fulfill this criteria? A 24 hour event is very literally "1 day", that feels like it's an edge case and the scope of like, what to put instead, and what edge cases we want to support should have been part of the ticket description.

like - if an event lasts from 00:00 Monday to 23:59 Tuesday, do we list that as "two days" or do we list that as 48 hours? What's the criteria for the change in representation? Do we have it say "X day(s)" normally for an event like that but have a special single edge cases for an event that happens to last 24 hours? what if the event is 23 hours instead lol, does that count as "1 day" or like something else?

It doesn't say to change it to hours, but I just noticed that you did that implicitly -- I think that's a good idea and if @kimadactyl thinks that satisfies the requirements then :slightly_smiling_face::+1:

Event view shows the day range and times for each day It does show the date range and the day range, but it doesn't do so individually for each day -- in this case that would be a huge chunk of the page UI displaying that unless we built it in a certain way, because the event spans 20 days lol. I feel like if we want a change in design on the frontend then that needs to be a separate ticket with a spec for that design listed and maybe a mockup rather than just this, which is kind of vague and I can't tell if I've met the AC or not in this case.

I feel like, when I went into this ticket, it felt very understandable, and now im looking at the acceptance criteria trying to figure out if im matching it and im like - "oh what about xyz", and it feels significantly less understandable hahaha

katjam commented 2 years ago

Does not - show Event index view shows events on every day they are active https://trans-dimension.placecal-staging.org/events?sort=time&period=week&repeating=on#

Does not - Event index view shows it the duration as the whole event length (i.e. "3 days" not "24 hours")

Events that are 24 or more hours show up as a rounded up whole number of days (25 hours is 2 days, 48 hours is 2 days)

Events like clubnights that cross a day boundary but are less than 24h show up as a number of hours properly (so they are in 2 diff days, but still have a normal amount of hours)

Event show view shows the day range and start and end time for the whole event

But Then on the page for the 5th - it shows the event on the 4th as well, but shows under the 4th, not the 5th - might make more sense to not show the event listed under 4th on that page

katjam commented 2 years ago

These are passing AC:

Events under 24 hours show up as an hour and minute count

Same as before:

Event show view shows the day range and start and end time for the whole event

katjam commented 2 years ago

Re-opening as the majority of these AC have not been met.

katjam commented 2 years ago

Not sure on priority of fixing this until / unless we have a real use case. Hard to find examples in the wild and presumably there are many many ways to represent a multi-day event in source calendars.

kimadactyl commented 2 years ago

Yeah that's fair. The prob is they pop up rarely and when I notice it's usually too late. I will do my best to keep an eye out.

I would also note that this is suffering from no implementation plan really being written (I would describe what we have as implementation notes, not a plan). I think creating a test calendar in google cal or whatever with various configirations of multi-day events is prob a prerequisite next time it gets tackled.

kimadactyl commented 2 years ago

Maybe if we engaged art galleries and museums this would come up more -- although then there's a related question of do we actually want something that's on for 3 months showing up every day?

lexiwitch commented 2 years ago

Does not - Event index view shows it the duration as the whole event length (i.e. "3 days" not "24 hours")

Yes, I thought I'd mentioned that it doesn't do this -- it requires restructuring the way the events index is displayed (i.e. the events index and the events show pages both use the same element to write that bar, so to resolve this we would have to choose between:

duplicate the entire bar UI and helper and so on
pass a thing to determine if its being rendered by the index versus the events show page

But Then on the page for the 5th - it shows the event on the 4th as well, but shows under the 4th, not the 5th - might make more sense to not show the event listed under 4th on that page

This is weird, but could be a very simple fix I think?

katjam commented 2 years ago

@alexandria-gfsc Thanks for replying! I think we can shelve this for now anyway, but is useful to have notes if we pick it up again.

geeksforsocialchange / PlaceCal