Merge pull request #4708 from consul/confirmation-instructions

Fix sending OAuth confirmation instructions

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

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 "Username", with: "Manuela Carmena"
-        fill_in "user_email",                 with: "manuela@consul.dev"
+        fill_in "Email", with: "manuela@consul.dev"
-        fill_in "user_password",              with: "judgementday"
+        fill_in "Password", with: "judgementday"
-        fill_in "user_password_confirmation", 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 "Email or username", with: "manuela@consul.dev"
-        fill_in "user_password", with: "judgementday"
+        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 "Email or username", with: "中村広"
-        fill_in "user_password", with: "xenomorph"
+        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 "Email or username", with: "peter@nyc.dev"
-        fill_in "user_password", with: "greatpower"
+        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 "Email or username", with: "peter@nyc.dev"
-        fill_in "user_password", with: "symbiote"
+        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 "Email or username", with: "venom@nyc.dev"
-        fill_in "user_password", with: "symbiote"
+        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,13 +318,13 @@ 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
+      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)
 
@@ -334,22 +334,55 @@ 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 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
@@ -362,12 +395,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 +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
 
       scenario "Try to register with the email of an already existing user, when an unconfirmed email was provided by oauth" do
@@ -399,8 +432,8 @@ describe "Users" do
 
         expect(page).to have_current_path(finish_signup_path)
 
-        expect(page).to have_field("user_email", with: "manuelacarmena@example.com")
+        expect(page).to have_field "Email", with: "manuelacarmena@example.com"
-        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"
@@ -416,10 +449,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 +489,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 +505,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 +530,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 +555,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 +565,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 "New password", with: "new password"
-    fill_in "user_password_confirmation", 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 +577,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 +591,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 +604,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 +623,9 @@ describe "Users" do
 
     expect(page).to have_content "Your password is expired"
 
-    fill_in "user_current_password", with: "judgmentday"
+    fill_in "Current password", with: "judgmentday"
-    fill_in "user_password", with: "123456789"
+    fill_in "New password", with: "123456789"
-    fill_in "user_password_confirmation", with: "123456789"
+    fill_in "Password confirmation", with: "123456789"
 
     click_button "Change your password"
 
@@ -629,9 +662,9 @@ describe "Users" do
 
     expect(page).to have_content "Your password is expired"
 
-    fill_in "user_current_password", with: "judgmentday"
+    fill_in "Current password", with: "judgmentday"
-    fill_in "user_password", with: "123456789"
+    fill_in "New password", with: "123456789"
-    fill_in "user_password_confirmation", 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."