From 34514bcfe9d6416e73ac918005f9890778f40eab Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 10 Feb 2026 10:34:42 -0500 Subject: [PATCH] Move theme-related helper methods out of controller (#37807) --- app/controllers/application_controller.rb | 17 ------ app/helpers/application_helper.rb | 6 --- app/helpers/theme_helper.rb | 18 +++++++ .../application_controller_spec.rb | 14 ----- spec/helpers/theme_helper_spec.rb | 54 +++++++++++++++++++ 5 files changed, 72 insertions(+), 37 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7b62c95aa64..f6d3ce35ab4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -17,9 +17,6 @@ class ApplicationController < ActionController::Base helper_method :current_account helper_method :current_session - helper_method :current_theme - helper_method :color_scheme - helper_method :contrast helper_method :single_user_mode? helper_method :use_seamless_external_login? helper_method :sso_account_settings @@ -173,20 +170,6 @@ class ApplicationController < ActionController::Base @current_session = SessionActivation.find_by(session_id: cookies.signed['_session_id']) if cookies.signed['_session_id'].present? end - def current_theme - return Setting.theme unless Themes.instance.names.include? current_user&.setting_theme - - current_user.setting_theme - end - - def color_scheme - current_user&.setting_color_scheme || 'auto' - end - - def contrast - current_user&.setting_contrast || 'auto' - end - def respond_with_error(code) respond_to do |format| format.any { render "errors/#{code}", layout: 'error', status: code, formats: [:html] } diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index b23968e3731..891fc905cd4 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -89,12 +89,6 @@ module ApplicationHelper Rails.env.production? ? site_title : "#{site_title} (Dev)" end - def page_color_scheme - return content_for(:force_color_scheme) if content_for(:force_color_scheme) - - color_scheme - end - def label_for_scope(scope) safe_join [ tag.samp(scope, class: { 'scope-danger' => SessionActivation::DEFAULT_SCOPES.include?(scope.to_s) }), diff --git a/app/helpers/theme_helper.rb b/app/helpers/theme_helper.rb index ea3cdfd39e5..b0f1649f999 100644 --- a/app/helpers/theme_helper.rb +++ b/app/helpers/theme_helper.rb @@ -46,6 +46,24 @@ module ThemeHelper ) end + def current_theme + return Setting.theme unless Themes.instance.names.include? current_user&.setting_theme + + current_user.setting_theme + end + + def color_scheme + current_user&.setting_color_scheme || 'auto' + end + + def contrast + current_user&.setting_contrast || 'auto' + end + + def page_color_scheme + content_for(:force_color_scheme).presence || color_scheme + end + private def active_custom_stylesheet diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f5e42ab4eba..24b775a5ad8 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -85,20 +85,6 @@ RSpec.describe ApplicationController do end end - describe 'helper_method :current_theme' do - it 'returns "default" when theme wasn\'t changed in admin settings' do - allow(Setting).to receive(:default_settings).and_return({ 'theme' => 'default' }) - - expect(controller.view_context.current_theme).to eq 'default' - end - - it 'returns instances\'s theme when user is not signed in' do - allow(Setting).to receive(:[]).with('theme').and_return 'contrast' - - expect(controller.view_context.current_theme).to eq 'contrast' - end - end - context 'with ActionController::RoutingError' do subject do routes.draw { get 'routing_error' => 'anonymous#routing_error' } diff --git a/spec/helpers/theme_helper_spec.rb b/spec/helpers/theme_helper_spec.rb index a9403f8b0c3..4c8702af745 100644 --- a/spec/helpers/theme_helper_spec.rb +++ b/spec/helpers/theme_helper_spec.rb @@ -100,6 +100,60 @@ RSpec.describe ThemeHelper do end end + describe '#current_theme' do + subject { helper.current_theme } + + context 'when user is not signed in' do + context 'when theme was not changed in settings' do + it { is_expected.to eq('default') } + end + + context 'when theme is changed in settings' do + before { Setting.theme = 'contrast' } + + it { is_expected.to eq('contrast') } + end + end + + context 'when user is signed in' do + before { allow(helper).to receive(:current_user).and_return(current_user) } + + let(:current_user) { Fabricate :user } + + context 'when user did not set theme' do + it { is_expected.to eq('default') } + end + + context 'when user set theme' do + before { current_user.settings.update(theme: 'alternate', noindex: false) } + + context 'when theme is valid' do + before { allow(Themes.instance).to receive(:names).and_return %w(default alternate good evil) } + + it { is_expected.to eq('alternate') } + end + + context 'when theme is not valid' do + it { is_expected.to eq('default') } + end + end + end + end + + describe '#page_color_scheme' do + subject { helper.page_color_scheme } + + context 'when force_color_scheme is present' do + before { helper.content_for(:force_color_scheme) { 'value' } } + + it { is_expected.to eq('value') } + end + + context 'when force_color_scheme is absent' do + it { is_expected.to eq('auto') } + end + end + private def html_links