Merge remote-tracking branch 'upstream/pull/5446'

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 471215c92234f8535cd459dad4b44e52b5f929f4..fa311ab39e0d302a241f0023c4b89795828641c5 100644 (file)
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -77,7 +77,24 @@ class UsersController < ApplicationController
         render :action => "new"
       else
         # Save the user record
-        save_new_user params[:email_hmac], params[:referer]
+        if save_new_user params[:email_hmac]
+          SIGNUP_IP_LIMITER&.update(request.remote_ip)
+          SIGNUP_EMAIL_LIMITER&.update(canonical_email(current_user.email))
+
+          flash[:matomo_goal] = Settings.matomo["goals"]["signup"] if defined?(Settings.matomo)
+
+          referer = welcome_path(welcome_options(params[:referer]))
+
+          if current_user.status == "active"
+            successful_login(current_user, referer)
+          else
+            session[:pending_user] = current_user.id
+            UserMailer.signup_confirm(current_user, current_user.generate_token_for(:new_user), referer).deliver_later
+            redirect_to :controller => :confirmations, :action => :confirm, :display_name => current_user.display_name
+          end
+        else
+          render :action => "new", :referer => params[:referer]
+        end
       end
     end
   end
@@ -238,7 +255,7 @@ class UsersController < ApplicationController
 
   private
 
-  def save_new_user(email_hmac, referer = nil)
+  def save_new_user(email_hmac)
     current_user.data_public = true
     current_user.description = "" if current_user.description.nil?
     current_user.creation_address = request.remote_ip
@@ -254,24 +271,7 @@ class UsersController < ApplicationController
       current_user.activate
     end
 
-    if current_user.save
-      SIGNUP_IP_LIMITER&.update(request.remote_ip)
-      SIGNUP_EMAIL_LIMITER&.update(canonical_email(current_user.email))
-
-      flash[:matomo_goal] = Settings.matomo["goals"]["signup"] if defined?(Settings.matomo)
-
-      referer = welcome_path(welcome_options(referer))
-
-      if current_user.status == "active"
-        successful_login(current_user, referer)
-      else
-        session[:pending_user] = current_user.id
-        UserMailer.signup_confirm(current_user, current_user.generate_token_for(:new_user), referer).deliver_later
-        redirect_to :controller => :confirmations, :action => :confirm, :display_name => current_user.display_name
-      end
-    else
-      render :action => "new", :referer => params[:referer]
-    end
+    current_user.save
   end
 
   def welcome_options(referer = nil)

diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb
index d2c3d5196ab9b5ccbcae32aaaed086e03650d7f5..0ddb8a87ad73f91606df90af224df851c9919c06 100644 (file)
--- a/test/application_system_test_case.rb
+++ b/test/application_system_test_case.rb
@@ -41,4 +41,8 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
   def within_sidebar(&)
     within("#sidebar_content", &)
   end
+
+  def within_content_body(&)
+    within("#content > .content-body", &)
+  end
 end

diff --git a/test/system/user_signup_test.rb b/test/system/user_signup_test.rb
index 0e02b904f7f44a830fa5d2eba043fef1e7c70ba6..ebd91340a53d6b51b78482a882fed2f7fb20b014 100644 (file)
--- a/test/system/user_signup_test.rb
+++ b/test/system/user_signup_test.rb
@@ -1,23 +1,64 @@
 require "application_system_test_case"
 
 class UserSignupTest < ApplicationSystemTestCase
+  include ActionMailer::TestHelper
+
+  def setup
+    stub_request(:get, /.*gravatar.com.*d=404/).to_return(:status => 404)
+  end
+
+  test "Sign up with confirmation email" do
+    visit root_path
+
+    click_on "Sign Up"
+
+    within_content_body do
+      fill_in "Email", :with => "new_user_account@example.com"
+      fill_in "Display Name", :with => "new_user_account"
+      fill_in "Password", :with => "new_user_password"
+      fill_in "Confirm Password", :with => "new_user_password"
+
+      assert_emails 1 do
+        click_on "Sign Up"
+
+        assert_content "We sent you a confirmation email"
+      end
+    end
+
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.count
+    assert_equal "new_user_account@example.com", email.to.first
+    email_text = email.parts[0].parts[0].decoded
+    match = %r{/user/new_user_account/confirm\?confirm_string=\S+}.match(email_text)
+    assert_not_nil match
+
+    visit match[0]
+
+    assert_content "new_user_account"
+    assert_content "Welcome!"
+  end
+
   test "Sign up from login page" do
     visit login_path
 
     click_on "Sign up"
 
-    assert_content "Confirm Password"
+    within_content_body do
+      assert_content "Confirm Password"
+    end
   end
 
   test "Show OpenID form when OpenID provider button is clicked" do
     visit login_path
 
-    assert_no_field "OpenID URL"
-    assert_no_button "Continue"
+    within_content_body do
+      assert_no_field "OpenID URL"
+      assert_no_button "Continue"
 
-    click_on "Log in with OpenID"
+      click_on "Log in with OpenID"
 
-    assert_field "OpenID URL"
-    assert_button "Continue"
+      assert_field "OpenID URL"
+      assert_button "Continue"
+    end
   end
 end