Skip to content

feat: add booking date range filters and freeze GPT-4 version#12508

Merged
PeerRich merged 10 commits intocalcom:mainfrom
RubricLab:main
Jan 16, 2024
Merged

feat: add booking date range filters and freeze GPT-4 version#12508
PeerRich merged 10 commits intocalcom:mainfrom
RubricLab:main

Conversation

@DexterStorey
Copy link
Copy Markdown
Contributor

@DexterStorey DexterStorey commented Nov 23, 2023

What does this PR do?

This PR adds optional filtering by date to the Cal.com consumer API, and integration with cal.ai.

It fixes a problem where many accounts can't access the getBookings route since they have historical buggy data - and the consumer API previously fetched all booking ever, leading to errors.

We now add the option to fetch booking within a range.

Partially Fixes #11957 (issue)

Additionally, this pr freezes the GPT-4 model (gpt-4-0613) version in an attempt to prevent model degradation.

Requirement/Documentation

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • 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?

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my PR needs changes to the documentation
  • I haven't checked if my changes generate no new warnings
  • I haven't added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 23, 2023

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

A member of the Team first needs to authorize it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 23, 2023

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

@github-actions github-actions Bot added ai area: AI, cal.ai api area: API, enterprise API, access token, OAuth Medium priority Created by Linear-GitHub Sync 🐛 bug Something isn't working labels Nov 23, 2023
@DexterStorey DexterStorey changed the title Feat: Add booking date range filters feat: add booking date range filters Nov 23, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 23, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 8, 2023

This PR is being marked as stale due to inactivity.

@github-actions github-actions Bot added the Stale label Dec 8, 2023
@github-actions
Copy link
Copy Markdown
Contributor

This PR is being closed due to inactivity. Please reopen if work is intended to be continued.

@github-actions github-actions Bot closed this Dec 15, 2023
@PeerRich PeerRich marked this pull request as ready for review December 15, 2023 04:40
@github-actions github-actions Bot removed the Stale label Dec 16, 2023
@DexterStorey
Copy link
Copy Markdown
Contributor Author

Tested and fixed!
This seems to work now.

When I go to: http://localhost:3002/bookings?dateFrom=2023-12-20T16:00:00Z&dateTo=2023-12-20T16:15:00Z&apiKey=API_KEY

I can see the booking:

Screenshot 2023-12-19 at 10 33 47 PM

But with: http://localhost:3002/bookings?dateFrom=2023-12-20T16:00:00Z&dateTo=2023-12-20T16:14:00Z&apiKey=API_KEY

It is hidden

Screenshot 2023-12-19 at 10 34 57 PM

With no dateFrom or dateTo passed, the api query responds normally, so in theory this is purely additive / optional.

http://localhost:3002/bookings?apiKey=API_KEY

Screenshot 2023-12-19 at 10 36 14 PM

@DexterStorey
Copy link
Copy Markdown
Contributor Author

DexterStorey commented Dec 20, 2023

🚀 This should fix #11957
Docs should probably be updated as we now have a new feature for the getBookings API (dateFrom, dateTo) which filters bookings by date.

@PeerRich PeerRich enabled auto-merge (squash) December 20, 2023 04:19
Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

Temporal filters are quite handy, thank you for the PR 🙏
Left a change request which I think should improve code quality

Comment thread apps/api/pages/api/bookings/_get.ts Outdated
auto-merge was automatically disabled December 20, 2023 18:43

Head branch was pushed to by a user without write access

Comment thread apps/api/pages/api/bookings/_get.ts
Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

Just a NIT, although your changes should are already better, this would just be much cleaner and easier to read. 🙌

Comment thread apps/api/pages/api/bookings/_get.ts
exception
exception previously approved these changes Dec 21, 2023
alishaz-polymath

This comment was marked as outdated.

@alishaz-polymath alishaz-polymath dismissed their stale review December 21, 2023 12:40

Testing something

Comment thread apps/api/pages/api/bookings/_get.ts Outdated
Comment on lines +166 to +167
const dateFrom = req.query.dateFrom ? new Date(req.query.dateFrom as string) : undefined;
const dateTo = req.query.dateTo ? new Date(req.query.dateTo as string) : undefined;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Apologies for missing this earlier, I think we need to put these in try catch and return a viable error message instead of the generic

{
    "message": "An error occured while querying the database."
}

that you'd get right now if you pass invalid dateFrom and/or dateTo. Wrapping them up in a try catch, or better yet, pull out a reusable function that validates while parsing these inputs would be awesome 🙌

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once these changes are in, it should be good to go 🙏

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @DexterStorey did you manage to get a chance to have a look ^?

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.

Hi @alishaz-polymath. I pushed some changes! 🤝

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 5, 2024

This PR is being marked as stale due to inactivity.

@github-actions github-actions Bot added Stale and removed Stale labels Jan 5, 2024
@DexterStorey DexterStorey changed the title feat: add booking date range filters feat: add booking date range filters and freeze GPT-4 version Jan 8, 2024
Comment on lines +21 to +25
export const schemaBookingGetParams = z.object({
dateFrom: iso8601.optional(),
dateTo: iso8601.optional(),
});

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.

support schema parsing for the bookings get api

async function handler(req: NextApiRequest) {
const { userId, isAdmin, prisma } = req;

const { dateFrom, dateTo } = schemaBookingGetParams.parse(req.query);
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.

@alishaz-polymath can you take a look at this?

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.

Screenshot 2024-01-08 at 3 35 00 PM

@github-actions github-actions Bot removed the Stale label Jan 9, 2024
@keithwillcode keithwillcode added this to the v3.7 milestone Jan 10, 2024
@keithwillcode keithwillcode added the community Created by Linear-GitHub Sync label Jan 11, 2024
import now from "./now";

const gptModel = "gpt-4";
const gptModel = "gpt-4-0613";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any specific reason why we picked this Model exaclty? Additional context would be really helpful

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.

It's the latest stable version released on June 13 AKA 0613 (:
https://platform.openai.com/docs/models/gpt-4-and-gpt-4-turbo

Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

Looks good, thank you @DexterStorey 🚀

@keithwillcode keithwillcode modified the milestones: v3.7, v3.8 Jan 15, 2024
@PeerRich PeerRich merged commit fd3fffd into calcom:main Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai area: AI, cal.ai api area: API, enterprise API, access token, OAuth 🐛 bug Something isn't working community Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issues with bad data -> gaps in the consumer api exposed by cal.ai

5 participants