Skip to content

feat: support slug and id on GET event-types#12491

Closed
ouwargui wants to merge 2 commits intocalcom:mainfrom
ouwargui:feat/event-type-by-slug/CAL-2745
Closed

feat: support slug and id on GET event-types#12491
ouwargui wants to merge 2 commits intocalcom:mainfrom
ouwargui:feat/event-type-by-slug/CAL-2745

Conversation

@ouwargui
Copy link
Copy Markdown
Contributor

@ouwargui ouwargui commented Nov 22, 2023 •
edited
Loading

What does this PR do?

Fixes #12411
/claim #12411

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?

  • Are there environment variables that should be set?
    • No
  • What are the minimal test data to have?
    • yarn db-seed
  • What is expected (happy path) to have (input and output)?
    • /api/event-types/:id should support id or slug as path param

Mandatory Tasks

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

Checklist

  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't added tests that prove my fix is effective or that my feature works

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 22, 2023

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

A member of the Team first needs to authorize it.

@algora-pbc algora-pbc Bot mentioned this pull request Nov 22, 2023
Closed
@algora-pbc
Copy link
Copy Markdown

algora-pbc Bot commented Nov 22, 2023

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe/Alipay.

@github-actions github-actions Bot added api area: API, enterprise API, access token, OAuth Medium priority Created by Linear-GitHub Sync ✨ feature New feature or request 💎 Bounty A bounty on Algora.io labels Nov 22, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 22, 2023 •
edited
Loading

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 22, 2023 •
edited
Loading

📦 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! 🙌

@aecorredor
Copy link
Copy Markdown

aecorredor commented Nov 27, 2023 •
edited
Loading

Cross-referencing this PR in #12392. If the CSRF fix gets merged before this PR, it'll break anyone who was using the public TRPC endpoints as a workaround for getting an event type by slug.

@aecorredor aecorredor mentioned this pull request Nov 27, 2023
Closed
6 tasks
@CarinaWolli CarinaWolli requested a review from a team December 7, 2023 21:04
emrysal
emrysal previously approved these changes Dec 7, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

Thank you, excellent!

emrysal
emrysal previously approved these changes Dec 7, 2023
View reviewed changes
@emrysal emrysal enabled auto-merge (squash) December 7, 2023 22:44
hariombalhara
hariombalhara reviewed Dec 8, 2023
View reviewed changes
Comment thread apps/api/pages/api/event-types/[id]/_auth-middleware.ts
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara Dec 8, 2023

Choose a reason for hiding this comment

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

We should rename the path to [idOrSlug]

hariombalhara
hariombalhara reviewed Dec 8, 2023
View reviewed changes
Comment thread apps/api/pages/api/event-types/[id]/_auth-middleware.ts Outdated
if (isAdmin) return;
const eventType = await prisma.eventType.findFirst({
where: { id, users: { some: { id: userId } } },
where: { OR: [{ id }, { slug }], AND: { users: { some: { id: userId } } } },
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara Dec 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Because it would either be id or slug when provided by user, we want to be clear if we are fetching by slug or id so using OR doesn't make sense.

Also, there could possibly be a scenario where an event-type has a numeric slug(say 123). In this case querying for event-types/123 could potentially fetch the eventType with that numeric slug. So, we need to be able to distinguish it clearly in the API request as to what the user wants.

Maybe we could go for /event-types?slug=something or a new endpoint maybe /event-types/bySlug/[slug] ?

hariombalhara
hariombalhara requested changes Dec 8, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

I think we should go for a change in logic a bit.

@emrysal emrysal dismissed their stale review December 8, 2023 17:23

In agreement with @hariom. Needs different solution.

auto-merge was automatically disabled December 18, 2023 23:33

Head branch was pushed to by a user without write access

@ouwargui ouwargui force-pushed the feat/event-type-by-slug/CAL-2745 branch from 7add76d to 9c4d547 Compare December 18, 2023 23:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 2, 2024

This PR is being marked as stale due to inactivity.

@github-actions github-actions Bot added the Stale label Jan 2, 2024
@ouwargui
Copy link
Copy Markdown
Contributor Author

ouwargui commented Jan 2, 2024

not stale, i didn't have time to work on it because of the holidays, but i'm back

@github-actions github-actions Bot removed the Stale label Jan 3, 2024
@keithwillcode keithwillcode added this to the v3.8 milestone Jan 10, 2024
@keithwillcode keithwillcode added the community Created by Linear-GitHub Sync label Jan 11, 2024
@keithwillcode
Copy link
Copy Markdown
Contributor

keithwillcode commented Jan 20, 2024

Closing as this PR now has 0 changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hariombalhara hariombalhara hariombalhara requested changes

@emrysal emrysal emrysal left review comments

Assignees

No one assigned

Labels

api area: API, enterprise API, access token, OAuth 🙋 Bounty claim 💎 Bounty A bounty on Algora.io community Created by Linear-GitHub Sync ✨ feature New feature or request Medium priority Created by Linear-GitHub Sync

Projects

None yet

Milestone

v3.8

Development

Successfully merging this pull request may close these issues.

[CAL-2745] API for getting an event type by slug

5 participants

@ouwargui @aecorredor @keithwillcode @emrysal @hariombalhara