search

GRA0007 / crab.fit

Enter your availability to find a time that works for everyone!
https://crab.fit
GNU General Public License v3.0
265 stars 37 forks source link

Saturday shows up twice #304

Open naikymen opened 1 year ago

naikymen commented 1 year ago

Description of the bug

When polling by week-day instead of specific dates, Saturday shows up twice if selected.

Sample: https://crab.fit/any-day-any-time-by-week-308952

Setup

Captura de Pantalla 2023-08-17 a la(s) 09 18 47

Issue

Captura de Pantalla 2023-08-17 a la(s) 09 20 51

To reproduce

  1. Go to crab.fit
  2. Select "by weekday"
  3. Select all weekdays and all times.
  4. Create
  5. Select any time on saturday

Expected behavior

Saturday should show up once, and selecting times on saturday should select only one saturday.

Additional information

Thanks for the awesome app, we love it.

WillNilges commented 9 months ago

I've seen this too on a self-hosted dev instance. It might have something to do with whether or not you adjust the time range. It didn't happen when I did 9-5 (the default) but it did when I did 8-8 or 9-8.

WillNilges commented 9 months ago

I think it has something to do with how the times wrap around (Maybe UTC?) I'm in EST and I noticed that it would act funny when I included 8PM.

image

I also noticed that there was a null between 12-31 and 01-01, which manifests itself here as a gap in the chart.

WillNilges commented 9 months ago

Reading the code some more, I think that null is a feature. I have a hacky idea for how to fix this: Deduplicate dates by checking if two days are a saturday if isSpecificDates is false.

WillNilges commented 9 months ago

Well, because now it's Sunday, Jan 6, 2024, the bug(s) no longer happens. But, I have been pawing through the code, trying to figure out if there isn't some way to fix this from the beginning, instead of having to modify the code that generates the chart.

So far, I've seen this app work just fine, save for this visual bug, so I'm a bit hesitant to mess with anything too low level, but this appears to be where the dates are chosen when you choose to make a weekly crab fit.

Wait, maybe it's here

WillNilges commented 9 months ago

Looking at that code, actually, I wonder if the best thing to do might be to find the nearest (probably future but it might not matter) Sunday, and use that instead of Temporal.Now. That might fix it 🤔

WillNilges commented 9 months ago

So as far as I can tell, the logic that actually creates the event is fine. It always generates 7 days. But in the editor, the 8th day shows up in the list of dates. I can't help but think that the dates are stored properly in the backend, and somewhere in the frontend, there's an off-by-one error or timezone nonsense that causes this.

I am like..... 63% sure that the problem is somewhere in convertDatesToTimes.ts