Skip to content

improve date query parsing (#1992)#2296

Merged
zomars merged 3 commits intocalcom:mainfrom
buschco:fix/1992
Mar 28, 2022
Merged

improve date query parsing (#1992)#2296
zomars merged 3 commits intocalcom:mainfrom
buschco:fix/1992

Conversation

@buschco
Copy link
Copy Markdown
Contributor

@buschco buschco commented Mar 27, 2022

What does this PR do?

Creating a new dayjs date, shows an offset of 2 hours whats makes sense rn since i live in Europe/Berlin Summer Time:
GMT+0200However, creating the date withdayjs(dateString.substr(11, 14), "Hmm");` will not work as expected (codesandbox playground):

dayjs("0200", "Hmm", "GMT").hour() 2
dayjs("0200", "Hmm", "Europe/Amsterdam").hour() 3

Correct me If i am wrong, but using dayjs to parse this string could be the issue here, so I wrote it w/o dayjs.

Could fix #1992 and #2250

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Created an event in my apple calendar that should only allow one free slot. It showed the wrong time w/o the fix

Checklist

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

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 27, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

docs – ./apps/docs

🔍 Inspect: https://vercel.com/cal/docs/FYKGVivqcXYqrspbTmDM6JYKRay9
✅ Preview: Canceled

[Deployment for 33b0da1 canceled]

calendso – ./apps/web

🔍 Inspect: https://vercel.com/cal/calendso/AmKhqb1Ac4Zc3MFJjZEzsKv7Hjsr
✅ Preview: https://calendso-git-fork-buschco-fix-1992-cal.vercel.app

[Deployment for 33b0da1 failed]

@vercel vercel bot temporarily deployed to Preview – docs March 27, 2022 14:06 Inactive
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 27, 2022

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

A member of the Team first needs to authorize it.

@PeerRich
Copy link
Copy Markdown
Member

thank you! we will take a look and test (@emrysal @zomars @alannnc)

@vercel vercel bot temporarily deployed to Preview – calendso March 27, 2022 14:31 Inactive
@vercel vercel bot temporarily deployed to Preview – docs March 27, 2022 20:18 Inactive
@vercel vercel bot temporarily deployed to Preview – docs March 28, 2022 18:26 Inactive
dateString.substr(10, 1) + (utcOffsetAsDate.hour() * 60 + utcOffsetAsDate.minute())
);
const date = dayjs(dateString.substr(0, 10)).utcOffset(utcOffset, true);
const offsetString = dateString.substr(11, 14); // hhmm
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor nitpick but it seems the the substr method is deprecated:

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah you are right. I didn't want to change to much of the code, since substring is not a 1:1 replacement for substr
👍

@vercel vercel bot temporarily deployed to Preview – calendso March 28, 2022 18:48 Inactive
@zomars zomars merged commit cc5537d into calcom:main Mar 28, 2022
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.

Apple Calendar events seems to be handled with the wrong timezone

3 participants