From d82c58e6cc5dd43b8bfb062838a24cac85ac9ca2 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 21 Jun 2021 13:54:42 -0400 Subject: [PATCH] DEV: Add simple digest for core stylesheets And move fonts + category_backgrounds to color definitions stylesheet. This will let us use the same core + plugin stylesheets in multisite. --- lib/stylesheet/compiler.rb | 5 ++-- lib/stylesheet/manager/builder.rb | 14 ++++++++-- spec/components/stylesheet/importer_spec.rb | 19 +++++-------- spec/components/stylesheet/manager_spec.rb | 31 ++++++++++++--------- 4 files changed, 39 insertions(+), 30 deletions(-) diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb index efc47185774..72231f41635 100644 --- a/lib/stylesheet/compiler.rb +++ b/lib/stylesheet/compiler.rb @@ -29,9 +29,6 @@ module Stylesheet file += File.read path case asset.to_s - when "desktop", "mobile" - file += importer.category_backgrounds - file += importer.font when "embed", "publish" file += importer.font when "wizard" @@ -39,6 +36,8 @@ module Stylesheet when Stylesheet::Manager::COLOR_SCHEME_STYLESHEET file += importer.import_color_definitions file += importer.import_wcag_overrides + file += importer.category_backgrounds + file += importer.font end end diff --git a/lib/stylesheet/manager/builder.rb b/lib/stylesheet/manager/builder.rb index 1265c286a6c..dcd223dc8a6 100644 --- a/lib/stylesheet/manager/builder.rb +++ b/lib/stylesheet/manager/builder.rb @@ -137,6 +137,10 @@ class Stylesheet::Manager::Builder !!(@target.to_s =~ Stylesheet::Manager::THEME_REGEX) end + def is_color_scheme? + !!(@target.to_s == Stylesheet::Manager::COLOR_SCHEME_STYLESHEET) + end + def scheme_slug Slug.for(ActiveSupport::Inflector.transliterate(@color_scheme.name), 'scheme') end @@ -146,8 +150,10 @@ class Stylesheet::Manager::Builder @digest ||= begin if is_theme? theme_digest - else + elsif is_color_scheme? color_scheme_digest + else + default_digest end end end @@ -171,7 +177,7 @@ class Stylesheet::Manager::Builder end def theme_digest - Digest::SHA1.hexdigest(scss_digest.to_s + color_scheme_digest.to_s + settings_digest + plugins_digest + uploads_digest) + Digest::SHA1.hexdigest(scss_digest.to_s + color_scheme_digest.to_s + settings_digest + uploads_digest) end # this protects us from situations where new versions of a plugin removed a file @@ -218,6 +224,10 @@ class Stylesheet::Manager::Builder Digest::SHA1.hexdigest(sha1s.sort!.join("\n")) end + def default_digest + Digest::SHA1.hexdigest "default-#{Stylesheet::Manager.last_file_updated}-#{plugins_digest}" + end + def color_scheme_digest cs = @color_scheme || theme&.color_scheme diff --git a/spec/components/stylesheet/importer_spec.rb b/spec/components/stylesheet/importer_spec.rb index 4e9a0f8c117..605b9069f8a 100644 --- a/spec/components/stylesheet/importer_spec.rb +++ b/spec/components/stylesheet/importer_spec.rb @@ -11,19 +11,16 @@ describe Stylesheet::Importer do context "#category_backgrounds" do it "applies CDN to background category images" do - expect(compile_css("mobile")).to_not include("body.category-") - expect(compile_css("desktop")).to_not include("body.category-") + expect(compile_css("color_definitions")).to_not include("body.category-") background = Fabricate(:upload) parent_category = Fabricate(:category) category = Fabricate(:category, parent_category_id: parent_category.id, uploaded_background: background) - expect(compile_css("mobile")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}") - expect(compile_css("desktop")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}") + expect(compile_css("color_definitions")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}") GlobalSetting.stubs(:cdn_url).returns("//awesome.cdn") - expect(compile_css("mobile")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background.url})}") - expect(compile_css("desktop")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background.url})}") + expect(compile_css("color_definitions")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background.url})}") end it "applies S3 CDN to background category images" do @@ -36,8 +33,7 @@ describe Stylesheet::Importer do background = Fabricate(:upload_s3) category = Fabricate(:category, uploaded_background: background) - expect(compile_css("mobile")).to include("body.category-#{category.slug}{background-image:url(https://s3.cdn/original") - expect(compile_css("desktop")).to include("body.category-#{category.slug}{background-image:url(https://s3.cdn/original") + expect(compile_css("color_definitions")).to include("body.category-#{category.slug}{background-image:url(https://s3.cdn/original") end end @@ -45,8 +41,7 @@ describe Stylesheet::Importer do context "#font" do it "includes font variable" do default_font = ":root{--font-family: Arial, sans-serif}" - expect(compile_css("desktop")).to include(default_font) - expect(compile_css("mobile")).to include(default_font) + expect(compile_css("color_definitions")).to include(default_font) expect(compile_css("embed")).to include(default_font) expect(compile_css("publish")).to include(default_font) end @@ -58,14 +53,14 @@ describe Stylesheet::Importer do SiteSetting.base_font = base_font[:key] SiteSetting.heading_font = heading_font[:key] - expect(compile_css("desktop")) + expect(compile_css("color_definitions")) .to include(":root{--font-family: #{base_font[:stack]}}") .and include(":root{--heading-font-family: #{heading_font[:stack]}}") set_cdn_url("http://cdn.localhost") # uses CDN and includes cache-breaking param - expect(compile_css("mobile")) + expect(compile_css("color_definitions")) .to include("http://cdn.localhost/fonts/#{base_font[:variants][0][:filename]}?v=#{DiscourseFonts::VERSION}") .and include("http://cdn.localhost/fonts/#{heading_font[:variants][0][:filename]}?v=#{DiscourseFonts::VERSION}") end diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index da6768d07a8..1c7673ac933 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -188,22 +188,12 @@ describe Stylesheet::Manager do DiscoursePluginRegistry.reset! end - it 'can correctly account for plugins in digest' do - theme = Fabricate(:theme) - manager = manager(theme.id) - - builder = Stylesheet::Manager::Builder.new( - target: :desktop_theme, theme: theme, manager: manager - ) - + it 'can correctly account for plugins in default digest' do + builder = Stylesheet::Manager::Builder.new(target: :desktop, manager: manager) digest1 = builder.digest DiscoursePluginRegistry.stylesheets["fake"] = Set.new(["fake_file"]) - - builder = Stylesheet::Manager::Builder.new( - target: :desktop_theme, theme: theme, manager: manager - ) - + builder = Stylesheet::Manager::Builder.new(target: :desktop, manager: manager) digest2 = builder.digest expect(digest1).not_to eq(digest2) @@ -282,6 +272,21 @@ describe Stylesheet::Manager do expect(digest1).not_to eq(digest2) end + + it 'returns different digest based on target' do + theme = Fabricate(:theme) + builder = Stylesheet::Manager::Builder.new(target: :desktop_theme, theme: theme, manager: manager) + expect(builder.digest).to eq(builder.theme_digest) + + builder = Stylesheet::Manager::Builder.new(target: :color_definitions, manager: manager) + expect(builder.digest).to eq(builder.color_scheme_digest) + + builder = Stylesheet::Manager::Builder.new(target: :admin, manager: manager) + expect(builder.digest).to eq(builder.default_digest) + + builder = Stylesheet::Manager::Builder.new(target: :desktop, manager: manager) + expect(builder.digest).to eq(builder.default_digest) + end end describe 'color_scheme_digest' do