From 44aa46ca05939cc986ae8e42b24b6101f49e8b5a Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Fri, 18 Jun 2021 10:16:26 +0800 Subject: [PATCH] Code review comments. --- .../discourse/app/lib/theme-selector.js | 2 +- .../discourse/lib/bootstrap-json/index.js | 4 +- .../user-preferences-interface-test.js | 4 +- app/controllers/bootstrap_controller.rb | 2 +- app/controllers/stylesheets_controller.rb | 2 +- app/models/theme.rb | 6 +-- .../_discourse_publish_stylesheet.html.erb | 2 +- app/views/layouts/application.html.erb | 10 ++++- app/views/layouts/embed.html.erb | 2 +- .../layouts/finish_installation.html.erb | 2 +- app/views/layouts/no_ember.html.erb | 27 +++++++++--- app/views/qunit/index.html.erb | 4 +- app/views/qunit/theme.html.erb | 4 +- app/views/wizard/index.html.erb | 2 +- app/views/wizard/qunit.html.erb | 4 +- lib/stylesheet/importer.rb | 2 + lib/stylesheet/manager.rb | 25 +++++++---- lib/stylesheet/manager/builder.rb | 18 ++++---- lib/svg_sprite/svg_sprite.rb | 9 ++-- lib/theme_modifier_helper.rb | 2 +- spec/components/stylesheet/manager_spec.rb | 6 --- spec/components/svg_sprite/svg_sprite_spec.rb | 11 ++++- spec/fixtures/onebox/discourse_topic.response | 2 +- .../onebox/discourse_topic_reply.response | 2 +- spec/lib/content_security_policy_spec.rb | 16 ++++++-- spec/requests/bootstrap_controller_spec.rb | 2 +- spec/requests/safe_mode_controller_spec.rb | 8 ++++ spec/requests/svg_sprite_controller_spec.rb | 2 +- .../listable_topic_serializer_spec.rb | 41 +++++++++++++++++++ 29 files changed, 159 insertions(+), 64 deletions(-) create mode 100644 spec/serializers/listable_topic_serializer_spec.rb diff --git a/app/assets/javascripts/discourse/app/lib/theme-selector.js b/app/assets/javascripts/discourse/app/lib/theme-selector.js index 32326b4a88b..fdc4cb9517d 100644 --- a/app/assets/javascripts/discourse/app/lib/theme-selector.js +++ b/app/assets/javascripts/discourse/app/lib/theme-selector.js @@ -2,7 +2,7 @@ import cookie, { removeCookie } from "discourse/lib/cookie"; import I18n from "I18n"; import deprecated from "discourse-common/lib/deprecated"; -const keySelector = "meta[name=discourse_theme_ids]"; +const keySelector = "meta[name=discourse_theme_id]"; export function currentThemeKey() { // eslint-disable-next-line no-console diff --git a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js index 7c78d2d1f50..18f7b80c0fe 100644 --- a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js +++ b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js @@ -34,9 +34,9 @@ function head(buffer, bootstrap) { buffer.push(``); buffer.push(``); } - if (bootstrap.theme_ids) { + if (bootstrap.theme_id) { buffer.push( - `` + `` ); } diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-interface-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-interface-test.js index 6869f9fbad9..312fd2a82a1 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-interface-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-interface-test.js @@ -100,7 +100,7 @@ acceptance("User Preferences - Interface", function (needs) { test("shows no default option for light scheme when theme's color scheme is user selectable", async function (assert) { let meta = document.createElement("meta"); - meta.name = "discourse_theme_ids"; + meta.name = "discourse_theme_id"; meta.content = "2"; document.getElementsByTagName("head")[0].appendChild(meta); @@ -128,7 +128,7 @@ acceptance("User Preferences - Interface", function (needs) { await selectKit(".light-color-scheme .select-kit").expand(); assert.equal(count(".light-color-scheme .select-kit .select-kit-row"), 2); - document.querySelector("meta[name='discourse_theme_ids']").remove(); + document.querySelector("meta[name='discourse_theme_id']").remove(); }); }); diff --git a/app/controllers/bootstrap_controller.rb b/app/controllers/bootstrap_controller.rb index 5bb4dc6ddb3..7840dcab3e2 100644 --- a/app/controllers/bootstrap_controller.rb +++ b/app/controllers/bootstrap_controller.rb @@ -51,7 +51,7 @@ class BootstrapController < ApplicationController ).map { |f| script_asset_path(f) } bootstrap = { - theme_ids: [theme_id], + theme_id: theme_id, title: SiteSetting.title, current_homepage: current_homepage, locale_script: locale, diff --git a/app/controllers/stylesheets_controller.rb b/app/controllers/stylesheets_controller.rb index 427f2b9203a..df53cf58030 100644 --- a/app/controllers/stylesheets_controller.rb +++ b/app/controllers/stylesheets_controller.rb @@ -47,7 +47,7 @@ class StylesheetsController < ApplicationController theme_id = if target.include?("theme") split_target, theme_id = target.split(/_(-?[0-9]+)/) - Theme.where(id: theme_id).pluck_first(:id) if theme_id.present? + theme_id if theme_id.present? && Theme.exists?(id: theme_id) else split_target, color_scheme_id = target.split(/_(-?[0-9]+)/) Theme.where(color_scheme_id: color_scheme_id).pluck_first(:id) diff --git a/app/models/theme.rb b/app/models/theme.rb index ad5f63be4c5..fe71405f192 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -203,6 +203,7 @@ class Theme < ActiveRecord::Base def self.transform_ids(id) return [] if id.blank? + id = id.to_i get_set_cache "transformed_ids_#{id}" do all_ids = @@ -297,8 +298,7 @@ class Theme < ActiveRecord::Base end def self.lookup_modifier(theme_ids, modifier_name) - theme_ids = [theme_ids] unless Array === theme_ids - theme_ids = transform_ids(theme_ids) + theme_ids = [theme_ids] unless theme_ids.is_a?(Array) get_set_cache("#{theme_ids.join(",")}:modifier:#{modifier_name}:#{Theme.compiler_version}") do ThemeModifierSet.resolve_modifier_for_themes(theme_ids, modifier_name) @@ -350,7 +350,7 @@ class Theme < ActiveRecord::Base end def self.refresh_message_for_targets(targets, theme_ids) - theme_ids = [theme_ids] unless theme_ids === Array + theme_ids = [theme_ids] unless theme_ids.is_a?(Array) targets.each_with_object([]) do |target, data| theme_ids.each do |theme_id| diff --git a/app/views/common/_discourse_publish_stylesheet.html.erb b/app/views/common/_discourse_publish_stylesheet.html.erb index 90fe9a9f9e2..ae48d98f98f 100644 --- a/app/views/common/_discourse_publish_stylesheet.html.erb +++ b/app/views/common/_discourse_publish_stylesheet.html.erb @@ -1,4 +1,4 @@ -<%= discourse_stylesheet_link_tag 'publish', theme_ids: nil %> +<%= discourse_stylesheet_link_tag 'publish', theme_id: nil %> <%- if rtl? %> <%= discourse_stylesheet_link_tag(mobile_view? ? :publish_mobile_rtl : :publish_mobile_rtl) %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 47159a614a5..b5ec502ee61 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -5,7 +5,7 @@ <%= content_for?(:title) ? yield(:title) : SiteSetting.title %> - + <%= render partial: "layouts/head" %> <%= discourse_csrf_tags %> @@ -90,6 +90,9 @@ <%- unless customization_disabled? %> <%= theme_lookup("header") %> + <%- end %> + + <%- if allow_plugins? %> <%= build_plugin_html 'server:header' %> <%- end %> @@ -115,6 +118,9 @@ <%- unless customization_disabled? %> <%= theme_lookup("body_tag") %> <%- end %> - <%= build_plugin_html 'server:before-body-close' %> + + <%- if allow_plugins? %> + <%= build_plugin_html 'server:before-body-close' %> + <%- end %> diff --git a/app/views/layouts/embed.html.erb b/app/views/layouts/embed.html.erb index 42b70d08fe5..90711df5d2c 100644 --- a/app/views/layouts/embed.html.erb +++ b/app/views/layouts/embed.html.erb @@ -3,7 +3,7 @@ - <%= discourse_stylesheet_link_tag 'embed', theme_ids: nil %> + <%= discourse_stylesheet_link_tag 'embed', theme_id: nil %> <%- unless customization_disabled? %> <%= discourse_stylesheet_link_tag :embedded_theme %> <%- end %> diff --git a/app/views/layouts/finish_installation.html.erb b/app/views/layouts/finish_installation.html.erb index 22c2b04e55f..4cff210af79 100644 --- a/app/views/layouts/finish_installation.html.erb +++ b/app/views/layouts/finish_installation.html.erb @@ -1,6 +1,6 @@ - <%= discourse_stylesheet_link_tag 'wizard', theme_ids: nil %> + <%= discourse_stylesheet_link_tag 'wizard', theme_id: nil %> <%= discourse_color_scheme_stylesheets %> <%= render partial: "layouts/head" %> diff --git a/app/views/layouts/no_ember.html.erb b/app/views/layouts/no_ember.html.erb index b36b9205b5d..c11948d364c 100644 --- a/app/views/layouts/no_ember.html.erb +++ b/app/views/layouts/no_ember.html.erb @@ -8,13 +8,22 @@ <%= render partial: "common/discourse_stylesheet" %> <%= discourse_csrf_tags %> - <%= theme_lookup("head_tag") %> + <%- unless customization_disabled? %> + <%= theme_lookup("head_tag") %> + <%- end %> + <%= yield(:no_ember_head) %> - <%= build_plugin_html 'server:before-head-close' %> + + <%- if allow_plugins? %> + <%= build_plugin_html 'server:before-head-close' %> + <%- end -%> <%- unless customization_disabled? %> <%= theme_lookup("header") %> + <%- end %> + + <%- if allow_plugins? %> <%= build_plugin_html 'server:header' %> <%- end %> @@ -24,9 +33,15 @@ <%= yield %> - <%= theme_lookup("footer") %> - <%= theme_lookup("body_tag") %> - <%= build_plugin_html 'no-client:footer' %> - <%= build_plugin_html 'server:before-body-close' %> + + <%- unless customization_disabled? %> + <%= theme_lookup("footer") %> + <%= theme_lookup("body_tag") %> + <%- end %> + + <%- if allow_plugins? %> + <%= build_plugin_html 'no-client:footer' %> + <%= build_plugin_html 'server:before-body-close' %> + <%- end %> diff --git a/app/views/qunit/index.html.erb b/app/views/qunit/index.html.erb index d4d072376e3..00c92302e02 100644 --- a/app/views/qunit/index.html.erb +++ b/app/views/qunit/index.html.erb @@ -3,8 +3,8 @@ QUnit Test Runner <%= discourse_color_scheme_stylesheets %> - <%= discourse_stylesheet_link_tag(:desktop, theme_ids: nil) %> - <%= discourse_stylesheet_link_tag(:test_helper, theme_ids: nil) %> + <%= discourse_stylesheet_link_tag(:desktop, theme_id: nil) %> + <%= discourse_stylesheet_link_tag(:test_helper, theme_id: nil) %> <%= preload_script "discourse/tests/test_helper" %> <%= preload_script "discourse/tests/core_plugins_tests" %> <%= preload_script "discourse/tests/test_starter" %> diff --git a/app/views/qunit/theme.html.erb b/app/views/qunit/theme.html.erb index 3016de7711f..412dcb8c0d1 100644 --- a/app/views/qunit/theme.html.erb +++ b/app/views/qunit/theme.html.erb @@ -4,8 +4,8 @@ Theme QUnit Test Runner <%= discourse_color_scheme_stylesheets %> <%- if !@suggested_themes %> - <%= discourse_stylesheet_link_tag(:desktop, theme_ids: nil) %> - <%= discourse_stylesheet_link_tag(:test_helper, theme_ids: nil) %> + <%= discourse_stylesheet_link_tag(:desktop, theme_id: nil) %> + <%= discourse_stylesheet_link_tag(:test_helper, theme_id: nil) %> <%= preload_script "locales/en" %> <%= preload_script "discourse/tests/theme_qunit_ember_jquery" %> <%= preload_script "discourse/tests/theme_qunit_vendor" %> diff --git a/app/views/wizard/index.html.erb b/app/views/wizard/index.html.erb index 5765863cfa7..7fb1c7332c9 100644 --- a/app/views/wizard/index.html.erb +++ b/app/views/wizard/index.html.erb @@ -1,6 +1,6 @@ - <%= discourse_stylesheet_link_tag :wizard, theme_ids: nil %> + <%= discourse_stylesheet_link_tag :wizard, theme_id: nil %> <%= discourse_color_scheme_stylesheets %> <%= preload_script "locales/#{I18n.locale}" %> <%- if ExtraLocalesController.client_overrides_exist? %> diff --git a/app/views/wizard/qunit.html.erb b/app/views/wizard/qunit.html.erb index f1e767d9c0a..52e1ffaf901 100644 --- a/app/views/wizard/qunit.html.erb +++ b/app/views/wizard/qunit.html.erb @@ -2,8 +2,8 @@ QUnit Test Runner - <%= discourse_stylesheet_link_tag(:test_helper, theme_ids: nil) %> - <%= discourse_stylesheet_link_tag :wizard, theme_ids: nil %> + <%= discourse_stylesheet_link_tag(:test_helper, theme_id: nil) %> + <%= discourse_stylesheet_link_tag :wizard, theme_id: nil %> <%= preload_script "wizard/test/test_helper" %> <%= csrf_meta_tags %> diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb index d9282465976..7826354e26d 100644 --- a/lib/stylesheet/importer.rb +++ b/lib/stylesheet/importer.rb @@ -173,6 +173,8 @@ module Stylesheet end def theme_import(target) + return "" if !@theme_id + attr = target == :embedded_theme ? :embedded_scss : :scss target = target.to_s.gsub("_theme", "").to_sym diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index 2f4b6791619..42f96e45535 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -134,7 +134,7 @@ class Stylesheet::Manager attr_reader :theme_ids def initialize(theme_id: nil) - @theme_id = theme_id || SiteSetting.default_theme_id + @theme_id = theme_id @theme_ids = Theme.transform_ids(@theme_id) @themes_cache = {} end @@ -218,16 +218,23 @@ class Stylesheet::Manager scss_checker = ScssChecker.new(target, stale_theme_ids) - load_themes(stale_theme_ids).each do |theme| - theme_id = theme.id + themes = @theme_id.blank? ? [nil] : load_themes(stale_theme_ids) + + themes.each do |theme| + theme_id = theme&.id data = { target: target, theme_id: theme_id } builder = Builder.new(target: target, theme: theme, manager: self) + is_theme = builder.is_theme? + has_theme = builder.theme.present? - next if builder.theme.component && !scss_checker.has_scss(theme_id) - builder.compile unless File.exists?(builder.stylesheet_fullpath) - href = builder.stylesheet_path(current_hostname) - - cache.defer_set("path_#{target}_#{theme_id}_#{current_hostname}", href) + if is_theme && !has_theme + next + else + next if builder.theme&.component && !scss_checker.has_scss(theme_id) + builder.compile unless File.exists?(builder.stylesheet_fullpath) + href = builder.stylesheet_path(current_hostname) + cache.defer_set("path_#{target}_#{theme_id}_#{current_hostname}", href) + end data[:new_href] = href stylesheets << data @@ -239,7 +246,7 @@ class Stylesheet::Manager end def color_scheme_stylesheet_details(color_scheme_id = nil, media) - theme_id = @theme_ids.first + theme_id = @theme_id || SiteSetting.default_theme_id color_scheme = begin ColorScheme.find(color_scheme_id) diff --git a/lib/stylesheet/manager/builder.rb b/lib/stylesheet/manager/builder.rb index a04de17edea..1265c286a6c 100644 --- a/lib/stylesheet/manager/builder.rb +++ b/lib/stylesheet/manager/builder.rb @@ -3,7 +3,7 @@ class Stylesheet::Manager::Builder attr_reader :theme - def initialize(target: :desktop, theme:, color_scheme: nil, manager:) + def initialize(target: :desktop, theme: nil, color_scheme: nil, manager:) @target = target @theme = theme @color_scheme = color_scheme @@ -115,11 +115,11 @@ class Stylesheet::Manager::Builder def qualified_target if is_theme? - "#{@target}_#{theme.id}" + "#{@target}_#{theme&.id}" elsif @color_scheme "#{@target}_#{scheme_slug}_#{@color_scheme&.id.to_s}" else - scheme_string = theme && theme.color_scheme ? "_#{theme.color_scheme.id}" : "" + scheme_string = theme&.color_scheme ? "_#{theme.color_scheme.id}" : "" "#{@target}#{scheme_string}" end end @@ -186,10 +186,10 @@ class Stylesheet::Manager::Builder end def settings_digest - theme_ids = Theme.is_parent_theme?(theme.id) ? @manager.theme_ids : [theme.id] - themes = - if Theme.is_parent_theme?(theme.id) + if !theme + [] + elsif Theme.is_parent_theme?(theme.id) @manager.load_themes(@manager.theme_ids) else [@manager.get_theme(theme.id)] @@ -211,7 +211,7 @@ class Stylesheet::Manager::Builder def uploads_digest sha1s = [] - theme.upload_fields.map do |upload_field| + (theme&.upload_fields || []).map do |upload_field| sha1s << upload_field.upload.sha1 end @@ -247,7 +247,9 @@ class Stylesheet::Manager::Builder def resolve_baked_field(target, name) theme_ids = - if Theme.is_parent_theme?(theme.id) + if !theme + [] + elsif Theme.is_parent_theme?(theme.id) @manager.theme_ids else [theme.id] diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb index 0754ab7b2a4..c9eb7340c3d 100644 --- a/lib/svg_sprite/svg_sprite.rb +++ b/lib/svg_sprite/svg_sprite.rb @@ -358,7 +358,7 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL def self.search(searched_icon) searched_icon = process(searched_icon.dup) - sprite_sources([SiteSetting.default_theme_id]).each do |fname| + sprite_sources(SiteSetting.default_theme_id).each do |fname| next if !File.exist?(fname) svg_file = Nokogiri::XML(File.open(fname)) @@ -381,7 +381,7 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL def self.icon_picker_search(keyword) results = Set.new - sprite_sources([SiteSetting.default_theme_id]).each do |fname| + sprite_sources(SiteSetting.default_theme_id).each do |fname| next if !File.exist?(fname) svg_file = Nokogiri::XML(File.open(fname)) @@ -465,9 +465,10 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL return [] if theme_id.blank? theme_icon_settings = [] + theme_ids = Theme.transform_ids(theme_id) # Need to load full records for default values - Theme.where(id: Theme.transform_ids(theme_id)).each do |theme| + Theme.where(id: theme_ids).each do |theme| settings = theme.cached_settings.each do |key, value| if key.to_s.include?("_icon") && String === value theme_icon_settings |= value.split('|') @@ -475,7 +476,7 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL end end - theme_icon_settings |= ThemeModifierHelper.new(theme_ids: [theme_id]).svg_icons + theme_icon_settings |= ThemeModifierHelper.new(theme_ids: theme_ids).svg_icons theme_icon_settings end diff --git a/lib/theme_modifier_helper.rb b/lib/theme_modifier_helper.rb index 2299d179f9c..8f6be146916 100644 --- a/lib/theme_modifier_helper.rb +++ b/lib/theme_modifier_helper.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ThemeModifierHelper def initialize(request: nil, theme_ids: nil) - @theme_ids = theme_ids || [request&.env&.[](:resolved_theme_id)] + @theme_ids = theme_ids || Theme.transform_ids(request&.env&.[](:resolved_theme_id)) end ThemeModifierSet.modifiers.keys.each do |modifier| diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index cf4b35e8e45..7b64c8d4d5c 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -12,12 +12,6 @@ describe Stylesheet::Manager do Theme.clear_default! link = manager.stylesheet_link_tag(:embedded_theme) expect(link).to eq("") - - theme = Fabricate(:theme) - SiteSetting.default_theme_id = theme.id - - link = manager.stylesheet_link_tag(:embedded_theme) - expect(link).not_to eq("") end it "still returns something for no themes" do diff --git a/spec/components/svg_sprite/svg_sprite_spec.rb b/spec/components/svg_sprite/svg_sprite_spec.rb index 3494d6f1723..584800ea028 100644 --- a/spec/components/svg_sprite/svg_sprite_spec.rb +++ b/spec/components/svg_sprite/svg_sprite_spec.rb @@ -139,12 +139,21 @@ describe SvgSprite do it 'includes icons defined in theme modifiers' do theme = Fabricate(:theme) + child_theme = Fabricate(:theme, component: true) + theme.add_relative_theme!(:child, child_theme) expect(SvgSprite.all_icons(theme.id)).not_to include("dragon") theme.theme_modifier_set.svg_icons = ["dragon"] theme.save! - expect(SvgSprite.all_icons(theme.id)).to include("dragon") + + child_theme.theme_modifier_set.svg_icons = ["fly"] + child_theme.save! + + icons = SvgSprite.all_icons(theme.id) + + expect(icons).to include("dragon") + expect(icons).to include("fly") end it 'includes custom icons from a sprite in a theme' do diff --git a/spec/fixtures/onebox/discourse_topic.response b/spec/fixtures/onebox/discourse_topic.response index 00a0e24fe99..18ef0d722f1 100644 --- a/spec/fixtures/onebox/discourse_topic.response +++ b/spec/fixtures/onebox/discourse_topic.response @@ -9,7 +9,7 @@ [image] And that too in just over an year, way to go! [boom]"> - + diff --git a/spec/fixtures/onebox/discourse_topic_reply.response b/spec/fixtures/onebox/discourse_topic_reply.response index d204320e713..36a5e7a4008 100644 --- a/spec/fixtures/onebox/discourse_topic_reply.response +++ b/spec/fixtures/onebox/discourse_topic_reply.response @@ -9,7 +9,7 @@ [image] And that too in just over an year, way to go! [boom]"> - + diff --git a/spec/lib/content_security_policy_spec.rb b/spec/lib/content_security_policy_spec.rb index 32a4db46d34..20ea52d3b25 100644 --- a/spec/lib/content_security_policy_spec.rb +++ b/spec/lib/content_security_policy_spec.rb @@ -274,7 +274,7 @@ describe ContentSecurityPolicy do } def theme_policy - policy([theme.id]) + policy(theme.id) end it 'can be extended by themes' do @@ -303,13 +303,23 @@ describe ContentSecurityPolicy do theme.theme_modifier_set.csp_extensions = ["script-src: https://from-theme-flag.script", "worker-src: from-theme-flag.worker"] theme.save! + child_theme = Fabricate(:theme, component: true) + theme.add_relative_theme!(:child, child_theme) + child_theme.theme_modifier_set.csp_extensions = ["script-src: https://child-theme-flag.script", "worker-src: child-theme-flag.worker"] + child_theme.save! + expect(parse(theme_policy)['script-src']).to include('https://from-theme-flag.script') + expect(parse(theme_policy)['script-src']).to include('https://child-theme-flag.script') expect(parse(theme_policy)['worker-src']).to include('from-theme-flag.worker') + expect(parse(theme_policy)['worker-src']).to include('child-theme-flag.worker') theme.destroy! + child_theme.destroy! expect(parse(theme_policy)['script-src']).to_not include('https://from-theme-flag.script') expect(parse(theme_policy)['worker-src']).to_not include('from-theme-flag.worker') + expect(parse(theme_policy)['worker-src']).to_not include('from-theme-flag.worker') + expect(parse(theme_policy)['worker-src']).to_not include('child-theme-flag.worker') end it 'is extended automatically when themes reference external scripts' do @@ -352,7 +362,7 @@ describe ContentSecurityPolicy do end.to_h end - def policy(theme_ids = [], path_info: "/") - ContentSecurityPolicy.policy(theme_ids, path_info: path_info) + def policy(theme_id = nil, path_info: "/") + ContentSecurityPolicy.policy(theme_id, path_info: path_info) end end diff --git a/spec/requests/bootstrap_controller_spec.rb b/spec/requests/bootstrap_controller_spec.rb index 182c307191c..cd61f711146 100644 --- a/spec/requests/bootstrap_controller_spec.rb +++ b/spec/requests/bootstrap_controller_spec.rb @@ -27,7 +27,7 @@ describe BootstrapController do bootstrap = json['bootstrap'] expect(bootstrap).to be_present expect(bootstrap['title']).to be_present - expect(bootstrap['theme_ids']).to eq([theme.id]) + expect(bootstrap['theme_id']).to eq(theme.id) expect(bootstrap['setup_data']['base_url']).to eq(Discourse.base_url) expect(bootstrap['stylesheets']).to be_present diff --git a/spec/requests/safe_mode_controller_spec.rb b/spec/requests/safe_mode_controller_spec.rb index 871e1c5dc33..12e1f330668 100644 --- a/spec/requests/safe_mode_controller_spec.rb +++ b/spec/requests/safe_mode_controller_spec.rb @@ -10,10 +10,18 @@ RSpec.describe SafeModeController do theme.save! theme.set_default! + Fabricate(:admin) # Avoid wizard page + + get '/' + + expect(response.status).to eq(200) + expect(response.body).to include("data-theme-id=\"#{theme.id}\"") + get '/safe-mode' expect(response.status).to eq(200) expect(response.body).not_to include("My Custom Header") + expect(response.body).not_to include("data-theme-id=\"#{theme.id}\"") end end diff --git a/spec/requests/svg_sprite_controller_spec.rb b/spec/requests/svg_sprite_controller_spec.rb index 60584b8ea8b..047e0e462fe 100644 --- a/spec/requests/svg_sprite_controller_spec.rb +++ b/spec/requests/svg_sprite_controller_spec.rb @@ -16,7 +16,7 @@ describe SvgSpriteController do theme = Fabricate(:theme) theme.set_field(target: :settings, name: :yaml, value: "custom_icon: dragon") theme.save! - get "/svg-sprite/#{Discourse.current_hostname}/svg-#{theme.id}-#{SvgSprite.version([theme.id])}.js" + get "/svg-sprite/#{Discourse.current_hostname}/svg-#{theme.id}-#{SvgSprite.version(theme.id)}.js" expect(response.status).to eq(200) end diff --git a/spec/serializers/listable_topic_serializer_spec.rb b/spec/serializers/listable_topic_serializer_spec.rb new file mode 100644 index 00000000000..098a5034e5e --- /dev/null +++ b/spec/serializers/listable_topic_serializer_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true +require 'rails_helper' + +describe ListableTopicSerializer do + fab!(:topic) { Fabricate(:topic) } + + describe '#excerpt' do + it 'can be extended by theme modifiers' do + payload = TopicListItemSerializer.new(topic, + scope: Guardian.new, + root: false + ).as_json + + expect(payload[:excerpt]).to eq(nil) + + theme = Fabricate(:theme) + + child_theme = Fabricate(:theme, component: true).tap do |t| + theme.add_relative_theme!(:child, t) + end + + child_theme.theme_modifier_set.serialize_topic_excerpts = true + child_theme.save! + + request = ActionController::TestRequest.new( + { resolved_theme_id: theme.id }, + nil, + nil + ) + + guardian = Guardian.new(nil, request) + + payload = TopicListItemSerializer.new(topic, + scope: guardian, + root: false + ).as_json + + expect(payload[:excerpt]).to eq(topic.excerpt) + end + end +end