From: Tom Hughes Date: Sun, 29 Dec 2024 19:11:13 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/5448' X-Git-Tag: live~409 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/782e619de229a06c4ffe55cc840e7f63c5693836?hp=c7e038a4d098ef9f42eb13edf29aef24cbd537ef Merge remote-tracking branch 'upstream/pull/5448' --- diff --git a/app/assets/javascripts/diary_entry.js b/app/assets/javascripts/diary_entry.js index 625d43a52..bd5fd4dd6 100644 --- a/app/assets/javascripts/diary_entry.js +++ b/app/assets/javascripts/diary_entry.js @@ -2,8 +2,10 @@ $(document).ready(function () { var marker, map; function setLocation(e) { - $("#latitude").val(e.latlng.lat); - $("#longitude").val(e.latlng.lng); + const latlng = e.latlng.wrap(); + + $("#latitude").val(latlng.lat); + $("#longitude").val(latlng.lng); if (marker) { map.removeLayer(marker); diff --git a/app/controllers/api/messages_controller.rb b/app/controllers/api/messages_controller.rb index cbbd8539c..ea5abe8df 100644 --- a/app/controllers/api/messages_controller.rb +++ b/app/controllers/api/messages_controller.rb @@ -5,7 +5,6 @@ module Api before_action :authorize before_action :check_api_writable, :only => [:create, :update, :destroy] - before_action :check_api_readable, :except => [:create, :update, :destroy] authorize_resource diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 471215c92..fa311ab39 100644 --- 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/app/views/confirmations/confirm.html.erb b/app/views/confirmations/confirm.html.erb index 4f98a8539..08dea27b6 100644 --- a/app/views/confirmations/confirm.html.erb +++ b/app/views/confirmations/confirm.html.erb @@ -28,7 +28,9 @@

- <%= t ".resend_html", - :reconfirm_link => link_to(t(".click_here"), url_for(:action => "confirm_resend")) %> + <%= t ".if_need_resend" %>

+ <%= bootstrap_form_tag :url => url_for(:action => "confirm_resend") do |f| %> + <%= f.submit t(".resend_button"), :class => "btn btn-outline-primary" %> + <% end %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index cb34b665d..5628d3ea9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1726,8 +1726,8 @@ en: success: "Confirmed your account, thanks for signing up!" already active: "This account has already been confirmed." unknown token: "That confirmation code has expired or does not exist." - resend_html: "If you need us to resend the confirmation email, %{reconfirm_link}." - click_here: click here + if_need_resend: "If you need us to resend the confirmation email, click the button below." + resend_button: Resend the confirmation email confirm_resend: failure: "User %{name} not found." confirm_email: diff --git a/config/routes.rb b/config/routes.rb index 58d629d69..d7b1acd3a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -186,7 +186,7 @@ OpenStreetMap::Application.routes.draw do post "/user/new" => "users#create" get "/user/terms" => "users#terms" post "/user/save" => "users#save" - get "/user/:display_name/confirm/resend" => "confirmations#confirm_resend", :as => :user_confirm_resend + post "/user/:display_name/confirm/resend" => "confirmations#confirm_resend", :as => :user_confirm_resend match "/user/:display_name/confirm" => "confirmations#confirm", :via => [:get, :post] match "/user/confirm" => "confirmations#confirm", :via => [:get, :post] match "/user/confirm-email" => "confirmations#confirm_email", :via => [:get, :post] diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index d2c3d5196..0ddb8a87a 100644 --- 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/controllers/confirmations_controller_test.rb b/test/controllers/confirmations_controller_test.rb index 79213441f..c8926eb75 100644 --- a/test/controllers/confirmations_controller_test.rb +++ b/test/controllers/confirmations_controller_test.rb @@ -13,7 +13,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest { :controller => "confirmations", :action => "confirm", :display_name => "username" } ) assert_routing( - { :path => "/user/username/confirm/resend", :method => :get }, + { :path => "/user/username/confirm/resend", :method => :post }, { :controller => "confirmations", :action => "confirm_resend", :display_name => "username" } ) @@ -193,7 +193,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_difference "ActionMailer::Base.deliveries.size", 1 do perform_enqueued_jobs do - get user_confirm_resend_path(user) + post user_confirm_resend_path(user) end end @@ -216,7 +216,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - get user_confirm_resend_path(user) + post user_confirm_resend_path(user) end end @@ -227,7 +227,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest def test_confirm_resend_unknown_user assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - get user_confirm_resend_path(:display_name => "No Such User") + post user_confirm_resend_path(:display_name => "No Such User") end end diff --git a/test/system/confirmation_resend_test.rb b/test/system/confirmation_resend_test.rb deleted file mode 100644 index 6ef85818a..000000000 --- a/test/system/confirmation_resend_test.rb +++ /dev/null @@ -1,23 +0,0 @@ -require "application_system_test_case" - -class ConfirmationResendSystemTest < ApplicationSystemTestCase - def setup - @user = build(:user) - visit user_new_path - - within ".new_user" do - fill_in "Email", :with => @user.email - fill_in "Display Name", :with => @user.display_name - fill_in "Password", :with => "testtest" - fill_in "Confirm Password", :with => "testtest" - click_on "Sign Up" - end - end - - test "flash message should not contain raw html" do - visit user_confirm_resend_path(@user) - - assert_content "sent a new confirmation" - assert_no_content "

" - end -end diff --git a/test/system/user_signup_test.rb b/test/system/user_signup_test.rb index 0e02b904f..2fb90fc3a 100644 --- a/test/system/user_signup_test.rb +++ b/test/system/user_signup_test.rb @@ -1,23 +1,102 @@ 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 with confirmation email resending" 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 2 do + click_on "Sign Up" + + assert_content "We sent you a confirmation email" + + click_on "Resend the confirmation email" + + assert_content "Email Address or Username" + end + end + + assert_content "sent a new confirmation" + assert_no_content "

" + + email = ActionMailer::Base.deliveries.last + 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