DEV: Extract theme resolution to a helper (#27426)

This ensures that the theme id is resolved as early as possible in the
request cycle. This is necessary for the custom homepage to skip
preloading the wrong data.
This commit is contained in:
Penar Musaraj 2024-06-20 11:33:46 -04:00 committed by GitHub
parent d29160131d
commit 33de5abb6e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 51 additions and 35 deletions

View File

@ -9,6 +9,7 @@ class ApplicationController < ActionController::Base
include GlobalPath include GlobalPath
include Hijack include Hijack
include ReadOnlyMixin include ReadOnlyMixin
include ThemeResolver
include VaryHeader include VaryHeader
attr_reader :theme_id attr_reader :theme_id
@ -478,34 +479,7 @@ class ApplicationController < ActionController::Base
resolve_safe_mode resolve_safe_mode
return if request.env[NO_THEMES] return if request.env[NO_THEMES]
theme_id = nil @theme_id ||= ThemeResolver.resolve_theme_id(request, guardian, current_user)
if (preview_theme_id = request[:preview_theme_id]&.to_i) &&
guardian.allow_themes?([preview_theme_id], include_preview: true)
theme_id = preview_theme_id
end
user_option = current_user&.user_option
if theme_id.blank?
ids, seq = cookies[:theme_ids]&.split("|")
id = ids&.split(",")&.map(&:to_i)&.first
if id.present? && seq && seq.to_i == user_option&.theme_key_seq.to_i
theme_id = id if guardian.allow_themes?([id])
end
end
if theme_id.blank?
ids = user_option&.theme_ids || []
theme_id = ids.first if guardian.allow_themes?(ids)
end
if theme_id.blank? && SiteSetting.default_theme_id != -1 &&
guardian.allow_themes?([SiteSetting.default_theme_id])
theme_id = SiteSetting.default_theme_id
end
@theme_id = request.env[:resolved_theme_id] = theme_id
end end
def guardian def guardian
@ -518,7 +492,7 @@ class ApplicationController < ActionController::Base
end end
def current_homepage def current_homepage
current_user&.user_option&.homepage || HomepageHelper.resolve(request, current_user) current_user&.user_option&.homepage || HomepageHelper.resolve(@theme_id, current_user)
end end
def serialize_data(obj, serializer, opts = nil) def serialize_data(obj, serializer, opts = nil)

View File

@ -560,7 +560,7 @@ module ApplicationHelper
end end
def current_homepage def current_homepage
current_user&.user_option&.homepage || HomepageHelper.resolve(request, current_user) current_user&.user_option&.homepage || HomepageHelper.resolve(theme_id, current_user)
end end
def build_plugin_html(name) def build_plugin_html(name)

View File

@ -1598,7 +1598,9 @@ Discourse::Application.routes.draw do
constraints: HomePageConstraint.new("finish_installation"), constraints: HomePageConstraint.new("finish_installation"),
as: "installation_redirect" as: "installation_redirect"
root to: "custom#index", constraints: HomePageConstraint.new("custom"), as: "custom_index" root to: "custom_homepage#index",
constraints: HomePageConstraint.new("custom"),
as: "custom_index"
get "/user-api-key/new" => "user_api_keys#new" get "/user-api-key/new" => "user_api_keys#new"
post "/user-api-key" => "user_api_keys#create" post "/user-api-key" => "user_api_keys#create"

View File

@ -9,7 +9,11 @@ class HomePageConstraint
return @filter == "finish_installation" if SiteSetting.has_login_hint? return @filter == "finish_installation" if SiteSetting.has_login_hint?
current_user = CurrentUser.lookup_from_env(request.env) current_user = CurrentUser.lookup_from_env(request.env)
homepage = current_user&.user_option&.homepage || HomepageHelper.resolve(request, current_user)
# ensures we resolve the theme id as early as possible
theme_id = ThemeResolver.resolve_theme_id(request, Guardian.new(current_user), current_user)
homepage = current_user&.user_option&.homepage || HomepageHelper.resolve(theme_id, current_user)
homepage == @filter homepage == @filter
rescue Discourse::InvalidAccess, Discourse::ReadOnly rescue Discourse::InvalidAccess, Discourse::ReadOnly
false false

View File

@ -1,8 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
class HomepageHelper class HomepageHelper
def self.resolve(request = nil, current_user = nil) def self.resolve(theme_id = nil, current_user = nil)
return "custom" if ThemeModifierHelper.new(request: request).custom_homepage return "custom" if ThemeModifierHelper.new(theme_ids: theme_id).custom_homepage
current_user ? SiteSetting.homepage : SiteSetting.anonymous_homepage current_user ? SiteSetting.homepage : SiteSetting.anonymous_homepage
end end

36
lib/theme_resolver.rb Normal file
View File

@ -0,0 +1,36 @@
# frozen_string_literal: true
module ThemeResolver
def self.resolve_theme_id(request, guardian, current_user)
return request.env[:resolved_theme_id] if request.env[:resolved_theme_id] != nil
theme_id = nil
if (preview_theme_id = request[:preview_theme_id]&.to_i) &&
guardian.allow_themes?([preview_theme_id], include_preview: true)
theme_id = preview_theme_id
end
user_option = current_user&.user_option
if theme_id.blank? && request.cookie_jar[:theme_ids].present?
ids, seq = request.cookie_jar[:theme_ids]&.split("|")
id = ids&.split(",")&.map(&:to_i)&.first
if id.present? && seq && seq.to_i == user_option&.theme_key_seq.to_i
theme_id = id if guardian.allow_themes?([id])
end
end
if theme_id.blank?
ids = user_option&.theme_ids || []
theme_id = ids.first if guardian.allow_themes?(ids)
end
if theme_id.blank? && SiteSetting.default_theme_id != -1 &&
guardian.allow_themes?([SiteSetting.default_theme_id])
theme_id = SiteSetting.default_theme_id
end
request.env[:resolved_theme_id] = theme_id
end
end

View File

@ -14,7 +14,7 @@ RSpec.describe HomepageHelper do
expect(HomepageHelper.resolve).to eq("custom") expect(HomepageHelper.resolve).to eq("custom")
end end
context "when first item in top menu is no valid for anons" do context "when first item in top menu is not valid for anons" do
it "distinguishes between auth homepage and anon homepage" do it "distinguishes between auth homepage and anon homepage" do
SiteSetting.top_menu = "new|top|latest|unread" SiteSetting.top_menu = "new|top|latest|unread"