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

feat: email confirmation on change #13443

Merged
merged 35 commits into from
Feb 7, 2024
Merged

Conversation

sean-brydon
Copy link
Member

@sean-brydon sean-brydon commented Jan 29, 2024

Show old and new email in email confirmation box

Fixes: #8856

CleanShot.2024-02-05.at.11.57.05.mp4

Copy link

vercel bot commented Jan 29, 2024

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 Feb 7, 2024 10:13am
dev ❌ Failed (Inspect) Feb 7, 2024 10:13am
6 Ignored Deployments
Name Status Preview Comments Updated (UTC)
ai ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 10:13am
cal ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 10:13am
cal-demo ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 10:13am
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 10:13am
qa ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 10:13am
ui ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 10:13am

@github-actions github-actions bot added 2 points Created by SyncLinear.com authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in High priority Created by Linear-GitHub Sync 🐛 bug Something isn't working labels Jan 29, 2024
Copy link
Contributor

github-actions bot commented Jan 29, 2024

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

@keithwillcode keithwillcode added consumer core area: core, team members only labels Jan 29, 2024
@sean-brydon
Copy link
Member Author

@itsberkaya @ciaranha maybe we can come up with a proper design for this in the future - i think this is fine for now let me know if you want any changes here.

Copy link
Contributor

github-actions bot commented Jan 29, 2024

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

New Page Added

The following page was added to the bundle from the code in this PR:

Page Size (compressed) First Load % of Budget (350 KB)
/auth/verify-email-change 89.82 KB 277.96 KB 79.42%

Copy link

deploysentinel bot commented Jan 29, 2024

Current Playwright Test Results Summary

✅ 442 Passing - ⚠️ 24 Flaky

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

