Skip to content

fix: team booking page having same slug as the org#12213

Merged
hariombalhara merged 4 commits into
mainfrom
11-02-Fix_team_with_same_name_as_slug_not_loading
Nov 14, 2023
Merged

fix: team booking page having same slug as the org#12213
hariombalhara merged 4 commits into
mainfrom
11-02-Fix_team_with_same_name_as_slug_not_loading

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara commented Nov 3, 2023

What does this PR do?

Fixes the bug where depending on which team would be fetched first by prisma(which is usually last-modified first), sometimes Org is served for a team page because of slug confusion

Also added a repository for team to kickoff moving to repository pattern for querying.

Motivation to move to the pattern along with this bug fix

  • The issue arose because we can't query the team and org by slug reliably because slugs aren't unique in table, so we need well tested code to handle that querying thing
  • Going forward we plan to move metadata.isOrganization to a column so that querying is faster and more reliable and then we would just need to modify the repository code.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Mandatory Tasks

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

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 14, 2023 10:38am
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 14, 2023 10:38am
cal-demo 🔄 Building (Inspect) Visit Preview 💬 Add feedback Nov 14, 2023 10:38am
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 14, 2023 10:38am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Nov 14, 2023 10:38am
qa ⬜️ Ignored (Inspect) Visit Preview Nov 14, 2023 10:38am
ui ⬜️ Ignored (Inspect) Visit Preview Nov 14, 2023 10:38am

@hariombalhara
Copy link
Copy Markdown
Member Author

hariombalhara commented Nov 3, 2023

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 3, 2023

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 3, 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! 🙌

@hariombalhara hariombalhara changed the title Fix team with same name as slug not loading fix: team booking page having same slug as the org Nov 3, 2023
@hariombalhara hariombalhara force-pushed the 11-02-Fix_team_with_same_name_as_slug_not_loading branch from 351535c to a83d319 Compare November 3, 2023 12:23
@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Nov 3, 2023

Current Playwright Test Results Summary

✅ 13 Passing - ⚠️ 1 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 11/14/2023 11:43:50am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 8754bee

Started: 11/14/2023 11:42:45am UTC

⚠️ Flakes

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should be able to reschedule
Retry 2Retry 1Initial Attempt
16.13% (45) 45 / 279 runs
failed over last 7 days
79.57% (222) 222 / 279 runs
flaked over last 7 days

View Detailed Build Results


Comment thread packages/lib/server/repository/team.ts
@keithwillcode keithwillcode requested a review from a team November 3, 2023 15:59
if (!team) return null;
// HACK: I am not sure how to make Prisma in peace with TypeScript with this repository pattern
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return team as any;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is a return type on function already that should ensure that the function callers would get typesafey as long as this function is well tested.

@hariombalhara hariombalhara force-pushed the 11-02-Fix_team_with_same_name_as_slug_not_loading branch from 59bc6d7 to 04164be Compare November 3, 2023 16:13
// In cases where there are many teams with the same slug, we need to find out the one and only one that matches our criteria
.filter((team) => {
// We need an org if isOrgView otherwise we need a team
return isOrg ? team.metadata?.isOrganization : !team.metadata?.isOrganization;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is what fixes the bug. So, even if we get wrong result, we validate it here and then reject it.

@hariombalhara hariombalhara force-pushed the 11-02-Fix_team_with_same_name_as_slug_not_loading branch from 04164be to e2fd525 Compare November 3, 2023 16:17
@hariombalhara hariombalhara marked this pull request as ready for review November 3, 2023 16:17
Comment thread packages/lib/server/queries/teams/index.ts
Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

Edit: can be fixed in another PR

Screen.Recording.2023-11-07.at.9.06.46.PM.mov

I tried creating a team with same slug as my org and it was successfull but on the next page (onboard-members) I get an error. teams.create route returned already existing org instead of creating a team

@hariombalhara
Copy link
Copy Markdown
Member Author

Edit: can be fixed in another PR

Screen.Recording.2023-11-07.at.9.06.46.PM.mov
I tried creating a team with same slug as my org and it was successfull but on the next page (onboard-members) I get an error. teams.create route returned already existing org instead of creating a team

Thanks I will take it up later.

Udit-takkar
Udit-takkar previously approved these changes Nov 14, 2023
Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

LGTM.
Great work again in using the repository pattern here!

@hariombalhara hariombalhara merged commit 8c2ce97 into main Nov 14, 2023
@hariombalhara hariombalhara deleted the 11-02-Fix_team_with_same_name_as_slug_not_loading branch November 14, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only High priority Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants