Skip to content

fix: Creating teams for organizations in wizard#9632

Merged
leog merged 3 commits into
mainfrom
fix/create-teams-org
Jun 19, 2023
Merged

fix: Creating teams for organizations in wizard#9632
leog merged 3 commits into
mainfrom
fix/create-teams-org

Conversation

@leog
Copy link
Copy Markdown
Contributor

@leog leog commented Jun 19, 2023

What does this PR do?

When added the possibility to register an org checking if billing was enabled, we introduced a ramification where requestedSlug metadata property was not set but we relied on it to let the user create new teams. Expanding that check to cover the other scenario as well.

Type of change

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

How should this be tested?

Go to the org wizard and create teams, it should let you do it in a non-team-billing environment scenario.

Mandatory Tasks

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

@leog leog requested a review from a team June 19, 2023 13:55
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 19, 2023

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

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2023 3:00pm
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2023 3:00pm
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2023 3:00pm
web-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2023 3:00pm

"my_profile": "My Profile",
"my_settings": "My Settings",
"sender_id_info": "Name or number shown as the sender of an SMS (some countries do not allow alphanumeric sender IDs)",
"no_organization_slug": "There was an error creating teams for this organization. Missing URL slug.",
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.

Adding a proper message for the error to not show coded to the end user

},
onError: (error) => {
showToast(error.message, "error");
showToast(t(error.message), "error");
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.

Show the translated message to the end user, not the code

...(!IS_TEAM_BILLING_ENABLED && { slug }),
metadata: {
requestedSlug: slug,
...(IS_TEAM_BILLING_ENABLED && { requestedSlug: slug }),
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.

Adding the check to not have the metadata if we are not in a team-billing environment

const organization = await prisma.team.findFirst({ where: { id: orgId }, select: { metadata: true } });
const organization = await prisma.team.findFirst({
where: { id: orgId },
select: { slug: true, metadata: true },
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.

Be sure to also get slug from the newly created org to check for it too

const metadata = teamMetadataSchema.parse(organization?.metadata);

if (!metadata?.requestedSlug) throw new TRPCError({ code: "BAD_REQUEST", message: "no_organization" });
if (!metadata?.requestedSlug || !organization?.slug)
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.

Not only rely on requestedSlug to show an error, when no team-billing is enabled, org slug is the one defined.

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.

I am still getting the error, should that be && maybe?

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.

Sharp eyes :)


if (!metadata?.requestedSlug) throw new TRPCError({ code: "BAD_REQUEST", message: "no_organization" });
if (!metadata?.requestedSlug || !organization?.slug)
throw new TRPCError({ code: "BAD_REQUEST", message: "no_organization_slug" });
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.

Making the error code more specific.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 19, 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! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Jun 19, 2023

Current Playwright Test Results Summary

✅ 108 Passing - ⚠️ 3 Flaky

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

(Last updated on 06/19/2023 02:36:54pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 0c313bc

Started: 06/19/2023 02:34:15pm UTC

⚠️ Flakes

📄   apps/web/playwright/login.2fa.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
2FA Tests should allow a user to disable 2FA
Retry 2Retry 1Initial Attempt
3.41% (10) 10 / 293 runs
failed over last 7 days
26.96% (79) 79 / 293 runs
flaked over last 7 days

📄   apps/web/playwright/event-types.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Event Types tests user -- old-booker can add multiple organizer address
Retry 1Initial Attempt
1.38% (4) 4 / 290 runs
failed over last 7 days
29.31% (85) 85 / 290 runs
flaked over last 7 days

📄   apps/web/playwright/webhook.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
FORM_SUBMITTED can submit a form and get a submission event
Retry 1Initial Attempt
4.15% (9) 9 / 217 runs
failed over last 7 days
15.67% (34) 34 / 217 runs
flaked over last 7 days

View Detailed Build Results


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.

minor unrelated nit - looks good

throw new TRPCError({ code: "BAD_REQUEST", message: "no_organization_slug" });

const userMembership = await prisma.membership.findFirst({
where: {
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.

Not directly related to this PR, but this needs a check to confirm the membership isn't pending

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.

Suggested change
where: {
where: {
accepted: true,

@leog leog enabled auto-merge (squash) June 19, 2023 14:26
Copy link
Copy Markdown
Member

@CarinaWolli CarinaWolli left a comment

Choose a reason for hiding this comment

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

Works now :)

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

Labels

None yet

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants