-
Notifications
You must be signed in to change notification settings - Fork 7k
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 signup not working #13717
Conversation
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
@@ -235,10 +235,11 @@ export async function createNewUsersConnectToOrgIfExists({ | |||
const usernameOrEmail = usernamesOrEmails[index]; | |||
const { orgId, autoAccept } = connectionInfoMap[usernameOrEmail]; | |||
const [emailUser, emailDomain] = usernameOrEmail.split("@"); | |||
const username = | |||
emailDomain === autoAcceptEmailDomain | |||
const username = input.isOrg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a non sub-team invite, we shouldn't set username during the invite as user is allowed to change the username during the signup.
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 4 Ignored Deployments
|
08c3f17
to
3093e71
Compare
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
emailDomain === autoAcceptEmailDomain | ||
? slugify(emailUser) | ||
: slugify(`${emailUser}-${emailDomain.split(".")[0]}`); | ||
input.isOrg || parentId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensures that for a regular team the username isn't set because user is allowed to change the username during signup in that context.
Current Playwright Test Results Summary✅ 63 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 02/16/2024 12:49:15pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 7284929 Started: 02/16/2024 12:46:48pm UTC
|
|
2 Test Cases Affected |
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Update Profile Cannot update a users email when existing user has same email (verification enabled)
Retry 1 • Initial Attempt |
0.39% (1)1 / 256 runfailed over last 7 days |
49.61% (127)127 / 256 runsflaked over last 7 days |
Update Profile Can update a users email (verification enabled)
Retry 2 • Retry 1 • Initial Attempt |
12.11% (31)31 / 256 runsfailed over last 7 days |
51.56% (132)132 / 256 runsflaked over last 7 days |
3093e71
to
2400067
Compare
2400067
to
b9ed91c
Compare
Graphite AutomationsA Graphite automation took an action on this PR • (02/16/24)1 reviewer was added based on Keith Williams's automation, 'Add consumer team as reviewer' |
emailDomain === autoAcceptEmailDomain | ||
? slugify(emailUser) | ||
: slugify(`${emailUser}-${emailDomain.split(".")[0]}`); | ||
|
||
// As a regular team member is allowed to change username during signup, we don't set any username for him | ||
const regularTeamMemberUsername = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we accidentally started setting it for regular teams by this PR
https://github.com/calcom/cal.com/pull/11238/files
cc @joeauyeung for confirmation
ed388d9
to
98ccaba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Regular Team invites are working fine.
Checking orgs now
* Fix team invite signup not working * Add unit test --------- Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
What does this PR do?
Fixes #13719
Type of change
How should this be tested?
Mandatory Tasks
Checklist
NOTE: It isn't possible to test this flow using e2e