From 08605312d8d34a9e4ba2648da80f5b61d70348ff Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Tue, 1 Sep 2026 02:32:14 +0000 Subject: [PATCH] Consolidate settings update protocol Amp-Thread-ID: https://ampcode.com/threads/T-01a05ac7-e6c4-720e-b250-7b0cd93639bd Co-authored-by: Amp --- .../settings/appearance_controller.rb | 16 ++++++---------- app/controllers/settings/base_controller.rb | 14 ++++++++++++++ app/controllers/settings/editors_controller.rb | 7 +------ app/controllers/settings/privacy_controller.rb | 7 +------ app/controllers/settings/profile_controller.rb | 7 +------ .../settings_appearance_controller_test.rb | 1 + .../settings_editors_controller_test.rb | 17 +++++++++++++++++ .../settings_profile_controller_test.rb | 10 ++++++++-- 8 files changed, 49 insertions(+), 30 deletions(-) create mode 100644 test/controllers/settings_editors_controller_test.rb diff --git a/app/controllers/settings/appearance_controller.rb b/app/controllers/settings/appearance_controller.rb index 344e3e91e..98656328a 100644 --- a/app/controllers/settings/appearance_controller.rb +++ b/app/controllers/settings/appearance_controller.rb @@ -1,15 +1,11 @@ class Settings::AppearanceController < Settings::BaseController def update_theme - if @user.update(theme_params) - redirect_back( - fallback_location: my_settings_appearance_path, - notice: "Settings updated successfully", - inertia: { clear_history: true } - ) - else - flash.now[:error] = @user.errors.full_messages.to_sentence.presence || "Failed to update settings" - render_settings_page(status: :unprocessable_entity) - end + update_user_settings( + theme_params, + redirect_location: my_settings_appearance_path, + back: true, + clear_history: true + ) end private diff --git a/app/controllers/settings/base_controller.rb b/app/controllers/settings/base_controller.rb index 0a30ba931..05daeb05b 100644 --- a/app/controllers/settings/base_controller.rb +++ b/app/controllers/settings/base_controller.rb @@ -14,6 +14,20 @@ def render_settings_page(status: :ok, extra_props: {}) status: status end + def update_user_settings(attributes, redirect_location:, back: false, clear_history: false) + if @user.update(attributes) + redirect_options = { notice: "Settings updated successfully" } + redirect_options[:inertia] = { clear_history: true } if clear_history + + return redirect_back(fallback_location: redirect_location, **redirect_options) if back + + redirect_to redirect_location, **redirect_options + else + flash.now[:error] = @user.errors.full_messages.to_sentence.presence || "Failed to update settings" + render_settings_page(status: :unprocessable_entity) + end + end + # Lightweight props shared by every settings page def common_props(active_section:) { active_section: active_section, diff --git a/app/controllers/settings/editors_controller.rb b/app/controllers/settings/editors_controller.rb index 99f718039..7d9093d37 100644 --- a/app/controllers/settings/editors_controller.rb +++ b/app/controllers/settings/editors_controller.rb @@ -1,11 +1,6 @@ class Settings::EditorsController < Settings::BaseController def update - if @user.update(editor_params) - redirect_to my_settings_editors_path, notice: "Settings updated successfully" - else - flash.now[:error] = @user.errors.full_messages.to_sentence.presence || "Failed to update settings" - render_settings_page(status: :unprocessable_entity) - end + update_user_settings(editor_params, redirect_location: my_settings_editors_path) end private diff --git a/app/controllers/settings/privacy_controller.rb b/app/controllers/settings/privacy_controller.rb index 141db7705..50d2607a2 100644 --- a/app/controllers/settings/privacy_controller.rb +++ b/app/controllers/settings/privacy_controller.rb @@ -1,11 +1,6 @@ class Settings::PrivacyController < Settings::BaseController def update - if @user.update(privacy_params) - redirect_back(fallback_location: my_settings_privacy_path, notice: "Settings updated successfully") - else - flash.now[:error] = @user.errors.full_messages.to_sentence.presence || "Failed to update settings" - render_settings_page(status: :unprocessable_entity) - end + update_user_settings(privacy_params, redirect_location: my_settings_privacy_path, back: true) end def rotate_api_key diff --git a/app/controllers/settings/profile_controller.rb b/app/controllers/settings/profile_controller.rb index d3eed6585..d50c7c375 100644 --- a/app/controllers/settings/profile_controller.rb +++ b/app/controllers/settings/profile_controller.rb @@ -66,12 +66,7 @@ def pin_current_timezone(timezones) end def update_section(permitted_params) - if @user.update(permitted_params) - redirect_back(fallback_location: my_settings_profile_path, notice: "Settings updated successfully") - else - flash.now[:error] = @user.errors.full_messages.to_sentence.presence || "Failed to update settings" - render_settings_page(status: :unprocessable_entity) - end + update_user_settings(permitted_params, redirect_location: my_settings_profile_path, back: true) end def region_params diff --git a/test/controllers/settings_appearance_controller_test.rb b/test/controllers/settings_appearance_controller_test.rb index b7dacab6b..5ea951570 100644 --- a/test/controllers/settings_appearance_controller_test.rb +++ b/test/controllers/settings_appearance_controller_test.rb @@ -13,6 +13,7 @@ class SettingsAppearanceControllerTest < ActionDispatch::IntegrationTest assert_redirected_to my_settings_appearance_path assert_equal "nord", user.reload.theme assert_equal "nord", cookies[:hackatime_theme] + assert_equal "Settings updated successfully", flash[:notice] follow_redirect!(headers: { "X-Inertia" => "true" }) diff --git a/test/controllers/settings_editors_controller_test.rb b/test/controllers/settings_editors_controller_test.rb new file mode 100644 index 000000000..0ace853cf --- /dev/null +++ b/test/controllers/settings_editors_controller_test.rb @@ -0,0 +1,17 @@ +require "test_helper" + +class SettingsEditorsControllerTest < ActionDispatch::IntegrationTest + test "editor update persists settings and redirects directly to editors" do + user = create(:user) + sign_in_as(user) + + patch my_settings_editors_update_path, + params: { user: { hackatime_extension_text_type: "clock_emoji" } }, + headers: { "HTTP_REFERER" => my_settings_profile_path } + + assert_response :redirect + assert_redirected_to my_settings_editors_path + assert_equal "clock_emoji", user.reload.hackatime_extension_text_type + assert_equal "Settings updated successfully", flash[:notice] + end +end diff --git a/test/controllers/settings_profile_controller_test.rb b/test/controllers/settings_profile_controller_test.rb index bf9416c5e..20dd3e823 100644 --- a/test/controllers/settings_profile_controller_test.rb +++ b/test/controllers/settings_profile_controller_test.rb @@ -16,13 +16,17 @@ class SettingsProfileControllerTest < ActionDispatch::IntegrationTest user = create(:user) user.update!(slack_username: "slack_name") sign_in_as(user) + return_path = "#{my_settings_profile_path}?section=display-name" - patch my_settings_profile_display_name_path, params: { user: { display_name_override: "Custom Name" } } + patch my_settings_profile_display_name_path, + params: { user: { display_name_override: "Custom Name" } }, + headers: { "HTTP_REFERER" => return_path } assert_response :redirect - assert_redirected_to my_settings_profile_path + assert_redirected_to return_path assert_equal "Custom Name", user.reload.display_name_override assert_equal "Custom Name", user.display_name + assert_equal "Settings updated successfully", flash[:notice] end test "display name update clears blank override" do @@ -47,6 +51,8 @@ class SettingsProfileControllerTest < ActionDispatch::IntegrationTest assert_response :unprocessable_entity assert_inertia_component "Users/Settings/Profile" + assert_predicate flash[:error], :present? + assert_nil user.reload.display_name_override end test "username update with invalid username returns unprocessable entity" do