FEATURE: Allow themes to specify modifiers in their about.json file (#9097)

There are three modifiers:
- serialize_topic_excerpts (boolean)
- csp_extensions (array of strings)
- svg_icons (array of strings)

When multiple themes are active, the values will be combined. The combination method varies based on the setting. CSP/SVG arrays will be combined. serialize_topic_excerpts will use `Enumerable#any`.
This commit is contained in:
David Taylor 2020-03-11 13:30:45 +00:00 committed by GitHub
parent 0754c7c404
commit d1474e94a1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 205 additions and 6 deletions

View File

@ -145,6 +145,13 @@ class RemoteTheme < ActiveRecord::Base
raise ImportError, I18n.t("themes.import_error.about_json_values", errors: self.errors.full_messages.join(","))
end
ThemeModifierSet::MODIFIERS.keys.each do |modifier_name|
theme.theme_modifier_set.public_send(:"#{modifier_name}=", theme_info.dig("modifiers", modifier_name.to_s))
end
if !theme.theme_modifier_set.valid?
raise ImportError, I18n.t("themes.import_error.modifier_values", errors: theme.theme_modifier_set.errors.full_messages.join(","))
end
importer.all_files.each do |filename|
next unless opts = ThemeField.opts_from_file_path(filename)
value = importer[filename]

View File

@ -17,6 +17,7 @@ class Theme < ActiveRecord::Base
has_many :parent_themes, -> { order(:name) }, through: :parent_theme_relation, source: :parent_theme
has_many :color_schemes
belongs_to :remote_theme, dependent: :destroy
has_one :theme_modifier_set, dependent: :destroy
has_one :settings_field, -> { where(target_id: Theme.targets[:settings], name: "yaml") }, class_name: 'ThemeField'
has_one :javascript_cache, dependent: :destroy
@ -34,6 +35,10 @@ class Theme < ActiveRecord::Base
changed_schemes << scheme if scheme
end
def theme_modifier_set
super || build_theme_modifier_set
end
after_save do
changed_colors.each(&:save!)
changed_schemes.each(&:save!)
@ -44,6 +49,8 @@ class Theme < ActiveRecord::Base
changed_fields.each(&:save!)
changed_fields.clear
theme_modifier_set.save!
if saved_change_to_name?
theme_fields.select(&:basic_html_field?).each(&:invalidate_baked!)
end
@ -112,8 +119,8 @@ class Theme < ActiveRecord::Base
end
def self.get_set_cache(key, &blk)
if val = @cache[key]
return val
if @cache.hash.key? key.to_s
return @cache[key]
end
@cache[key] = blk.call
end
@ -247,6 +254,15 @@ class Theme < ActiveRecord::Base
(@cache[cache_key] = val || "").html_safe
end
def self.lookup_modifier(theme_ids, modifier_name)
theme_ids = [theme_ids] unless Array === theme_ids
theme_ids = transform_ids(theme_ids)
get_set_cache("#{theme_ids.join(",")}:modifier:#{modifier_name}:#{ThemeField::COMPILER_VERSION}") do
ThemeModifierSet.resolve_modifier_for_themes(theme_ids, modifier_name)
end
end
def self.remove_from_cache!
clear_cache!
end
@ -521,6 +537,13 @@ class Theme < ActiveRecord::Base
end
end
meta[:modifiers] = {}.tap do |hash|
ThemeModifierSet::MODIFIERS.keys.each do |modifier|
value = self.theme_modifier_set.public_send(modifier)
hash[modifier] = value if !value.nil?
end
end
meta[:learn_more] = "https://meta.discourse.org/t/beginners-guide-to-using-discourse-themes/91966"
end

View File

@ -0,0 +1,69 @@
# frozen_string_literal: true
class ThemeModifierSet < ActiveRecord::Base
class ThemeModifierSetError < StandardError; end
belongs_to :theme
MODIFIERS ||= {
serialize_topic_excerpts: { combine_mode: :any, type: :boolean },
csp_extensions: { combine_mode: :flatten, type: :string_array },
svg_icons: { combine_mode: :flatten, type: :string_array },
}
validate :type_validator
def type_validator
MODIFIERS.each do |k, config|
value = public_send(k)
next if value.nil?
case config[:type]
when :boolean
next if [true, false].include?(value)
when :string_array
next if value.is_a?(Array) && value.all? { |v| v.is_a?(String) }
end
errors.add(k, :invalid)
end
end
after_save do
if saved_change_to_svg_icons?
SvgSprite.expire_cache
end
if saved_change_to_csp_extensions?
CSP::Extension.clear_theme_extensions_cache!
end
end
# Given the ids of multiple active themes / theme components, this function
# will combine them into a 'resolved' behavior
def self.resolve_modifier_for_themes(theme_ids, modifier_name)
return nil if !(config = MODIFIERS[modifier_name])
all_values = self.where(theme_id: theme_ids).where.not(modifier_name => nil).pluck(modifier_name)
case config[:combine_mode]
when :any
all_values.any?
when :flatten
all_values.flatten(1)
else
raise ThemeModifierSetError "Invalid theme modifier combine_mode"
end
end
end
# == Schema Information
#
# Table name: theme_modifier_sets
#
# id :bigint not null, primary key
# theme_id :bigint not null
# serialize_topic_excerpts :boolean
# csp_extensions :string is an Array
# svg_icons :string is an Array
#
# Indexes
#
# index_theme_modifier_sets_on_theme_id (theme_id) UNIQUE
#

View File

@ -111,7 +111,7 @@ class ListableTopicSerializer < BasicTopicSerializer
alias :include_new_posts? :has_user_data
def include_excerpt?
pinned || SiteSetting.always_include_topic_excerpts
pinned || SiteSetting.always_include_topic_excerpts || ThemeModifierHelper.new(request: scope.request).serialize_topic_excerpts
end
def pinned

View File

@ -83,6 +83,7 @@ en:
generic: An error occured while importing that theme
about_json: "Import Error: about.json does not exist, or is invalid"
about_json_values: "about.json contains invalid values: %{errors}"
flag_values: "about.json flags contain invalid values: %{errors}"
git: "Error cloning git repository, access is denied or repository is not found"
unpack_failed: "Failed to unpack file"
file_too_big: "The uncompressed file is too big."

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class AddThemeModifierSet < ActiveRecord::Migration[6.0]
def change
create_table(:theme_modifier_sets) do |t|
t.references :theme, index: { unique: true }, null: false
t.column :serialize_topic_excerpts, :boolean
t.column :csp_extensions, :string, array: true
t.column :svg_icons, :string, array: true
end
end
end

View File

@ -44,16 +44,18 @@ class ContentSecurityPolicy
Theme.where(id: Theme.transform_ids(theme_ids)).find_each do |theme|
theme.cached_settings.each do |setting, value|
extensions << build_theme_extension(value) if setting.to_s == THEME_SETTING
extensions << build_theme_extension(value.split("|")) if setting.to_s == THEME_SETTING
end
end
extensions << build_theme_extension(ThemeModifierHelper.new(theme_ids: theme_ids).csp_extensions)
extensions
end
def build_theme_extension(raw)
def build_theme_extension(entries)
{}.tap do |extension|
raw.split('|').each do |entry|
entries.each do |entry|
directive, source = entry.split(':', 2).map(&:strip)
extension[directive] ||= []

View File

@ -399,6 +399,8 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
end
end
theme_icon_settings |= ThemeModifierHelper.new(theme_ids: theme_ids).svg_icons
theme_icon_settings
end

View File

@ -0,0 +1,12 @@
# frozen_string_literal: true
class ThemeModifierHelper
def initialize(request: nil, theme_ids: nil)
@theme_ids = theme_ids || request&.env&.[](:resolved_theme_ids)
end
ThemeModifierSet::MODIFIERS.keys.each do |modifier|
define_method(modifier) do
Theme.lookup_modifier(@theme_ids, modifier)
end
end
end

View File

@ -142,6 +142,16 @@ describe SvgSprite do
expect(SvgSprite.all_icons([parent_theme.id])).to include("dragon")
end
it 'includes icons defined in theme flags' do
theme = Fabricate(: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")
end
it 'includes custom icons from a sprite in a theme' do
theme = Fabricate(:theme)
fname = "custom-theme-icon-sprite.svg"

View File

@ -100,6 +100,7 @@ describe ThemeStore::ZipExporter do
"tertiary": "858585"
}
},
"modifiers": {},
"learn_more": "https://meta.discourse.org/t/beginners-guide-to-using-discourse-themes/91966"
)

View File

@ -165,6 +165,21 @@ describe ContentSecurityPolicy do
expect(parse(theme_policy)['script-src']).to_not include('https://from-theme.net')
expect(parse(theme_policy)['worker-src']).to_not include('from-theme.com')
end
it 'can be extended by theme flags' do
policy # call this first to make sure further actions clear the cache
theme.theme_modifier_set.csp_extensions = ["script-src: https://from-theme-flag.script", "worker-src: from-theme-flag.worker"]
theme.save!
expect(parse(theme_policy)['script-src']).to include('https://from-theme-flag.script')
expect(parse(theme_policy)['worker-src']).to include('from-theme-flag.worker')
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')
end
end
it 'can be extended by site setting' do

View File

@ -0,0 +1,17 @@
# frozen_string_literal: true
require 'rails_helper'
describe ThemeModifierHelper do
fab!(:theme) { Fabricate(:theme).tap { |t| t.theme_modifier_set.update!(serialize_topic_excerpts: true) } }
it "defines a getter for modifiers" do
tmh = ThemeModifierHelper.new(theme_ids: [theme.id])
expect(tmh.serialize_topic_excerpts).to eq(true)
end
it "can extract theme ids from a request object" do
request = Rack::Request.new({ resolved_theme_ids: [theme.id] })
tmh = ThemeModifierHelper.new(request: request)
expect(tmh.serialize_topic_excerpts).to eq(true)
end
end