(Last updated on 02/07/2024 10:29:08am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: a3a1482

Started: 02/07/2024 10:17:54am UTC

⚠️ Flakes

📄   apps/web/playwright/booking/multipleEmailQuestion.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Multiple Email Question and Each Other Question Booking With Multiple Email Question and checkbox group Question Multiple Email required and checkbox group required
Retry 1Initial Attempt
1.93% (6) 6 / 311 runs
failed over last 7 days
6.43% (20) 20 / 311 runs
flaked over last 7 days

📄   apps/web/playwright/booking-seats.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking with Seats User can create a seated event (2 seats as example)
Retry 1Initial Attempt
0.39% (1) 1 / 256 run
failed over last 7 days
5.08% (13) 13 / 256 runs
flaked over last 7 days
Reschedule for booking with seats If rescheduled/cancelled booking with seats it should display the correct number of seats
Retry 1Initial Attempt
0% (0) 0 / 255 runs
failed over last 7 days
3.92% (10) 10 / 255 runs
flaked over last 7 days

📄   apps/web/playwright/booking/checkboxGroupQuestion.e2e.ts • 3 Flakes

Top 1 Common Error Messages

null

3 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Checkbox Group Question and Each Other Question Booking With Checkbox Group Question and Address Question Booking With Checkbox Group Question and checkbox Question Checkbox Group required and checkbox required
Retry 1Initial Attempt
0.33% (1) 1 / 304 run
failed over last 7 days
4.93% (15) 15 / 304 runs
flaked over last 7 days
Booking With Checkbox Group Question and Each Other Question Booking With Checkbox Group Question and Address Question Booking With Checkbox Group Question and Radio group Question Checkbox Group required and Radio group required
Retry 1Initial Attempt
0.34% (1) 1 / 298 run
failed over last 7 days
4.03% (12) 12 / 298 runs
flaked over last 7 days
Booking With Checkbox Group Question and Each Other Question Booking With Checkbox Group Question and Address Question Booking With Checkbox Group Question and select Question Checkbox Group required and select required
Retry 1Initial Attempt
0.68% (2) 2 / 296 runs
failed over last 7 days
5.07% (15) 15 / 296 runs
flaked over last 7 days

📄   apps/web/playwright/booking/allQuestions.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With All Questions Selecting and filling all questions as optional
Retry 1Initial Attempt
0.33% (1) 1 / 306 run
failed over last 7 days
5.23% (16) 16 / 306 runs
flaked over last 7 days

📄   apps/web/playwright/event-types.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Event Types tests -- future user can add multiple organizer address
Retry 1Initial Attempt
1.22% (4) 4 / 328 runs
failed over last 7 days
17.99% (59) 59 / 328 runs
flaked over last 7 days
Event Types tests -- legacy user Different Locations Tests Can add Link Meeting as location and book with it
Retry 1Initial Attempt
0% (0) 0 / 320 runs
failed over last 7 days
1.56% (5) 5 / 320 runs
flaked over last 7 days

📄   apps/web/playwright/booking/addressQuestione2e/addressQuestion.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Address Question and Each Other Question Booking With Address Question and multiselect Question Address required and multiselect text required
Retry 1Initial Attempt
0% (0) 0 / 309 runs
failed over last 7 days
6.47% (20) 20 / 309 runs
flaked over last 7 days

📄   apps/web/playwright/booking/phoneQuestion.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Phone Question and Each Other Question Booking With Phone Question and Address Question Booking With Phone Question and Multi email Question Phone and Multi email required
Retry 1Initial Attempt
0.33% (1) 1 / 305 run
failed over last 7 days
4.59% (14) 14 / 305 runs
flaked over last 7 days
Booking With Phone Question and Each Other Question Booking With Phone Question and Address Question Booking With Phone Question and Radio group Question Phone required and Radio group not required
Retry 2Retry 1Initial Attempt
0.33% (1) 1 / 301 run
failed over last 7 days
7.64% (23) 23 / 301 runs
flaked over last 7 days

📄   apps/web/playwright/organization/organization-invitation.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Organization Email not matching orgAutoAcceptEmail Org Invitation
Retry 1Initial Attempt
13.64% (42) 42 / 308 runs
failed over last 7 days
6.17% (19) 19 / 308 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 8 Flakes

Top 1 Common Error Messages

null

8 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should open embed iframe on click - Configured with light theme
Retry 1Initial Attempt
-2.57% (-8) -8 / 311 runs
failed over last 7 days
53.38% (166) 166 / 311 runs
flaked over last 7 days
Popup Tests should be able to reschedule
Retry 1Initial Attempt
-109.21% (-166) -166 / 152 runs
failed over last 7 days
109.21% (166) 166 / 152 runs
flaked over last 7 days
Popup Tests should open Routing Forms embed on click
Retry 1Initial Attempt
-96.73% (-148) -148 / 153 runs
failed over last 7 days
99.35% (152) 152 / 153 runs
flaked over last 7 days
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe according to system theme when no theme is configured through Embed API
Retry 1Initial Attempt
-102.01% (-152) -152 / 149 runs
failed over last 7 days
102.01% (152) 152 / 149 runs
flaked over last 7 days
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe according to system theme when configured with 'auto' theme using Embed API
Retry 1Initial Attempt
-102.01% (-152) -152 / 149 runs
failed over last 7 days
102.01% (152) 152 / 149 runs
flaked over last 7 days
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe(Booker Profile Page) with dark theme when configured with dark theme using Embed API
Retry 1Initial Attempt
-101.34% (-151) -151 / 149 runs
failed over last 7 days
101.34% (151) 151 / 149 runs
flaked over last 7 days
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe(Event Booking Page) with dark theme when configured with dark theme using Embed API
Retry 1Initial Attempt
-101.34% (-151) -151 / 149 runs
failed over last 7 days
101.34% (151) 151 / 149 runs
flaked over last 7 days
Popup Tests prendered embed should be loaded and apply the config given to it
Retry 1Initial Attempt
-101.34% (-151) -151 / 149 runs
failed over last 7 days
101.34% (151) 151 / 149 runs
flaked over last 7 days

📄   apps/web/playwright/profile.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

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 1Initial Attempt
0% (0) 0 / 1 runs
failed over last 7 days
100% (1) 1 / 1 run
flaked over last 7 days
Update Profile Can update a users email (verification enabled)
Retry 1Initial Attempt
54.55% (6) 6 / 11 runs
failed over last 7 days
9.09% (1) 1 / 11 run
flaked over last 7 days

📄   apps/web/playwright/settings/upload-avatar.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
UploadAvatar can upload an image
Retry 1Initial Attempt
0.63% (2) 2 / 316 runs
failed over last 7 days
11.39% (36) 36 / 316 runs
flaked over last 7 days

View Detailed Build Results


Copy link
Contributor

@keithwillcode keithwillcode left a comment

Choose a reason for hiding this comment

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

@sean-brydon the issue mentioned as being fixed here says that no confirmation email is sent to confirm the new email is correct. Is there more to be added to this PR or was it decided that won’t be implemented?

Copy link
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.

When Changing email it would be better to show verify email dialog (like on booking page).

Currently after you change your email the emailVerified is set to null and you see banner only after some time or reloading and no email is sent to verify email. you have to click on 'Resend Email' to actually send the email

Screenshot 2024-01-29 at 4 18 36 PM

@sean-brydon
Copy link
Member Author

When Changing email it would be better to show verify email dialog (like on booking page).

Currently after you change your email the emailVerified is set to null and you see banner only after some time or reloading and no email is sent to verify email. you have to click on 'Resend Email' to actually send the email

Screenshot 2024-01-29 at 4 18 36 PM

Yeah in talks with @ciaranha about this as a improvement to this PR - great shout udit this is what we will be going with

will return to draft and update accordingly

@sean-brydon sean-brydon marked this pull request as draft January 29, 2024 10:56
token: z.string(),
});

