From d9a055d05862bf2a1e56e263d467801df94ee0cb Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Fri, 5 Apr 2024 17:33:09 +0200 Subject: [PATCH] [Users] Redirect to name changes when name is invalid The more or less unintrusive message has been there for a few months now. Start forcing a name change when accessing through a browser, ignore api for now --- app/controllers/application_controller.rb | 9 ++++ app/views/layouts/default.html.erb | 7 --- .../user_name_change_requests/new.html.erb | 7 +++ .../functional/application_controller_test.rb | 17 ++++++++ ...er_name_change_requests_controller_test.rb | 43 +++++++++++++------ 5 files changed, 64 insertions(+), 19 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ca06b47d1..a0d71c23c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -10,6 +10,7 @@ class ApplicationController < ActionController::Base before_action :normalize_search before_action :api_check before_action :enable_cors + before_action :check_valid_username after_action :reset_current_user layout "default" @@ -30,6 +31,14 @@ class ApplicationController < ActionController::Base response.headers["Access-Control-Allow-Headers"] = "Authorization" end + def check_valid_username + return if params[:controller] == "user_name_change_requests" + + if request.format.html? && CurrentUser.user.name_error + redirect_to new_user_name_change_request_path + end + end + protected def api_check diff --git a/app/views/layouts/default.html.erb b/app/views/layouts/default.html.erb index abbe75328..9a22b0212 100644 --- a/app/views/layouts/default.html.erb +++ b/app/views/layouts/default.html.erb @@ -30,13 +30,6 @@
<%= render "news_updates/notice", news_update: NewsUpdate.recent %> - <% if CurrentUser.is_member? && (name_error = CurrentUser.name_error) %> -
-

Your current username '<%= CurrentUser.pretty_name %>' is invalid. Please change it <%= link_to "here", new_user_name_change_request_path %>.

- Error: <%= name_error %> -

- <% end %> - <% if CurrentUser.user.is_banned? %> <%= render "users/ban_notice" %> <% end %> diff --git a/app/views/user_name_change_requests/new.html.erb b/app/views/user_name_change_requests/new.html.erb index 09dd52f1c..2ccf139f3 100644 --- a/app/views/user_name_change_requests/new.html.erb +++ b/app/views/user_name_change_requests/new.html.erb @@ -1,5 +1,12 @@
+ <% if CurrentUser.is_member? && (name_error = CurrentUser.user.name_error) %> +
+

Your current username '<%= CurrentUser.user.pretty_name %>' is invalid. You must change it to continue using the site.

+ Error: <%= name_error %> +

+ <% end %> +

Name Change Request

You can request a name change once per week.

diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index 62b3942e8..6606f3680 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -143,5 +143,22 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest assert_equal("s", post.reload.rating) end end + + context "when the user has an invalid username" do + setup do + @user = build(:user, name: "12345") + @user.save(validate: false) + end + + should "redirect for html requests" do + get_auth posts_path, @user, params: { format: :html } + assert_redirected_to new_user_name_change_request_path + end + + should "not redirect for json requests" do + get_auth posts_path, @user, params: { format: :json } + assert_response :success + end + end end end diff --git a/test/functional/user_name_change_requests_controller_test.rb b/test/functional/user_name_change_requests_controller_test.rb index 08a9a08b7..1a0223472 100644 --- a/test/functional/user_name_change_requests_controller_test.rb +++ b/test/functional/user_name_change_requests_controller_test.rb @@ -7,14 +7,6 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest setup do @user = create(:privileged_user) @admin = create(:admin_user) - as(@user) do - @change_request = UserNameChangeRequest.create!( - :user_id => @user.id, - :original_name => @user.name, - :desired_name => "abc", - :change_reason => "hello" - ) - end end context "new action" do @@ -22,16 +14,43 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest get_auth new_user_name_change_request_path, @user assert_response :success end - end - context "create action" do - should "work" do - post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "zun" }} + should "render for a user with a currently invalid username" do + @user.update_columns(name: "12345") + get_auth new_user_name_change_request_path, @user assert_response :success end end + context "create action" do + should "work" do + post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "zun" } } + change_request = UserNameChangeRequest.last + assert_redirected_to user_name_change_request_path(change_request) + assert_equal("zun", @user.reload.name) + end + + should "work for a user with a currently invalid name" do + @user.update_columns(name: "12345") + post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "zun" } } + change_request = UserNameChangeRequest.last + assert_redirected_to user_name_change_request_path(change_request) + assert_equal("zun", @user.reload.name) + end + end + context "show action" do + setup do + as(@user) do + @change_request = UserNameChangeRequest.create!( + user_id: @user.id, + original_name: @user.name, + desired_name: "abc", + change_reason: "hello", + ) + end + end + should "render" do get_auth user_name_change_request_path(@change_request), @user assert_response :success