View File

@ -37,6 +37,9 @@ describe RemoteTheme do
"love": "#{love_color}",
"tertiary-low": "#{tertiary_low_color}"
}
},
"modifiers": {
"serialize_topic_excerpts": true
}
}
JSON
@ -85,6 +88,8 @@ describe RemoteTheme do
expect(remote.theme_version).to eq("1.0")
expect(remote.minimum_discourse_version).to eq("1.0.0")
expect(@theme.theme_modifier_set.serialize_topic_excerpts).to eq(true)
expect(@theme.theme_fields.length).to eq(9)
mapped = Hash[*@theme.theme_fields.map { |f| ["#{f.target_id}-#{f.name}", f.value] }.flatten]

View File

@ -0,0 +1,24 @@
# frozen_string_literal: true
require 'rails_helper'
describe ThemeModifierSet do
describe "#resolve_modifiers_for_themes" do
it "returns nil for unknown modifier" do
expect(ThemeModifierSet.resolve_modifier_for_themes([1, 2], :unknown_modifier)).to eq(nil)
end
it "resolves serialize_topic_excerpts correctly" do
t1 = Fabricate(:theme)
t1.theme_modifier_set.update!(serialize_topic_excerpts: true)
t2 = Fabricate(:theme)
t2.theme_modifier_set.update!(serialize_topic_excerpts: false)
expect(ThemeModifierSet.resolve_modifier_for_themes([t1.id, t2.id], :serialize_topic_excerpts)).to eq(true)
t1 = Fabricate(:theme)
t1.theme_modifier_set.update!(serialize_topic_excerpts: nil)
expect(ThemeModifierSet.resolve_modifier_for_themes([t1.id, t2.id], :serialize_topic_excerpts)).to eq(false)
end
end
end