From ba4595f6ce73976ecf678e0f86cb496e96b80a05 Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 11 Oct 2021 12:12:10 +0200 Subject: [PATCH 1/2] Use the label text in the specs Using the label text in the specs is superior to using the name because it tests the label is correctly associated to its form control. --- spec/system/users_auth_spec.rb | 104 ++++++++++++++++----------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/spec/system/users_auth_spec.rb b/spec/system/users_auth_spec.rb index 32f49d134..7376a8b62 100644 --- a/spec/system/users_auth_spec.rb +++ b/spec/system/users_auth_spec.rb @@ -8,10 +8,10 @@ describe "Users" do visit "/" click_link "Register" - fill_in "user_username", with: "Manuela Carmena" - fill_in "user_email", with: "manuela@consul.dev" - fill_in "user_password", with: "judgementday" - fill_in "user_password_confirmation", with: "judgementday" + fill_in "Username", with: "Manuela Carmena" + fill_in "Email", with: "manuela@consul.dev" + fill_in "Password", with: "judgementday" + fill_in "Confirm password", with: "judgementday" check "user_terms_of_service" click_button "Register" @@ -38,8 +38,8 @@ describe "Users" do visit "/" click_link "Sign in" - fill_in "user_login", with: "manuela@consul.dev" - fill_in "user_password", with: "judgementday" + fill_in "Email or username", with: "manuela@consul.dev" + fill_in "Password", with: "judgementday" click_button "Enter" expect(page).to have_content "You have been signed in successfully." @@ -50,8 +50,8 @@ describe "Users" do visit "/" click_link "Sign in" - fill_in "user_login", with: "中村広" - fill_in "user_password", with: "xenomorph" + fill_in "Email or username", with: "中村広" + fill_in "Password", with: "xenomorph" click_button "Enter" expect(page).to have_content "You have been signed in successfully." @@ -63,8 +63,8 @@ describe "Users" do visit "/" click_link "Sign in" - fill_in "user_login", with: "peter@nyc.dev" - fill_in "user_password", with: "greatpower" + fill_in "Email or username", with: "peter@nyc.dev" + fill_in "Password", with: "greatpower" click_button "Enter" expect(page).to have_content "You have been signed in successfully." @@ -80,15 +80,15 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "Sign in" - fill_in "user_login", with: "peter@nyc.dev" - fill_in "user_password", with: "symbiote" + fill_in "Email or username", with: "peter@nyc.dev" + fill_in "Password", with: "symbiote" click_button "Enter" expect(page).not_to have_content "You have been signed in successfully." expect(page).to have_content "Invalid Email or username or password." - fill_in "user_login", with: "venom@nyc.dev" - fill_in "user_password", with: "symbiote" + fill_in "Email or username", with: "venom@nyc.dev" + fill_in "Password", with: "symbiote" click_button "Enter" expect(page).to have_content "You have been signed in successfully." @@ -226,10 +226,10 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "My account" - expect(page).to have_field("account_username", with: "manuela") + expect(page).to have_field "Username", with: "manuela" visit edit_user_registration_path - expect(page).to have_field("user_email", with: "manuelacarmena@example.com") + expect(page).to have_field "Email", with: "manuelacarmena@example.com" end scenario "Sign up when Oauth provider has an unverified email" do @@ -254,10 +254,10 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "My account" - expect(page).to have_field("account_username", with: "manuela") + expect(page).to have_field "Username", with: "manuela" visit edit_user_registration_path - expect(page).to have_field("user_email", with: "manuelacarmena@example.com") + expect(page).to have_field "Email", with: "manuelacarmena@example.com" end scenario "Sign up, when no email was provided by OAuth provider" do @@ -268,7 +268,7 @@ describe "Users" do click_link "Sign up with Twitter" expect(page).to have_current_path(finish_signup_path) - fill_in "user_email", with: "manueladelascarmenas@example.com" + fill_in "Email", with: "manueladelascarmenas@example.com" click_button "Register" expect(page).to have_content "To continue, please click on the confirmation link that we have sent you via email" @@ -284,10 +284,10 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "My account" - expect(page).to have_field("account_username", with: "manuela") + expect(page).to have_field "Username", with: "manuela" visit edit_user_registration_path - expect(page).to have_field("user_email", with: "manueladelascarmenas@example.com") + expect(page).to have_field "Email", with: "manueladelascarmenas@example.com" end scenario "Cancelling signup" do @@ -318,10 +318,10 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "My account" - expect(page).to have_field("account_username", with: user.username) + expect(page).to have_field "Username", with: user.username visit edit_user_registration_path - expect(page).to have_field("user_email", with: user.email) + expect(page).to have_field "Email", with: user.email end scenario "Try to register with the username of an already existing user" do @@ -334,22 +334,22 @@ describe "Users" do expect(page).to have_current_path(finish_signup_path) - expect(page).to have_field("user_username", with: "manuela") + expect(page).to have_field "Username", with: "manuela" click_button "Register" expect(page).to have_current_path(do_finish_signup_path) - fill_in "user_username", with: "manuela2" + fill_in "Username", with: "manuela2" click_button "Register" expect_to_be_signed_in click_link "My account" - expect(page).to have_field("account_username", with: "manuela2") + expect(page).to have_field "Username", with: "manuela2" visit edit_user_registration_path - expect(page).to have_field("user_email", with: "manuelacarmena@example.com") + expect(page).to have_field "Email", with: "manuelacarmena@example.com" end scenario "Try to register with the email of an already existing user, when no email was provided by oauth" do @@ -362,12 +362,12 @@ describe "Users" do expect(page).to have_current_path(finish_signup_path) - fill_in "user_email", with: "manuela@example.com" + fill_in "Email", with: "manuela@example.com" click_button "Register" expect(page).to have_current_path(do_finish_signup_path) - fill_in "user_email", with: "somethingelse@example.com" + fill_in "Email", with: "somethingelse@example.com" click_button "Register" expect(page).to have_content "To continue, please click on the confirmation link that we have sent you via email" @@ -383,10 +383,10 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "My account" - expect(page).to have_field("account_username", with: "manuela") + expect(page).to have_field "Username", with: "manuela" visit edit_user_registration_path - expect(page).to have_field("user_email", with: "somethingelse@example.com") + expect(page).to have_field "Email", with: "somethingelse@example.com" end scenario "Try to register with the email of an already existing user, when an unconfirmed email was provided by oauth" do @@ -399,8 +399,8 @@ describe "Users" do expect(page).to have_current_path(finish_signup_path) - expect(page).to have_field("user_email", with: "manuelacarmena@example.com") - fill_in "user_email", with: "somethingelse@example.com" + expect(page).to have_field "Email", with: "manuelacarmena@example.com" + fill_in "Email", with: "somethingelse@example.com" click_button "Register" expect(page).to have_content "To continue, please click on the confirmation link that we have sent you via email" @@ -416,10 +416,10 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "My account" - expect(page).to have_field("account_username", with: "manuela") + expect(page).to have_field "Username", with: "manuela" visit edit_user_registration_path - expect(page).to have_field("user_email", with: "somethingelse@example.com") + expect(page).to have_field "Email", with: "somethingelse@example.com" end end @@ -456,10 +456,10 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "My account" - expect(page).to have_field("account_username", with: "manuela") + expect(page).to have_field "Username", with: "manuela" visit edit_user_registration_path - expect(page).to have_field("user_email", with: "manuelacarmena@example.com") + expect(page).to have_field "Email", with: "manuelacarmena@example.com" end scenario "Try to register with username and email of an already existing user" do @@ -472,7 +472,7 @@ describe "Users" do expect(page).to have_current_path(finish_signup_path) - expect(page).to have_field("user_username", with: "manuela") + expect(page).to have_field "Username", with: "manuela" click_button "Register" @@ -497,10 +497,10 @@ describe "Users" do within("#notice") { click_button "Close" } click_link "My account" - expect(page).to have_field("account_username", with: "manuela2") + expect(page).to have_field "Username", with: "manuela2" visit edit_user_registration_path - expect(page).to have_field("user_email", with: "manuela@consul.dev") + expect(page).to have_field "Email", with: "manuela@consul.dev" end end end @@ -522,7 +522,7 @@ describe "Users" do click_link "Sign in" click_link "Forgotten your password?" - fill_in "user_email", with: "manuela@consul.dev" + fill_in "Email", with: "manuela@consul.dev" click_button "Send instructions" expect(page).to have_content "If your email address is in our database, in a few minutes "\ @@ -532,8 +532,8 @@ describe "Users" do sent_token = /.*reset_password_token=(.*)".*/.match(action_mailer)[1] visit edit_user_password_path(reset_password_token: sent_token) - fill_in "user_password", with: "new password" - fill_in "user_password_confirmation", with: "new password" + fill_in "New password", with: "new password" + fill_in "Confirm new password", with: "new password" click_button "Change my password" expect(page).to have_content "Your password has been changed successfully." @@ -544,7 +544,7 @@ describe "Users" do click_link "Sign in" click_link "Forgotten your password?" - fill_in "user_email", with: "fake@mail.dev" + fill_in "Email", with: "fake@mail.dev" click_button "Send instructions" expect(page).to have_content "If your email address is in our database, in a few minutes "\ @@ -558,7 +558,7 @@ describe "Users" do click_link "Sign in" click_link "Haven't received instructions to activate your account?" - fill_in "user_email", with: "manuela@consul.dev" + fill_in "Email", with: "manuela@consul.dev" click_button "Re-send instructions" expect(page).to have_content "If your email address is in our database, in a few minutes you "\ @@ -571,7 +571,7 @@ describe "Users" do click_link "Sign in" click_link "Haven't received instructions to activate your account?" - fill_in "user_email", with: "fake@mail.dev" + fill_in "Email", with: "fake@mail.dev" click_button "Re-send instructions" expect(page).to have_content "If your email address is in our database, in a few minutes you "\ @@ -590,9 +590,9 @@ describe "Users" do expect(page).to have_content "Your password is expired" - fill_in "user_current_password", with: "judgmentday" - fill_in "user_password", with: "123456789" - fill_in "user_password_confirmation", with: "123456789" + fill_in "Current password", with: "judgmentday" + fill_in "New password", with: "123456789" + fill_in "Password confirmation", with: "123456789" click_button "Change your password" @@ -629,9 +629,9 @@ describe "Users" do expect(page).to have_content "Your password is expired" - fill_in "user_current_password", with: "judgmentday" - fill_in "user_password", with: "123456789" - fill_in "user_password_confirmation", with: "123456789" + fill_in "Current password", with: "judgmentday" + fill_in "New password", with: "123456789" + fill_in "Password confirmation", with: "123456789" click_button "Change your password" expect(page).to have_content "must be different than the current password." From 0493893ab8c28fa9af95d38b441a47c636001ad7 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 8 Oct 2021 18:06:54 +0200 Subject: [PATCH 2/2] Fix send confirmation instructions on do_finish_signup action When we try to register with omniauth and the email or username already exists, we use the finish_signup and do_finish_signup actions to allow the user to choose another email or username. The do_finish_signup action of the registration controller calls the send_oauth_confirmation_instructions method which is responsible for sending the confirmation email. In this method we were only validating the case that the email is duplicated. Now we add one more condition that allows us to send the instructions for the case in which we have had to change our username. --- app/models/user.rb | 2 +- spec/system/users_auth_spec.rb | 35 +++++++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 918667840..0e4be645d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -348,7 +348,7 @@ class User < ApplicationRecord end def send_oauth_confirmation_instructions - if oauth_email != email + if oauth_email != email || confirmed_at.nil? update(confirmed_at: nil) send_confirmation_instructions end diff --git a/spec/system/users_auth_spec.rb b/spec/system/users_auth_spec.rb index 7376a8b62..02eadcfd3 100644 --- a/spec/system/users_auth_spec.rb +++ b/spec/system/users_auth_spec.rb @@ -324,7 +324,7 @@ describe "Users" do expect(page).to have_field "Email", with: user.email end - scenario "Try to register with the username of an already existing user" do + scenario "Try to register with verified email and with the username of an already existing user" do create(:user, username: "manuela", email: "manuela@consul.dev", password: "judgementday") OmniAuth.config.add_mock(:twitter, twitter_hash_with_verified_email) @@ -352,6 +352,39 @@ describe "Users" do expect(page).to have_field "Email", with: "manuelacarmena@example.com" end + scenario "Try to register with unverified email and with the username of an already existing user" do + create(:user, username: "manuela", email: "manuela@consul.dev", password: "judgementday") + OmniAuth.config.add_mock(:twitter, twitter_hash_with_email) + + visit "/" + click_link "Register" + click_link "Sign up with Twitter" + + expect(page).to have_current_path(finish_signup_path) + expect(page).to have_field "Username", with: "manuela" + + click_button "Register" + + expect(page).to have_current_path(do_finish_signup_path) + + fill_in "Username", with: "manuela2" + click_button "Register" + confirm_email + + expect(page).to have_content "Your account has been confirmed" + + visit "/" + click_link "Sign in" + click_link "Sign in with Twitter" + + within("#notice") { click_button "Close" } + click_link "My account" + expect(page).to have_field "Username", with: "manuela2" + + visit edit_user_registration_path + expect(page).to have_field "Email", with: "manuelacarmena@example.com" + end + scenario "Try to register with the email of an already existing user, when no email was provided by oauth" do create(:user, username: "peter", email: "manuela@example.com") OmniAuth.config.add_mock(:twitter, twitter_hash)