Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: team invite links #10110

Merged
merged 2 commits into from
Jul 12, 2023

Conversation

zomars
Copy link
Member

@zomars zomars commented Jul 12, 2023

What does this PR do?

Fixes #10106

Type of change

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

How should this be tested?

  • Create a team and bulk invite new members, on the DB all should have the teamID properly set.
  • Use the copy invite link button. There should be only one active token for it.

Mandatory Tasks

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

Checklist

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

@linear
Copy link

linear bot commented Jul 12, 2023

CAL-2160 Team invites aren't working for non-users

Found a bug? Please fill out the sections below. 👍

Issue Summary

Introduced in #8355

Seems like invites to external users aren't being sent when there's more than one.

Steps to Reproduce

  1. Create a team
  2. Invite a non-Cal user (all good so far)
  3. Try inviting another non-Cal user

Any other relevant information. For example, why do you consider this a bug and what did you expect to happen instead?

The current DB schema has a wrong unique constrain on teamID for VerificationToken, also seems like a Team can only have 1 active VerificationToken at a time.

Actual Results

  • There can only be 1 active invite at a time per team

Expected Results

  • A team should be able to have multiple invite tokens active

Technical details

  • Browser version, screen recording, console logs, network requests: You can make a recording with Bird Eats Bug.
  • Node.js version
  • Anything else that you think could be an issue.

Evidence

Created a team and bulk-invited 3 new memebers. Only the last one had a relationship with the team.

Source image

Invited a new one and this one got the relationship with the team now. So now previous invites are stuck in limbo.

Source image

@vercel
Copy link

vercel bot commented Jul 12, 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 Jul 12, 2023 8:37pm
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 12, 2023 8:37pm
cal-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 12, 2023 8:37pm
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 12, 2023 8:37pm

@github-actions github-actions bot added High priority Created by Linear-GitHub Sync 🐛 bug Something isn't working ❗️ migrations contains migration files and removed 🐛 bug Something isn't working High priority Created by Linear-GitHub Sync labels Jul 12, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 12, 2023

Thank you for following the naming conventions! 🙏

Copy link
Member Author

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Ready for review

@@ -83,7 +82,7 @@ export default function Signup({ prepopulateFormValues, token, orgSlug }: Signup
};

return (
<LicenseRequired>
Copy link
Member Author

Choose a reason for hiding this comment

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

LicenseRequired only works for logged in users. All users landing will be logged off.

Copy link
Member

Choose a reason for hiding this comment

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

we dont need this for this component anyway. great catch!

@@ -114,7 +115,16 @@ export async function getTeamWithMembers(id?: number, slug?: string, userId?: nu
...eventType,
metadata: EventTypeMetaDataSchema.parse(eventType.metadata),
}));
return { ...team, metadata: teamMetadataSchema.parse(team.metadata), eventTypes, members };
/** Don't leak invite tokens to the frontend */
const { inviteTokens, ...teamWithoutInviteTokens } = team;
Copy link
Member Author

Choose a reason for hiding this comment

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

Also to keep previous frontend signature

@@ -0,0 +1,2 @@
-- DropIndex
DROP INDEX "VerificationToken_teamId_key";
Copy link
Member Author

Choose a reason for hiding this comment

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

A team can have multiple tokens. Dropped the unique requirement

@@ -270,7 +270,7 @@ model Team {
parent Team? @relation("organization", fields: [parentId], references: [id], onDelete: Cascade)
children Team[] @relation("organization")
orgUsers User[] @relation("scope")
inviteToken VerificationToken?
inviteTokens VerificationToken[]
Copy link
Member Author

Choose a reason for hiding this comment

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

A team may have multiple tokens

@@ -310,7 +310,7 @@ model VerificationToken {
expiresInDays Int?
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt
teamId Int? @unique
teamId Int?
Copy link
Member Author

Choose a reason for hiding this comment

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

This was causing that only the most recent invite had the proper team relationship.

@@ -22,7 +22,7 @@ export const createInviteHandler = async ({ ctx, input }: CreateInviteOptions) =
const token = randomBytes(32).toString("hex");
await prisma.verificationToken.create({
data: {
identifier: "",
identifier: "invite-link-for-teamId-" + teamId,
Copy link
Member Author

Choose a reason for hiding this comment

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

Only to distinguish the copy link token VS the each individual email.

@alwaysmeticulous
Copy link

alwaysmeticulous bot commented Jul 12, 2023

🤖 Meticulous spotted visual differences in 46 of 180 screens tested: view and approve differences detected.

Last updated for commit c4b4892. This comment will update as new commits are pushed.

@github-actions
Copy link
Contributor

📦 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

deploysentinel bot commented Jul 12, 2023

Current Playwright Test Results Summary

✅ 88 Passing - ⚠️ 2 Flaky

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

(Last updated on 07/12/2023 08:42:23pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: c4b4892

Started: 07/12/2023 08:40:35pm UTC

⚠️ Flakes

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Managed Event Types tests Can create managed event type
Retry 2Retry 1Initial Attempt
0.88% (3) 3 / 339 runs
failed over last 7 days
12.09% (41) 41 / 339 runs
flaked over last 7 days

📄   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 1Initial Attempt
2.38% (1) 1 / 42 run
failed over last 7 days
97.62% (41) 41 / 42 runs
flaked over last 7 days

View Detailed Build Results


Copy link
Contributor

@alannnc alannnc left a comment

Choose a reason for hiding this comment

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

Code looks good!

@zomars zomars merged commit 9b37d65 into main Jul 12, 2023
32 of 33 checks passed
@zomars zomars deleted the zomars/cal-2160-team-invites-arent-working-for-non-users branch July 12, 2023 21:24
fritterhoff pushed a commit to hm-edu/cal.com that referenced this pull request Jul 25, 2023
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 ❗️ migrations contains migration files Urgent Created by Linear-GitHub Sync
Projects
No open projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[CAL-2160] Team invites aren't working for non-users
4 participants