From f4108702c8c90564c78d618013b5820d884df846 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Fri, 21 Jun 2024 11:24:11 -0400 Subject: [PATCH] FIX: Regression in custom homepage modifier used in theme components (#27569) --- app/controllers/application_controller.rb | 2 +- app/helpers/application_helper.rb | 2 +- lib/homepage_constraint.rb | 5 +- lib/homepage_helper.rb | 4 +- spec/system/homepage_spec.rb | 143 +++++++++++++--------- 5 files changed, 91 insertions(+), 65 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 001a7735139..219786ea9b7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -492,7 +492,7 @@ class ApplicationController < ActionController::Base end def current_homepage - current_user&.user_option&.homepage || HomepageHelper.resolve(@theme_id, current_user) + current_user&.user_option&.homepage || HomepageHelper.resolve(request, current_user) end def serialize_data(obj, serializer, opts = nil) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 9817ac85a06..e0a0d22a1ef 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -560,7 +560,7 @@ module ApplicationHelper end def current_homepage - current_user&.user_option&.homepage || HomepageHelper.resolve(theme_id, current_user) + current_user&.user_option&.homepage || HomepageHelper.resolve(request, current_user) end def build_plugin_html(name) diff --git a/lib/homepage_constraint.rb b/lib/homepage_constraint.rb index 3c6fbd2bd40..a80cfbd55f7 100644 --- a/lib/homepage_constraint.rb +++ b/lib/homepage_constraint.rb @@ -9,11 +9,10 @@ class HomePageConstraint return @filter == "finish_installation" if SiteSetting.has_login_hint? current_user = CurrentUser.lookup_from_env(request.env) - # ensures we resolve the theme id as early as possible - theme_id = ThemeResolver.resolve_theme_id(request, Guardian.new(current_user), current_user) + ThemeResolver.resolve_theme_id(request, Guardian.new(current_user), current_user) - homepage = current_user&.user_option&.homepage || HomepageHelper.resolve(theme_id, current_user) + homepage = current_user&.user_option&.homepage || HomepageHelper.resolve(request, current_user) homepage == @filter rescue Discourse::InvalidAccess, Discourse::ReadOnly false diff --git a/lib/homepage_helper.rb b/lib/homepage_helper.rb index 9eb684508f8..c4a77a24482 100644 --- a/lib/homepage_helper.rb +++ b/lib/homepage_helper.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true class HomepageHelper - def self.resolve(theme_id = nil, current_user = nil) - return "custom" if ThemeModifierHelper.new(theme_ids: theme_id).custom_homepage + def self.resolve(request = nil, current_user = nil) + return "custom" if ThemeModifierHelper.new(request: request).custom_homepage current_user ? SiteSetting.homepage : SiteSetting.anonymous_homepage end diff --git a/spec/system/homepage_spec.rb b/spec/system/homepage_spec.rb index cd9004021f4..2be3c5d8033 100644 --- a/spec/system/homepage_spec.rb +++ b/spec/system/homepage_spec.rb @@ -53,6 +53,64 @@ describe "Homepage", type: :system do expect(page).to have_css(".navigation-container .categories.active", text: "Categories") end + shared_examples "a custom homepage" do + it "shows the custom homepage component" do + visit "/" + + expect(page).to have_css(".new-home", text: "Hi friends!") + expect(page).to have_no_css(".list-container") + + find("#sidebar-section-content-community li:first-child").click + expect(page).to have_css(".list-container") + + find("#site-logo").click + + expect(page).to have_no_css(".list-container") + # ensure clicking on logo brings user back to the custom homepage + expect(page).to have_css(".new-home", text: "Hi friends!") + end + + it "respects the user's homepage choice" do + visit "/" + + expect(page).not_to have_css(".list-container") + expect(page).to have_css(".new-home", text: "Hi friends!") + + sign_in user + + visit "/u/#{user.username}/preferences/interface" + + homepage_picker = PageObjects::Components::SelectKit.new("#home-selector") + homepage_picker.expand + # user overrides theme custom homepage + homepage_picker.select_row_by_name("Top") + page.find(".btn-primary.save-changes").click + + # Wait for the save to complete + find(".btn-primary.save-changes:not([disabled])", wait: 5) + + find("#site-logo").click + + expect(page).to have_css(".navigation-container .top.active", text: "Top") + expect(page).to have_css(".top-lists") + + visit "/u/#{user.username}/preferences/interface" + + homepage_picker = PageObjects::Components::SelectKit.new("#home-selector") + homepage_picker.expand + # user selects theme custom homepage again + homepage_picker.select_row_by_name("(default)") + page.find(".btn-primary.save-changes").click + + # Wait for the save to complete + find(".btn-primary.save-changes:not([disabled])", wait: 5) + find("#site-logo").click + + expect(page).not_to have_css(".list-container") + expect(page).to have_css(".new-home", text: "Hi friends!") + end + end + context "when default theme uses a custom_homepage modifier" do before do theme.theme_modifier_set.custom_homepage = true @@ -76,64 +134,6 @@ describe "Homepage", type: :system do expect(page).to have_css(".alert-info") end - shared_examples "a custom homepage" do - it "shows the custom homepage component" do - visit "/" - - expect(page).to have_css(".new-home", text: "Hi friends!") - expect(page).to have_no_css(".list-container") - - find("#sidebar-section-content-community li:first-child").click - expect(page).to have_css(".list-container") - - find("#site-logo").click - - expect(page).to have_no_css(".list-container") - # ensure clicking on logo brings user back to the custom homepage - expect(page).to have_css(".new-home", text: "Hi friends!") - end - - it "respects the user's homepage choice" do - visit "/" - - expect(page).not_to have_css(".list-container") - expect(page).to have_css(".new-home", text: "Hi friends!") - - sign_in user - - visit "/u/#{user.username}/preferences/interface" - - homepage_picker = PageObjects::Components::SelectKit.new("#home-selector") - homepage_picker.expand - # user overrides theme custom homepage - homepage_picker.select_row_by_name("Top") - page.find(".btn-primary.save-changes").click - - # Wait for the save to complete - find(".btn-primary.save-changes:not([disabled])", wait: 5) - - find("#site-logo").click - - expect(page).to have_css(".navigation-container .top.active", text: "Top") - expect(page).to have_css(".top-lists") - - visit "/u/#{user.username}/preferences/interface" - - homepage_picker = PageObjects::Components::SelectKit.new("#home-selector") - homepage_picker.expand - # user selects theme custom homepage again - homepage_picker.select_row_by_name("(default)") - page.find(".btn-primary.save-changes").click - - # Wait for the save to complete - find(".btn-primary.save-changes:not([disabled])", wait: 5) - find("#site-logo").click - - expect(page).not_to have_css(".list-container") - expect(page).to have_css(".new-home", text: "Hi friends!") - end - end - context "when the theme adds content to the [custom-homepage] connector" do let!(:basic_html_field) do Fabricate( @@ -175,4 +175,31 @@ describe "Homepage", type: :system do include_examples "a custom homepage" end end + + context "when a theme component uses the custom_homepage modifier" do + let!(:component) { Fabricate(:theme, component: true) } + let!(:component_html_field) do + Fabricate( + :theme_field, + theme: component, + type_id: ThemeField.types[:html], + target_id: Theme.targets[:common], + name: "head_tag", + value: <<~HTML, + + HTML + ) + end + + before do + component.theme_modifier_set.custom_homepage = true + component.theme_modifier_set.save! + theme.add_relative_theme!(:child, component) + theme.set_default! + end + + include_examples "a custom homepage" + end end