export async function getServerSideProps(context: GetServerSidePropsContext) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Fetch serverside with the token passed in as props

Copy link
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.

Screen.Recording.2024-02-07.at.12.40.55.PM.mov

When I tried to change my email then I wasn't logged out immediately but only when i tried to reload or go to other page. The toast message says the email will be updated later but the UI displays the new email on settings/my-account/profile.

@sean-brydon
Copy link
Member Author

Screen.Recording.2024-02-07.at.12.40.55.PM.mov

When I tried to change my email then I wasn't logged out immediately but only when i tried to reload or go to other page. The toast message says the email will be updated later but the UI displays the new email on settings/my-account/profile.

Oh I know what’s happening here - can you test the rest of the flow will push a fix for this now

Copy link
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.

When I am logged in with Gmail (Idenitity provider) and try to change my email, I got logged out and got an email to set password and after logging in, I see email has not been verified which should have been verified as the password reset url was sent to email

Screenshot 2024-02-07 at 1 32 19 PM

Copy link
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.

If i try to change the email to one that already exists in the DB then there is no message like 'This email has been taken' and verify email is sent but the link doesn't work as the email already exist for other user

Screenshot 2024-02-07 at 1 54 25 PM

@sean-brydon
Copy link
Member Author

When I am logged in with Gmail (Idenitity provider) and try to change my email, I got logged out and got an email to set password and after logging in, I see email has not been verified which should have been verified as the password reset url was sent to email

Screenshot 2024-02-07 at 1 32 19 PM

Hmm - i thought we had tests for this so expected it to fail if this borked. Will write tests and fix

Copy link
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 🥇 💯 . Thanks for Fixing all the bugs and adding tests

@Udit-takkar Udit-takkar enabled auto-merge (squash) February 7, 2024 10:21
@Udit-takkar Udit-takkar merged commit 0e6f44e into main Feb 7, 2024
38 checks passed
@Udit-takkar Udit-takkar deleted the feat/email-verify-on-change branch February 7, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 points Created by SyncLinear.com authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in 🐛 bug Something isn't working consumer 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.

[CAL-1670] No verification required, when changing email
3 participants