diff --git a/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 b/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 index 20a16ab626..955d5a6710 100644 --- a/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 @@ -1,4 +1,7 @@ -import { default as computed } from "ember-addons/ember-computed-decorators"; +import { + default as computed, + observes +} from "ember-addons/ember-computed-decorators"; import { url } from "discourse/lib/computed"; import { popupAjaxError } from "discourse/lib/ajax-error"; import showModal from "discourse/lib/show-modal"; @@ -9,6 +12,18 @@ const THEME_UPLOAD_VAR = 2; export default Ember.Controller.extend({ editRouteName: "adminCustomizeThemes.edit", + @observes("allowChildThemes") + setSelectedThemeId() { + const available = this.get("selectableChildThemes"); + if ( + !this.get("selectedChildThemeId") && + available && + available.length > 0 + ) { + this.set("selectedChildThemeId", available[0].get("id")); + } + }, + @computed("model", "allThemes") parentThemes(model, allThemes) { let parents = allThemes.filter(theme => @@ -64,16 +79,21 @@ export default Ember.Controller.extend({ let themes = []; available.forEach(t => { - if (!childThemes || childThemes.indexOf(t) === -1) { + if ( + (!childThemes || childThemes.indexOf(t) === -1) && + Em.isEmpty(t.get("childThemes")) && + !t.get("user_selectable") && + !t.get("default") + ) { themes.push(t); } }); return themes.length === 0 ? null : themes; }, - @computed("allThemes", "allThemes.length", "model") + @computed("allThemes", "allThemes.length", "model", "parentThemes") availableChildThemes(allThemes, count) { - if (count === 1) { + if (count === 1 || this.get("parentThemes")) { return null; } diff --git a/app/assets/javascripts/admin/models/theme.js.es6 b/app/assets/javascripts/admin/models/theme.js.es6 index ae4bc73eec..c3d7fd800d 100644 --- a/app/assets/javascripts/admin/models/theme.js.es6 +++ b/app/assets/javascripts/admin/models/theme.js.es6 @@ -1,5 +1,6 @@ import RestModel from "discourse/models/rest"; import { default as computed } from "ember-addons/ember-computed-decorators"; +import { popupAjaxError } from "discourse/lib/ajax-error"; const THEME_UPLOAD_VAR = 2; @@ -150,7 +151,9 @@ const Theme = RestModel.extend({ saveChanges() { const hash = this.getProperties.apply(this, arguments); - return this.save(hash).then(() => this.set("changed", false)); + return this.save(hash) + .finally(() => this.set("changed", false)) + .catch(popupAjaxError); }, saveSettings(name, value) { diff --git a/app/assets/javascripts/admin/templates/customize-themes-show.hbs b/app/assets/javascripts/admin/templates/customize-themes-show.hbs index c83b93a176..375a7e95b5 100644 --- a/app/assets/javascripts/admin/templates/customize-themes-show.hbs +++ b/app/assets/javascripts/admin/templates/customize-themes-show.hbs @@ -137,7 +137,7 @@ {{/unless}} {{#if selectableChildThemes}}

- {{combo-box content=selectableChildThemes value=selectedChildThemeId}} + {{combo-box filterable=true content=selectableChildThemes value=selectedChildThemeId}} {{#d-button action="addChildTheme" icon="plus"}}{{i18n "admin.customize.theme.add"}}{{/d-button}}

{{/if}} diff --git a/app/assets/javascripts/discourse/controllers/preferences/interface.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/interface.js.es6 index 29ee8aa194..8cf32d9cc6 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/interface.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/interface.js.es6 @@ -66,7 +66,7 @@ export default Ember.Controller.extend(PreferencesTabController, { @observes("themeId") themeIdChanged() { const id = this.get("themeId"); - previewTheme(id); + previewTheme([id]); }, homeChanged() { diff --git a/app/assets/javascripts/discourse/initializers/live-development.js.es6 b/app/assets/javascripts/discourse/initializers/live-development.js.es6 index 994e4e36e6..fd1f6642cb 100644 --- a/app/assets/javascripts/discourse/initializers/live-development.js.es6 +++ b/app/assets/javascripts/discourse/initializers/live-development.js.es6 @@ -1,5 +1,5 @@ import DiscourseURL from "discourse/lib/url"; -import { currentThemeId, refreshCSS } from "discourse/lib/theme-selector"; +import { currentThemeIds, refreshCSS } from "discourse/lib/theme-selector"; // Use the message bus for live reloading of components for faster development. export default { @@ -58,12 +58,16 @@ export default { // Refresh if necessary document.location.reload(true); } else { - let themeId = currentThemeId(); - + const themeIds = currentThemeIds(); $("link").each(function() { if (me.hasOwnProperty("theme_id") && me.new_href) { - let target = $(this).data("target"); - if (me.theme_id === themeId && target === me.target) { + const target = $(this).data("target"); + const themeId = $(this).data("theme-id"); + if ( + themeIds.indexOf(me.theme_id) !== -1 && + target === me.target && + (!themeId || themeId === me.theme_id) + ) { refreshCSS(this, null, me.new_href); } } else if (this.href.match(me.name) && (me.hash || me.new_href)) { diff --git a/app/assets/javascripts/discourse/lib/theme-selector.js.es6 b/app/assets/javascripts/discourse/lib/theme-selector.js.es6 index 1c5d9c4f39..ce806b4496 100644 --- a/app/assets/javascripts/discourse/lib/theme-selector.js.es6 +++ b/app/assets/javascripts/discourse/lib/theme-selector.js.es6 @@ -1,7 +1,7 @@ import { ajax } from "discourse/lib/ajax"; import deprecated from "discourse-common/lib/deprecated"; -const keySelector = "meta[name=discourse_theme_id]"; +const keySelector = "meta[name=discourse_theme_ids]"; export function currentThemeKey() { if (console && console.warn && console.trace) { @@ -12,21 +12,26 @@ export function currentThemeKey() { } } -export function currentThemeId() { - let themeId = null; - let elem = _.first($(keySelector)); +export function currentThemeIds() { + const themeIds = []; + const elem = _.first($(keySelector)); if (elem) { - themeId = elem.content; - if (_.isEmpty(themeId)) { - themeId = null; - } else { - themeId = parseInt(themeId); - } + elem.content.split(",").forEach(num => { + num = parseInt(num, 10); + if (!isNaN(num)) { + themeIds.push(num); + } + }); } - return themeId; + return themeIds; +} + +export function currentThemeId() { + return currentThemeIds()[0]; } export function setLocalTheme(ids, themeSeq) { + ids = ids.reject(id => !id); if (ids && ids.length > 0) { $.cookie("theme_ids", `${ids.join(",")}|${themeSeq}`, { path: "/", @@ -76,23 +81,28 @@ export function refreshCSS(node, hash, newHref, options) { $orig.data("copy", reloaded); } -export function previewTheme(id) { - if (currentThemeId() !== id) { +export function previewTheme(ids = []) { + ids = ids.reject(id => !id); + if (!ids.includes(currentThemeId())) { Discourse.set("assetVersion", "forceRefresh"); - ajax(`/themes/assets/${id ? id : "default"}`).then(results => { - let elem = _.first($(keySelector)); - if (elem) { - elem.content = id; - } - - results.themes.forEach(theme => { - let node = $(`link[rel=stylesheet][data-target=${theme.target}]`)[0]; - if (node) { - refreshCSS(node, null, theme.url, { force: true }); + ajax(`/themes/assets/${ids.length > 0 ? ids.join("-") : "default"}`).then( + results => { + const elem = _.first($(keySelector)); + if (elem) { + elem.content = ids.join(","); } - }); - }); + + results.themes.forEach(theme => { + const node = $( + `link[rel=stylesheet][data-target=${theme.target}]` + )[0]; + if (node) { + refreshCSS(node, null, theme.new_href, { force: true }); + } + }); + } + ); } } diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index 5630724b77..08a4d10b76 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -182,11 +182,13 @@ class Admin::ThemesController < Admin::AdminController log_theme_change(original_json, @theme) format.json { render json: @theme, status: :ok } else - format.json { + format.json do + error = @theme.errors.full_messages.join(", ").presence + error = I18n.t("themes.bad_color_scheme") if @theme.errors[:color_scheme].present? + error ||= I18n.t("themes.other_error") - error = @theme.errors[:color_scheme] ? I18n.t("themes.bad_color_scheme") : I18n.t("themes.other_error") render json: { errors: [ error ] }, status: :unprocessable_entity - } + end end end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3972fa18c0..87d372f629 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -22,7 +22,7 @@ class ApplicationController < ActionController::Base include GlobalPath include Hijack - attr_reader :theme_id + attr_reader :theme_ids serialization_scope :guardian @@ -62,8 +62,8 @@ class ApplicationController < ActionController::Base after_action :remember_theme_id def remember_theme_id - if @theme_id - Stylesheet::Watcher.theme_id = @theme_id if defined? Stylesheet::Watcher + if @theme_ids.present? + Stylesheet::Watcher.theme_id = @theme_ids.first if defined? Stylesheet::Watcher end end end @@ -331,28 +331,33 @@ class ApplicationController < ActionController::Base resolve_safe_mode return if request.env[NO_CUSTOM] - theme_id = request[:preview_theme_id]&.to_i + theme_ids = [] + + if preview_theme_id = request[:preview_theme_id]&.to_i + theme_ids << preview_theme_id + end user_option = current_user&.user_option - unless theme_id + if theme_ids.blank? ids, seq = cookies[:theme_ids]&.split("|") ids = ids&.split(",")&.map(&:to_i) - if ids && ids.size > 0 && seq && seq.to_i == user_option&.theme_key_seq.to_i - theme_id = ids.first + if ids.present? && seq && seq.to_i == user_option&.theme_key_seq.to_i + theme_ids = ids if guardian.allow_themes?(ids) end end - theme_id ||= user_option&.theme_ids&.first + theme_ids = user_option&.theme_ids || [] if theme_ids.blank? - if theme_id && !guardian.allow_themes?(theme_id) - theme_id = nil + unless guardian.allow_themes?(theme_ids) + theme_ids = [] end - theme_id ||= SiteSetting.default_theme_id - theme_id = nil if theme_id.blank? || theme_id == -1 + if theme_ids.blank? && SiteSetting.default_theme_id != -1 + theme_ids << SiteSetting.default_theme_id + end - @theme_id = request.env[:resolved_theme_id] = theme_id + @theme_ids = request.env[:resolved_theme_ids] = theme_ids end def guardian @@ -502,10 +507,10 @@ class ApplicationController < ActionController::Base target = view_context.mobile_view? ? :mobile : :desktop data = - if @theme_id + if @theme_ids.present? { - top: Theme.lookup_field(@theme_id, target, "after_header"), - footer: Theme.lookup_field(@theme_id, target, "footer") + top: Theme.lookup_field(@theme_ids, target, "after_header"), + footer: Theme.lookup_field(@theme_ids, target, "footer") } else {} diff --git a/app/controllers/stylesheets_controller.rb b/app/controllers/stylesheets_controller.rb index 6b0fe317cf..364c3e58dc 100644 --- a/app/controllers/stylesheets_controller.rb +++ b/app/controllers/stylesheets_controller.rb @@ -29,7 +29,7 @@ class StylesheetsController < ApplicationController # we hold of re-compilation till someone asks for asset if target.include?("theme") split_target, theme_id = target.split(/_(-?[0-9]+)/) - theme = Theme.find(theme_id) if theme_id + theme = Theme.find_by(id: theme_id) if theme_id.present? else split_target, color_scheme_id = target.split(/_(-?[0-9]+)/) theme = Theme.find_by(color_scheme_id: color_scheme_id) diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index 22c2925107..2cc8e284a1 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -1,27 +1,26 @@ class ThemesController < ::ApplicationController def assets - theme_id = params[:id].to_i + theme_ids = params[:ids].to_s.split("-").map(&:to_i) - if params[:id] == "default" - theme_id = nil + if params[:ids] == "default" + theme_ids = nil else - raise Discourse::NotFound unless Theme.where(id: theme_id).exists? + raise Discourse::NotFound unless guardian.allow_themes?(theme_ids) end - object = [:mobile, :desktop, :desktop_theme, :mobile_theme].map do |target| - link = Stylesheet::Manager.stylesheet_link_tag(target, 'all', params[:id]) - if link - href = link.split(/["']/)[1] - if Rails.env.development? - href << (href.include?("?") ? "&" : "?") - href << SecureRandom.hex - end - { - target: target, - url: href - } + targets = view_context.mobile_view? ? [:mobile, :mobile_theme] : [:desktop, :desktop_theme] + targets << :admin if guardian.is_staff? + + object = targets.map do |target| + Stylesheet::Manager.stylesheet_data(target, theme_ids).map do |hash| + return hash unless Rails.env.development? + + dup_hash = hash.dup + dup_hash[:new_href] << (dup_hash[:new_href].include?("?") ? "&" : "?") + dup_hash[:new_href] << SecureRandom.hex + dup_hash end - end.compact + end.flatten render json: object.as_json end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 90d148be7f..046ab45f01 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -350,11 +350,11 @@ module ApplicationHelper end end - def theme_id + def theme_ids if customization_disabled? nil else - request.env[:resolved_theme_id] + request.env[:resolved_theme_ids] end end @@ -378,17 +378,17 @@ module ApplicationHelper end def theme_lookup(name) - lookup = Theme.lookup_field(theme_id, mobile_view? ? :mobile : :desktop, name) + lookup = Theme.lookup_field(theme_ids, mobile_view? ? :mobile : :desktop, name) lookup.html_safe if lookup end def discourse_stylesheet_link_tag(name, opts = {}) - if opts.key?(:theme_id) - id = opts[:theme_id] unless customization_disabled? + if opts.key?(:theme_ids) + ids = opts[:theme_ids] unless customization_disabled? else - id = theme_id + ids = theme_ids end - Stylesheet::Manager.stylesheet_link_tag(name, 'all', id) + Stylesheet::Manager.stylesheet_link_tag(name, 'all', ids) end end diff --git a/app/models/child_theme.rb b/app/models/child_theme.rb index e4eb2d0ef7..c37e417dbe 100644 --- a/app/models/child_theme.rb +++ b/app/models/child_theme.rb @@ -1,6 +1,24 @@ class ChildTheme < ActiveRecord::Base belongs_to :parent_theme, class_name: 'Theme' belongs_to :child_theme, class_name: 'Theme' + + validate :child_validations + + private + + def child_validations + if ChildTheme.exists?(["parent_theme_id = ? OR child_theme_id = ?", child_theme_id, parent_theme_id]) + errors.add(:base, I18n.t("themes.errors.no_multilevels_components")) + end + + if Theme.exists?(id: child_theme_id, user_selectable: true) + errors.add(:base, I18n.t("themes.errors.component_no_user_selectable")) + end + + if child_theme_id == SiteSetting.default_theme_id + errors.add(:base, I18n.t("themes.errors.component_no_default")) + end + end end # == Schema Information diff --git a/app/models/color_scheme.rb b/app/models/color_scheme.rb index a80b1e40b0..bd3efd15d8 100644 --- a/app/models/color_scheme.rb +++ b/app/models/color_scheme.rb @@ -241,12 +241,15 @@ class ColorScheme < ActiveRecord::Base def publish_discourse_stylesheet if self.id - themes = Theme.where(color_scheme_id: self.id).to_a - if themes.present? + theme_ids = Theme.where(color_scheme_id: self.id).pluck(:id) + if theme_ids.present? Stylesheet::Manager.cache.clear - themes.each do |theme| - theme.notify_scheme_change(_clear_manager_cache = false) - end + Theme.notify_theme_change( + theme_ids, + with_scheme: true, + clear_manager_cache: false, + all_themes: true + ) end end end diff --git a/app/models/theme.rb b/app/models/theme.rb index dcc99f3ce3..039d18953b 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -20,6 +20,12 @@ class Theme < ActiveRecord::Base has_many :color_schemes belongs_to :remote_theme + validate :user_selectable_validation + + scope :user_selectable, ->() { + where('user_selectable OR id = ?', SiteSetting.default_theme_id) + } + def notify_color_change(color) changed_colors << color end @@ -45,7 +51,6 @@ class Theme < ActiveRecord::Base remove_from_cache! clear_cached_settings! - notify_scheme_change if saved_change_to_color_scheme_id? end after_destroy do @@ -70,29 +75,37 @@ class Theme < ActiveRecord::Base end after_commit ->(theme) do - theme.notify_theme_change - end, on: :update + theme.notify_theme_change(with_scheme: theme.saved_change_to_color_scheme_id?) + end, on: [:create, :update] + + def self.get_set_cache(key, &blk) + if val = @cache[key] + return val + end + @cache[key] = blk.call + end def self.theme_ids - if ids = @cache["theme_ids"] - return ids + get_set_cache "theme_ids" do + Theme.pluck(:id) end - @cache["theme_ids"] = Set.new(Theme.pluck(:id)) end def self.user_theme_ids - if ids = @cache["user_theme_ids"] - return ids + get_set_cache "user_theme_ids" do + Theme.user_selectable.pluck(:id) + end + end + + def self.components_for(theme_id) + get_set_cache "theme_components_for_#{theme_id}" do + ChildTheme.where(parent_theme_id: theme_id).distinct.pluck(:child_theme_id) end - @cache["user_theme_ids"] = Set.new( - Theme - .where('user_selectable OR id = ?', SiteSetting.default_theme_id) - .pluck(:id) - ) end def self.expire_site_cache! Site.clear_anon_cache! + clear_cache! ApplicationSerializer.expire_cache_fragment!("user_themes") end @@ -101,7 +114,25 @@ class Theme < ActiveRecord::Base expire_site_cache! end + def self.transform_ids(ids, extend: true) + return [] if ids.blank? + + ids.uniq! + parent = ids.first + + components = ids[1..-1] + components.push(*components_for(parent)) if extend + components.sort!.uniq! + + [parent, *components] + end + def set_default! + if component? + raise Discourse::InvalidParameters.new( + I18n.t("themes.errors.component_no_default") + ) + end SiteSetting.default_theme_id = id Theme.expire_site_cache! end @@ -110,22 +141,32 @@ class Theme < ActiveRecord::Base SiteSetting.default_theme_id == id end - def self.lookup_field(theme_id, target, field) - return if theme_id.blank? + def component? + ChildTheme.exists?(child_theme_id: id) + end - cache_key = "#{theme_id}:#{target}:#{field}:#{ThemeField::COMPILER_VERSION}" + def user_selectable_validation + if component? && user_selectable + errors.add(:base, I18n.t("themes.errors.component_no_user_selectable")) + end + end + + def self.lookup_field(theme_ids, target, field) + return if theme_ids.blank? + theme_ids = [theme_ids] unless Array === theme_ids + + theme_ids = transform_ids(theme_ids) + cache_key = "#{theme_ids.join(",")}:#{target}:#{field}:#{ThemeField::COMPILER_VERSION}" lookup = @cache[cache_key] return lookup.html_safe if lookup target = target.to_sym - theme = find_by(id: theme_id) - - val = theme.resolve_baked_field(target, field) if theme + val = resolve_baked_field(theme_ids, target, field) (@cache[cache_key] = val || "").html_safe end - def self.remove_from_cache!(themes = nil) + def self.remove_from_cache! clear_cache! end @@ -141,33 +182,32 @@ class Theme < ActiveRecord::Base self.targets.invert[target_id] end - def notify_scheme_change(clear_manager_cache = true) - Stylesheet::Manager.cache.clear if clear_manager_cache - message = refresh_message_for_targets(["desktop", "mobile", "admin"], self) - MessageBus.publish('/file-change', message) - end - - def notify_theme_change + def self.notify_theme_change(theme_ids, with_scheme: false, clear_manager_cache: true, all_themes: false) Stylesheet::Manager.clear_theme_cache! + targets = [:mobile_theme, :desktop_theme] - themes = [self] + dependant_themes + if with_scheme + targets.prepend(:desktop, :mobile, :admin) + Stylesheet::Manager.cache.clear if clear_manager_cache + end + + if all_themes + message = theme_ids.map { |id| refresh_message_for_targets(targets, id) }.flatten + else + message = refresh_message_for_targets(targets, theme_ids).flatten + end - message = themes.map do |theme| - refresh_message_for_targets([:mobile_theme, :desktop_theme], theme) - end.compact.flatten MessageBus.publish('/file-change', message) end - def refresh_message_for_targets(targets, theme) + def notify_theme_change(with_scheme: false) + theme_ids = (dependant_themes&.pluck(:id) || []).unshift(self.id) + self.class.notify_theme_change(theme_ids, with_scheme: with_scheme) + end + + def self.refresh_message_for_targets(targets, theme_ids) targets.map do |target| - href = Stylesheet::Manager.stylesheet_href(target.to_sym, theme.id) - if href - { - target: target, - new_href: href, - theme_id: theme.id - } - end + Stylesheet::Manager.stylesheet_data(target.to_sym, theme_ids) end end @@ -180,48 +220,34 @@ class Theme < ActiveRecord::Base end def resolve_dependant_themes(direction) - - select_field, where_field = nil - if direction == :up - select_field = "parent_theme_id" + join_field = "parent_theme_id" where_field = "child_theme_id" elsif direction == :down - select_field = "child_theme_id" + join_field = "child_theme_id" where_field = "parent_theme_id" else raise "Unknown direction" end - themes = [] return [] unless id - uniq = Set.new - uniq << id + Theme.joins("JOIN child_themes ON themes.id = child_themes.#{join_field}").where("#{where_field} = ?", id) + end - iterations = 0 - added = [id] + def self.resolve_baked_field(theme_ids, target, name) + list_baked_fields(theme_ids, target, name).map { |f| f.value_baked || f.value }.join("\n") + end - while added.length > 0 && iterations < 5 + def self.list_baked_fields(theme_ids, target, name) + target = target.to_sym - iterations += 1 + fields = ThemeField.find_by_theme_ids(theme_ids) + .where(target_id: [Theme.targets[target], Theme.targets[:common]]) + .where(name: name.to_s) - new_themes = Theme.where("id in (SELECT #{select_field} - FROM child_themes - WHERE #{where_field} in (?))", added).to_a - - added = [] - new_themes.each do |theme| - unless uniq.include?(theme.id) - added << theme.id - uniq << theme.id - themes << theme - end - end - - end - - themes + fields.each(&:ensure_baked!) + fields end def resolve_baked_field(target, name) @@ -229,22 +255,8 @@ class Theme < ActiveRecord::Base end def list_baked_fields(target, name) - - target = target.to_sym - - theme_ids = [self.id] + (included_themes.map(&:id) || []) - fields = ThemeField.where(target_id: [Theme.targets[target], Theme.targets[:common]]) - .where(name: name.to_s) - .includes(:theme) - .joins(" - JOIN ( - SELECT #{theme_ids.map.with_index { |id, idx| "#{id} AS theme_id, #{idx} AS sort_column" }.join(" UNION ALL SELECT ")} - ) as X ON X.theme_id = theme_fields.theme_id" - ) - .order('sort_column, target_id') - - fields.each(&:ensure_baked!) - fields + theme_ids = (included_themes&.pluck(:id) || []).unshift(self.id) + self.class.list_baked_fields(theme_ids, target, name) end def remove_from_cache! @@ -288,21 +300,23 @@ class Theme < ActiveRecord::Base def all_theme_variables fields = {} - ([self] + (included_themes || [])).each do |theme| - theme&.theme_fields.each do |field| - next unless ThemeField.theme_var_type_ids.include?(field.type_id) - next if fields.key?(field.name) - fields[field.name] = field - end + ids = (included_themes&.pluck(:id) || []).unshift(self.id) + ThemeField.find_by_theme_ids(ids).where(type_id: ThemeField.theme_var_type_ids).each do |field| + next if fields.key?(field.name) + fields[field.name] = field end fields.values end def add_child_theme!(theme) - child_theme_relation.create!(child_theme_id: theme.id) - @included_themes = nil - child_themes.reload - save! + new_relation = child_theme_relation.new(child_theme_id: theme.id) + if new_relation.save + @included_themes = nil + child_themes.reload + save! + else + raise Discourse::InvalidParameters.new(new_relation.errors.full_messages.join(", ")) + end end def settings diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index 92d7fd2d17..c4d6ab4e94 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -4,6 +4,17 @@ class ThemeField < ActiveRecord::Base belongs_to :upload + scope :find_by_theme_ids, ->(theme_ids) { + return none unless theme_ids.present? + + where(theme_id: theme_ids) + .joins( + "JOIN ( + SELECT #{theme_ids.map.with_index { |id, idx| "#{id.to_i} AS theme_id, #{idx} AS sort_column" }.join(" UNION ALL SELECT ")} + ) as X ON X.theme_id = theme_fields.theme_id") + .order("sort_column") + } + def self.types @types ||= Enum.new(html: 0, scss: 1, diff --git a/app/views/common/_discourse_stylesheet.html.erb b/app/views/common/_discourse_stylesheet.html.erb index 5e66ec2ab2..45f6476e90 100644 --- a/app/views/common/_discourse_stylesheet.html.erb +++ b/app/views/common/_discourse_stylesheet.html.erb @@ -8,6 +8,6 @@ <%= discourse_stylesheet_link_tag(:admin) %> <%- end %> -<%- if theme_id %> +<%- if theme_ids.present? %> <%= discourse_stylesheet_link_tag(mobile_view? ? :mobile_theme : :desktop_theme) %> <%- end %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 934695067b..94bf307037 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -4,7 +4,7 @@ <%= content_for?(:title) ? yield(:title) : SiteSetting.title %> - + "> <%= render partial: "layouts/head" %> <%= discourse_csrf_tags %> diff --git a/app/views/layouts/crawler.html.erb b/app/views/layouts/crawler.html.erb index 9e2c662f80..98f15b0e46 100644 --- a/app/views/layouts/crawler.html.erb +++ b/app/views/layouts/crawler.html.erb @@ -10,7 +10,7 @@ <%- else %> <%= discourse_stylesheet_link_tag(mobile_view? ? :mobile : :desktop) %> <%- end %> - <%- if theme_id %> + <%- if theme_ids.present? %> <%= discourse_stylesheet_link_tag(mobile_view? ? :mobile_theme : :desktop_theme) %> <%- end %> <%= theme_lookup("head_tag") %> diff --git a/app/views/layouts/embed.html.erb b/app/views/layouts/embed.html.erb index b1faf885ed..8f91159c53 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_id: nil %> + <%= discourse_stylesheet_link_tag 'embed', theme_ids: 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 f573616b19..8fc25c08eb 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_id: nil %> + <%= discourse_stylesheet_link_tag 'wizard', theme_ids: nil %> <%= render partial: "common/special_font_face" %> <%= preload_script 'ember_jquery' %> <%= preload_script 'wizard-vendor' %> diff --git a/app/views/wizard/index.html.erb b/app/views/wizard/index.html.erb index fb4f420bd1..cecca0e834 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_id: nil %> + <%= discourse_stylesheet_link_tag :wizard, theme_ids: nil %> <%= preload_script 'ember_jquery' %> <%= preload_script 'wizard-vendor' %> <%= preload_script 'wizard-application' %> diff --git a/app/views/wizard/qunit.html.erb b/app/views/wizard/qunit.html.erb index 561a2ade04..31755ff4d8 100644 --- a/app/views/wizard/qunit.html.erb +++ b/app/views/wizard/qunit.html.erb @@ -4,7 +4,7 @@ QUnit Test Runner <%= stylesheet_link_tag "qunit" %> <%= stylesheet_link_tag "test_helper" %> - <%= discourse_stylesheet_link_tag :wizard %> + <%= discourse_stylesheet_link_tag :wizard, theme_ids: nil %> <%= javascript_include_tag "qunit" %> <%= javascript_include_tag "wizard/test/test_helper" %> <%= csrf_meta_tags %> diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index da0aa91dbb..84d70adf62 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -60,6 +60,10 @@ en: bad_color_scheme: "Can not update theme, invalid color scheme" other_error: "Something went wrong updating theme" error_importing: "Error cloning git repository, access is denied or repository is not found" + errors: + component_no_user_selectable: "Theme components can't be user-selectable" + component_no_default: "Theme components can't be default theme" + no_multilevels_components: "Themes with child themes can't be child themes themselves" settings_errors: invalid_yaml: "Provided YAML is invalid." data_type_not_a_number: "Setting `%{name}` type is unsupported. Supported types are `integer`, `bool`, `list` and `enum`" diff --git a/config/routes.rb b/config/routes.rb index 0ef2a56020..441f448240 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -814,7 +814,7 @@ Discourse::Application.routes.draw do get "/safe-mode" => "safe_mode#index" post "/safe-mode" => "safe_mode#enter", as: "safe_mode_enter" - get "/themes/assets/:id" => "themes#assets" + get "/themes/assets/:ids" => "themes#assets" if Rails.env == "test" || Rails.env == "development" get "/qunit" => "qunit#index" diff --git a/db/migrate/20180710172959_disallow_multi_levels_theme_components.rb b/db/migrate/20180710172959_disallow_multi_levels_theme_components.rb new file mode 100644 index 0000000000..7567ed5988 --- /dev/null +++ b/db/migrate/20180710172959_disallow_multi_levels_theme_components.rb @@ -0,0 +1,68 @@ +class DisallowMultiLevelsThemeComponents < ActiveRecord::Migration[5.2] + def up + @handled = [] + top_parents = DB.query(" + SELECT parent_theme_id, child_theme_id + FROM child_themes + WHERE parent_theme_id NOT IN (SELECT child_theme_id FROM child_themes) + ") + + top_parents.each do |top_parent| + migrate_child(top_parent, top_parent) + end + + if @handled.size > 0 + execute(" + DELETE FROM child_themes + WHERE parent_theme_id NOT IN (#{top_parents.map(&:parent_theme_id).join(", ")}) + ") + end + + execute(" + UPDATE themes + SET user_selectable = false + FROM child_themes + WHERE themes.id = child_themes.child_theme_id + AND themes.user_selectable = true + ") + + default = DB.query_single("SELECT value FROM site_settings WHERE name = 'default_theme_id'").first + if default + default_child = DB.query("SELECT 1 AS one FROM child_themes WHERE child_theme_id = ?", default.to_i).present? + execute("DELETE FROM site_settings WHERE name = 'default_theme_id'") if default_child + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end + + private + + def migrate_child(parent, top_parent) + unless already_exists?(top_parent.parent_theme_id, parent.child_theme_id) + execute(" + INSERT INTO child_themes (parent_theme_id, child_theme_id, created_at, updated_at) + VALUES (#{top_parent.parent_theme_id}, #{parent.child_theme_id}, now(), now()) + ") + end + + @handled << [top_parent.parent_theme_id, parent.parent_theme_id, parent.child_theme_id] + + children = DB.query(" + SELECT parent_theme_id, child_theme_id + FROM child_themes + WHERE parent_theme_id = :child", child: parent.child_theme_id + ) + + children.each do |child| + unless @handled.include?([top_parent.parent_theme_id, child.parent_theme_id, child.child_theme_id]) + migrate_child(child, top_parent) + end + end + end + + def already_exists?(parent, child) + DB.query("SELECT 1 AS one FROM child_themes WHERE child_theme_id = :child AND parent_theme_id = :parent", child: child, parent: parent).present? + end +end diff --git a/lib/guardian.rb b/lib/guardian.rb index 9c70f4b901..db7d168428 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -359,9 +359,15 @@ class Guardian end def allow_themes?(theme_ids) - theme_ids = [theme_ids] unless theme_ids.is_a?(Array) - allowed_ids = is_staff? ? Theme.theme_ids : Theme.user_theme_ids - (theme_ids - allowed_ids.to_a).empty? + if is_staff? && (theme_ids - Theme.theme_ids).blank? + return true + end + + parent = theme_ids.first + components = theme_ids[1..-1] || [] + + Theme.user_theme_ids.include?(parent) && + (components - Theme.components_for(parent)).empty? end private diff --git a/lib/middleware/anonymous_cache.rb b/lib/middleware/anonymous_cache.rb index 93620be0c9..f2539b0610 100644 --- a/lib/middleware/anonymous_cache.rb +++ b/lib/middleware/anonymous_cache.rb @@ -66,16 +66,16 @@ module Middleware end def cache_key - @cache_key ||= "ANON_CACHE_#{@env["HTTP_ACCEPT"]}_#{@env["HTTP_HOST"]}#{@env["REQUEST_URI"]}|m=#{is_mobile?}|c=#{is_crawler?}|b=#{has_brotli?}|t=#{theme_id}" + @cache_key ||= "ANON_CACHE_#{@env["HTTP_ACCEPT"]}_#{@env["HTTP_HOST"]}#{@env["REQUEST_URI"]}|m=#{is_mobile?}|c=#{is_crawler?}|b=#{has_brotli?}|t=#{theme_ids.join(",")}" end - def theme_id + def theme_ids ids, _ = @request.cookies['theme_ids']&.split('|') ids = ids&.split(",")&.map(&:to_i) if ids && Guardian.new.allow_themes?(ids) - ids.first + Theme.transform_ids(ids) else - nil + [] end end diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index a70ccae052..dec9325ac8 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -8,6 +8,7 @@ class Stylesheet::Manager CACHE_PATH ||= 'tmp/stylesheet-cache' MANIFEST_DIR ||= "#{Rails.root}/tmp/cache/assets/#{Rails.env}" MANIFEST_FULL_PATH ||= "#{MANIFEST_DIR}/stylesheet-manifest" + THEME_REGEX ||= /_theme$/ @lock = Mutex.new @@ -19,38 +20,65 @@ class Stylesheet::Manager cache.hash.keys.select { |k| k =~ /theme/ }.each { |k|cache.delete(k) } end - def self.stylesheet_href(target = :desktop, theme_id = :missing) - href = stylesheet_link_tag(target, 'all', theme_id) - if href - href.split(/["']/)[1] - end + def self.stylesheet_data(target = :desktop, theme_ids = :missing) + stylesheet_details(target, "all", theme_ids) end - def self.stylesheet_link_tag(target = :desktop, media = 'all', theme_id = :missing) + def self.stylesheet_link_tag(target = :desktop, media = 'all', theme_ids = :missing) + stylesheets = stylesheet_details(target, media, theme_ids) + stylesheets.map do |stylesheet| + href = stylesheet[:new_href] + theme_id = stylesheet[:theme_id] + data_theme_id = theme_id ? "data-theme-id=\"#{theme_id}\"" : "" + %[] + end.join("\n").html_safe + end + + def self.stylesheet_details(target = :desktop, media = 'all', theme_ids = :missing) + if theme_ids == :missing + theme_ids = [SiteSetting.default_theme_id] + end target = target.to_sym - if theme_id == :missing - theme_id = SiteSetting.default_theme_id - end + theme_ids = [theme_ids] unless Array === theme_ids + theme_ids = [theme_ids.first] unless target =~ THEME_REGEX + theme_ids = Theme.transform_ids(theme_ids, extend: false) current_hostname = Discourse.current_hostname - cache_key = "#{target}_#{theme_id}_#{current_hostname}" - tag = cache[cache_key] - return tag.dup.html_safe if tag + array_cache_key = "array_themes_#{theme_ids.join(",")}_#{target}_#{current_hostname}" + stylesheets = cache[array_cache_key] + return stylesheets if stylesheets.present? @lock.synchronize do - builder = self.new(target, theme_id) - if builder.is_theme? && !builder.theme - tag = "" - else - builder.compile unless File.exists?(builder.stylesheet_fullpath) - tag = %[] - end + stylesheets = [] + theme_ids.each do |theme_id| + data = { target: target } + cache_key = "path_#{target}_#{theme_id}_#{current_hostname}" + href = cache[cache_key] - cache[cache_key] = tag - tag.dup.html_safe + unless href + builder = self.new(target, theme_id) + is_theme = builder.is_theme? + has_theme = builder.theme.present? + + if is_theme && !has_theme + next + else + data[:theme_id] = builder.theme.id if has_theme && is_theme + builder.compile unless File.exists?(builder.stylesheet_fullpath) + href = builder.stylesheet_path(current_hostname) + end + cache[cache_key] = href + end + + data[:theme_id] = theme_id if theme_id.present? && data[:theme_id].blank? + data[:new_href] = href + stylesheets << data + end + cache[array_cache_key] = stylesheets.freeze + stylesheets end end @@ -100,6 +128,10 @@ class Stylesheet::Manager end.compact.max.to_i end + def self.cache_fullpath + "#{Rails.root}/#{CACHE_PATH}" + end + def initialize(target = :desktop, theme_id) @target = target @theme_id = theme_id @@ -162,10 +194,6 @@ class Stylesheet::Manager css end - def self.cache_fullpath - "#{Rails.root}/#{CACHE_PATH}" - end - def cache_fullpath self.class.cache_fullpath end @@ -225,7 +253,7 @@ class Stylesheet::Manager end def is_theme? - !!(@target.to_s =~ /_theme$/) + !!(@target.to_s =~ THEME_REGEX) end # digest encodes the things that trigger a recompile @@ -240,7 +268,7 @@ class Stylesheet::Manager end def theme - @theme ||= (Theme.find_by(id: @theme_id) || :nil) + @theme ||= Theme.find_by(id: @theme_id) || :nil @theme == :nil ? nil : @theme end @@ -271,7 +299,16 @@ class Stylesheet::Manager end def settings_digest - Digest::SHA1.hexdigest((theme&.included_settings || {}).to_json) + fields = ThemeField.where( + name: "yaml", + type_id: ThemeField.types[:yaml], + theme_id: @theme_id + ).pluck(:updated_at) + + settings = ThemeSetting.where(theme_id: @theme_id).pluck(:updated_at) + timestamps = fields.concat(settings).map!(&:to_f).sort!.join(",") + + Digest::SHA1.hexdigest(timestamps) end def uploads_digest diff --git a/lib/stylesheet/watcher.rb b/lib/stylesheet/watcher.rb index 91b13343d8..e2425c4fd7 100644 --- a/lib/stylesheet/watcher.rb +++ b/lib/stylesheet/watcher.rb @@ -8,7 +8,10 @@ module Stylesheet end def self.theme_id - @theme_id || SiteSetting.default_theme_id + if @theme_id.blank? && SiteSetting.default_theme_id != -1 + @theme_id = SiteSetting.default_theme_id + end + @theme_id end def self.watch(paths = nil) @@ -76,12 +79,8 @@ module Stylesheet Stylesheet::Manager.cache.clear message = ["desktop", "mobile", "admin"].map do |name| - { - target: name, - new_href: Stylesheet::Manager.stylesheet_href(name.to_sym), - theme_id: Stylesheet::Watcher.theme_id - } - end + Stylesheet::Manager.stylesheet_data(name.to_sym, Stylesheet::Watcher.theme_id) + end.flatten MessageBus.publish '/file-change', message end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 9b95f66e32..bdd0ef6092 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -2537,6 +2537,46 @@ describe Guardian do end end + describe "#allow_themes?" do + let(:theme) { Fabricate(:theme) } + let(:theme2) { Fabricate(:theme) } + + it "allows staff to use any themes" do + expect(Guardian.new(moderator).allow_themes?([theme.id, theme2.id])).to eq(true) + expect(Guardian.new(admin).allow_themes?([theme.id, theme2.id])).to eq(true) + end + + it "only allows normal users to use user-selectable themes or default theme" do + user_guardian = Guardian.new(user) + + expect(user_guardian.allow_themes?([theme.id, theme2.id])).to eq(false) + expect(user_guardian.allow_themes?([theme.id])).to eq(false) + expect(user_guardian.allow_themes?([theme2.id])).to eq(false) + + theme.set_default! + expect(user_guardian.allow_themes?([theme.id])).to eq(true) + expect(user_guardian.allow_themes?([theme2.id])).to eq(false) + expect(user_guardian.allow_themes?([theme.id, theme2.id])).to eq(false) + + theme2.update!(user_selectable: true) + expect(user_guardian.allow_themes?([theme2.id])).to eq(true) + expect(user_guardian.allow_themes?([theme2.id, theme.id])).to eq(false) + end + + it "allows child themes to be only used with their parent" do + user_guardian = Guardian.new(user) + + theme.update!(user_selectable: true) + theme2.update!(user_selectable: true) + expect(user_guardian.allow_themes?([theme.id, theme2.id])).to eq(false) + + theme2.update!(user_selectable: false) + theme.add_child_theme!(theme2) + expect(user_guardian.allow_themes?([theme.id, theme2.id])).to eq(true) + expect(user_guardian.allow_themes?([theme2.id])).to eq(false) + end + end + describe 'can_wiki?' do let(:post) { build(:post, created_at: 1.minute.ago) } diff --git a/spec/components/middleware/anonymous_cache_spec.rb b/spec/components/middleware/anonymous_cache_spec.rb index ce0b01e576..4d63d4f5e4 100644 --- a/spec/components/middleware/anonymous_cache_spec.rb +++ b/spec/components/middleware/anonymous_cache_spec.rb @@ -32,7 +32,7 @@ describe Middleware::AnonymousCache::Helper do context "per theme cache" do it "handles theme keys" do - theme = Theme.create(name: "test", user_id: -1, user_selectable: true) + theme = Fabricate(:theme, user_selectable: true) with_bad_theme_key = new_helper("HTTP_COOKIE" => "theme_ids=abc").cache_key with_no_theme_key = new_helper().cache_key diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index 1d4c19fd90..2df206297b 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -8,7 +8,7 @@ describe Stylesheet::Manager do link = Stylesheet::Manager.stylesheet_link_tag(:embedded_theme) expect(link).to eq("") - theme = Theme.create(name: "embedded", user_id: -1) + theme = Fabricate(:theme) SiteSetting.default_theme_id = theme.id link = Stylesheet::Manager.stylesheet_link_tag(:embedded_theme) @@ -16,10 +16,7 @@ describe Stylesheet::Manager do end it 'can correctly compile theme css' do - theme = Theme.new( - name: 'parent', - user_id: -1 - ) + theme = Fabricate(:theme) theme.set_field(target: :common, name: "scss", value: ".common{.scss{color: red;}}") theme.set_field(target: :desktop, name: "scss", value: ".desktop{.scss{color: red;}}") @@ -28,10 +25,7 @@ describe Stylesheet::Manager do theme.save! - child_theme = Theme.new( - name: 'parent', - user_id: -1, - ) + child_theme = Fabricate(:theme) child_theme.set_field(target: :common, name: "scss", value: ".child_common{.scss{color: red;}}") child_theme.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}") @@ -72,10 +66,7 @@ describe Stylesheet::Manager do it 'can correctly account for plugins in digest' do - theme = Theme.create!( - name: 'parent', - user_id: -1 - ) + theme = Fabricate(:theme) manager = Stylesheet::Manager.new(:desktop_theme, theme.id) digest1 = manager.digest @@ -92,10 +83,7 @@ describe Stylesheet::Manager do let(:image2) { file_from_fixtures("logo-dev.png") } it 'can correctly account for theme uploads in digest' do - theme = Theme.create!( - name: 'parent', - user_id: -1 - ) + theme = Fabricate(:theme) upload = UploadCreator.new(image, "logo.png").create_for(-1) field = ThemeField.create!( @@ -130,10 +118,7 @@ describe Stylesheet::Manager do describe 'color_scheme_digest' do it "changes with category background image" do - theme = Theme.new( - name: 'parent', - user_id: -1 - ) + theme = Fabricate(:theme) category1 = Fabricate(:category, uploaded_background_id: 123, updated_at: 1.week.ago) category2 = Fabricate(:category, uploaded_background_id: 456, updated_at: 2.days.ago) diff --git a/spec/components/theme_settings_manager_spec.rb b/spec/components/theme_settings_manager_spec.rb index eb29ed2024..1dd8a93d06 100644 --- a/spec/components/theme_settings_manager_spec.rb +++ b/spec/components/theme_settings_manager_spec.rb @@ -4,7 +4,7 @@ require 'theme_settings_manager' describe ThemeSettingsManager do let(:theme_settings) do - theme = Theme.create!(name: "awesome theme", user_id: -1) + theme = Fabricate(:theme) yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/valid_settings.yaml") theme.set_field(target: :settings, name: "yaml", value: yaml) theme.save! diff --git a/spec/components/wizard/step_updater_spec.rb b/spec/components/wizard/step_updater_spec.rb index 0a858c8b88..54b4535c48 100644 --- a/spec/components/wizard/step_updater_spec.rb +++ b/spec/components/wizard/step_updater_spec.rb @@ -194,6 +194,7 @@ describe Wizard::StepUpdater do context "without an existing scheme" do it "creates the scheme" do + ColorScheme.destroy_all updater = wizard.create_updater('colors', theme_previews: 'Dark', allow_dark_light_selection: true) updater.update expect(updater.success?).to eq(true) diff --git a/spec/fabricators/theme_fabricator.rb b/spec/fabricators/theme_fabricator.rb new file mode 100644 index 0000000000..39712756c3 --- /dev/null +++ b/spec/fabricators/theme_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:theme) do + name { sequence(:name) { |i| "Cool theme #{i + 1}" } } + user +end diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 015789e0b7..a0400e5d33 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -191,12 +191,12 @@ describe UserNotifications do Fabricate(:color_scheme_color, name: 'header_background', hex: '1E1E1E'), Fabricate(:color_scheme_color, name: 'tertiary', hex: '858585') ]) - theme = Theme.create!( - name: 'my name', - user_id: Fabricate(:admin).id, + theme = Fabricate(:theme, user_selectable: true, + user: Fabricate(:admin), color_scheme_id: cs.id ) + theme.set_default! html = subject.html_part.body.to_s diff --git a/spec/models/admin_dashboard_data_spec.rb b/spec/models/admin_dashboard_data_spec.rb index 1a96b1fe9e..2f4db5fa47 100644 --- a/spec/models/admin_dashboard_data_spec.rb +++ b/spec/models/admin_dashboard_data_spec.rb @@ -338,7 +338,7 @@ describe AdminDashboardData do describe '#out_of_date_themes' do let(:remote) { RemoteTheme.create!(remote_url: "https://github.com/org/testtheme") } - let!(:theme) { Theme.create!(remote_theme_id: remote.id, name: "Test< Theme", user_id: -1) } + let!(:theme) { Fabricate(:theme, remote_theme: remote, name: "Test< Theme") } it "outputs correctly formatted html" do remote.update!(local_version: "old version", remote_version: "new version", commits_behind: 2) diff --git a/spec/models/child_theme_spec.rb b/spec/models/child_theme_spec.rb new file mode 100644 index 0000000000..2c13206cb1 --- /dev/null +++ b/spec/models/child_theme_spec.rb @@ -0,0 +1,43 @@ +require 'rails_helper' + +describe ChildTheme do + describe "validations" do + it "doesn't allow children to become parents or parents to become children" do + theme = Fabricate(:theme) + child = Fabricate(:theme) + + child_theme = ChildTheme.new(parent_theme: theme, child_theme: child) + expect(child_theme.valid?).to eq(true) + child_theme.save! + + grandchild = Fabricate(:theme) + child_theme = ChildTheme.new(parent_theme: child, child_theme: grandchild) + expect(child_theme.valid?).to eq(false) + expect(child_theme.errors.full_messages).to contain_exactly(I18n.t("themes.errors.no_multilevels_components")) + + grandparent = Fabricate(:theme) + child_theme = ChildTheme.new(parent_theme: grandparent, child_theme: theme) + expect(child_theme.valid?).to eq(false) + expect(child_theme.errors.full_messages).to contain_exactly(I18n.t("themes.errors.no_multilevels_components")) + end + + it "doesn't allow a user selectable theme to be a child" do + parent = Fabricate(:theme) + selectable_theme = Fabricate(:theme, user_selectable: true) + + child_theme = ChildTheme.new(parent_theme: parent, child_theme: selectable_theme) + expect(child_theme.valid?).to eq(false) + expect(child_theme.errors.full_messages).to contain_exactly(I18n.t("themes.errors.component_no_user_selectable")) + end + + it "doesn't allow a default theme to be child" do + parent = Fabricate(:theme) + default = Fabricate(:theme) + default.set_default! + + child_theme = ChildTheme.new(parent_theme: parent, child_theme: default) + expect(child_theme.valid?).to eq(false) + expect(child_theme.errors.full_messages).to contain_exactly(I18n.t("themes.errors.component_no_default")) + end + end +end diff --git a/spec/models/color_scheme_spec.rb b/spec/models/color_scheme_spec.rb index 25121d4997..d1ea51532f 100644 --- a/spec/models/color_scheme_spec.rb +++ b/spec/models/color_scheme_spec.rb @@ -10,15 +10,15 @@ describe ColorScheme do it "correctly invalidates theme css when changed" do scheme = ColorScheme.create_from_base(name: 'Bob') - theme = Theme.new(name: 'Amazing Theme', color_scheme_id: scheme.id, user_id: -1) + theme = Fabricate(:theme, color_scheme_id: scheme.id) theme.set_field(name: :scss, target: :desktop, value: '.bob {color: $primary;}') theme.save! - href = Stylesheet::Manager.stylesheet_href(:desktop_theme, theme.id) + href = Stylesheet::Manager.stylesheet_data(:desktop_theme, theme.id)[0][:new_href] ColorSchemeRevisor.revise(scheme, colors: [{ name: 'primary', hex: 'bbb' }]) - href2 = Stylesheet::Manager.stylesheet_href(:desktop_theme, theme.id) + href2 = Stylesheet::Manager.stylesheet_data(:desktop_theme, theme.id)[0][:new_href] expect(href).not_to eq(href2) end diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb index 1cb0211035..ab5eb010a9 100644 --- a/spec/models/remote_theme_spec.rb +++ b/spec/models/remote_theme_spec.rb @@ -189,7 +189,7 @@ describe RemoteTheme do context ".out_of_date_themes" do let(:remote) { RemoteTheme.create!(remote_url: "https://github.com/org/testtheme") } - let!(:theme) { Theme.create!(remote_theme_id: remote.id, name: "Test Theme", user_id: -1) } + let!(:theme) { Fabricate(:theme, remote_theme: remote) } it "finds out of date themes" do remote.update!(local_version: "old version", remote_version: "new version", commits_behind: 2) diff --git a/spec/models/site_spec.rb b/spec/models/site_spec.rb index ecb3d4f8f0..2f578d157d 100644 --- a/spec/models/site_spec.rb +++ b/spec/models/site_spec.rb @@ -17,9 +17,9 @@ describe Site do end it "includes user themes and expires them as needed" do - default_theme = Theme.create!(user_id: -1, name: 'default') + default_theme = Fabricate(:theme) SiteSetting.default_theme_id = default_theme.id - user_theme = Theme.create!(user_id: -1, name: 'user theme', user_selectable: true) + user_theme = Fabricate(:theme, user_selectable: true) anon_guardian = Guardian.new user_guardian = Guardian.new(Fabricate(:user)) diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb index 8ec0a5249b..152531d8d1 100644 --- a/spec/models/theme_field_spec.rb +++ b/spec/models/theme_field_spec.rb @@ -7,6 +7,26 @@ describe ThemeField do ThemeField.destroy_all end + describe "scope: find_by_theme_ids" do + it "returns result in the specified order" do + theme = Fabricate(:theme) + theme2 = Fabricate(:theme) + theme3 = Fabricate(:theme) + + (0..1).each do |num| + ThemeField.create!(theme: theme, target_id: num, name: "header", value: "html") + ThemeField.create!(theme: theme2, target_id: num, name: "header", value: "html") + ThemeField.create!(theme: theme3, target_id: num, name: "header", value: "html") + end + + expect(ThemeField.find_by_theme_ids( + [theme3.id, theme.id, theme2.id] + ).pluck(:theme_id)).to eq( + [theme3.id, theme3.id, theme.id, theme.id, theme2.id, theme2.id] + ) + end + end + it "correctly generates errors for transpiled js" do html = < diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index d12e0fd10a..b17a155640 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -19,15 +19,16 @@ describe Theme do end let :customization do - Theme.create!(customization_params) + Fabricate(:theme, customization_params) end + let(:theme) { Fabricate(:theme, user: user) } + let(:child) { Fabricate(:theme, user: user) } it 'can properly clean up color schemes' do - theme = Theme.create!(name: 'bob', user_id: -1) scheme = ColorScheme.create!(theme_id: theme.id, name: 'test') scheme2 = ColorScheme.create!(theme_id: theme.id, name: 'test2') - Theme.create!(name: 'bob', user_id: -1, color_scheme_id: scheme2.id) + Fabricate(:theme, color_scheme_id: scheme2.id) theme.destroy! scheme2.reload @@ -38,8 +39,6 @@ describe Theme do end it 'can support child themes' do - child = Theme.new(name: '2', user_id: user.id) - child.set_field(target: :common, name: "header", value: "World") child.set_field(target: :desktop, name: "header", value: "Desktop") child.set_field(target: :mobile, name: "header", value: "Mobile") @@ -54,7 +53,7 @@ describe Theme do expect(Theme.lookup_field(child.id, :mobile, :header)).to eq("Worldie\nMobile") - parent = Theme.new(name: '1', user_id: user.id) + parent = Fabricate(:theme, user: user) parent.set_field(target: :common, name: "header", value: "Common Parent") parent.set_field(target: :mobile, name: "header", value: "Mobile Parent") @@ -68,18 +67,39 @@ describe Theme do end it 'can correctly find parent themes' do - grandchild = Theme.create!(name: 'grandchild', user_id: user.id) - child = Theme.create!(name: 'child', user_id: user.id) - theme = Theme.create!(name: 'theme', user_id: user.id) + theme.add_child_theme!(child) + + expect(child.dependant_themes.length).to eq(1) + end + + it "doesn't allow multi-level theme components" do + grandchild = Fabricate(:theme, user: user) + grandparent = Fabricate(:theme, user: user) theme.add_child_theme!(child) - child.add_child_theme!(grandchild) + expect do + child.add_child_theme!(grandchild) + end.to raise_error(Discourse::InvalidParameters, I18n.t("themes.errors.no_multilevels_components")) - expect(grandchild.dependant_themes.length).to eq(2) + expect do + grandparent.add_child_theme!(theme) + end.to raise_error(Discourse::InvalidParameters, I18n.t("themes.errors.no_multilevels_components")) + end + + it "doesn't allow a child to be user selectable" do + theme.add_child_theme!(child) + child.update(user_selectable: true) + expect(child.errors.full_messages).to contain_exactly(I18n.t("themes.errors.component_no_user_selectable")) + end + + it "doesn't allow a child to be set as the default theme" do + theme.add_child_theme!(child) + expect do + child.set_default! + end.to raise_error(Discourse::InvalidParameters, I18n.t("themes.errors.component_no_default")) end it 'should correct bad html in body_tag_baked and head_tag_baked' do - theme = Theme.new(user_id: -1, name: "test") theme.set_field(target: :common, name: "head_tag", value: "I am bold") theme.save! @@ -95,7 +115,6 @@ describe Theme do {{hello}} HTML - theme = Theme.new(user_id: -1, name: "test") theme.set_field(target: :common, name: "header", value: with_template) theme.save! @@ -106,8 +125,6 @@ HTML end it 'should create body_tag_baked on demand if needed' do - - theme = Theme.new(user_id: -1, name: "test") theme.set_field(target: :common, name: :body_tag, value: "test") theme.save @@ -116,6 +133,41 @@ HTML expect(Theme.lookup_field(theme.id, :desktop, :body_tag)).to match(/test<\/b>/) end + it 'can find fields for multiple themes' do + theme2 = Fabricate(:theme) + + theme.set_field(target: :common, name: :body_tag, value: "testtheme1") + theme2.set_field(target: :common, name: :body_tag, value: "theme2test") + theme.save! + theme2.save! + + field = Theme.lookup_field([theme.id, theme2.id], :desktop, :body_tag) + expect(field).to match(/testtheme1<\/b>/) + expect(field).to match(/theme2test<\/b>/) + end + + describe ".transform_ids" do + it "adds the child themes of the parent" do + child = Fabricate(:theme, id: 97) + child2 = Fabricate(:theme, id: 96) + + theme.add_child_theme!(child) + theme.add_child_theme!(child2) + expect(Theme.transform_ids([theme.id])).to eq([theme.id, child2.id, child.id]) + expect(Theme.transform_ids([theme.id, 94, 90])).to eq([theme.id, 90, 94, child2.id, child.id]) + end + + it "doesn't insert children when extend is false" do + child = Fabricate(:theme, id: 97) + child2 = Fabricate(:theme, id: 96) + + theme.add_child_theme!(child) + theme.add_child_theme!(child2) + expect(Theme.transform_ids([theme.id], extend: false)).to eq([theme.id]) + expect(Theme.transform_ids([theme.id, 94, 90, 70, 70], extend: false)).to eq([theme.id, 70, 90, 94]) + end + end + context "plugin api" do def transpile(html) f = ThemeField.create!(target_id: Theme.targets[:mobile], theme_id: 1, name: "after_header", value: html) @@ -152,14 +204,12 @@ HTML context 'theme vars' do it 'works in parent theme' do - - theme = Theme.new(name: 'theme', user_id: -1) theme.set_field(target: :common, name: :scss, value: 'body {color: $magic; }') theme.set_field(target: :common, name: :magic, value: 'red', type: :theme_var) theme.set_field(target: :common, name: :not_red, value: 'red', type: :theme_var) theme.save - parent_theme = Theme.new(name: 'parent theme', user_id: -1) + parent_theme = Fabricate(:theme) parent_theme.set_field(target: :common, name: :scss, value: 'body {background-color: $not_red; }') parent_theme.set_field(target: :common, name: :not_red, value: 'blue', type: :theme_var) parent_theme.save @@ -171,7 +221,6 @@ HTML end it 'can generate scss based off theme vars' do - theme = Theme.new(name: 'theme', user_id: -1) theme.set_field(target: :common, name: :scss, value: 'body {color: $magic; content: quote($content)}') theme.set_field(target: :common, name: :magic, value: 'red', type: :theme_var) theme.set_field(target: :common, name: :content, value: 'Sam\'s Test', type: :theme_var) @@ -187,7 +236,6 @@ HTML end it 'can handle uploads based of ThemeField' do - theme = Theme.new(name: 'theme', user_id: -1) upload = UploadCreator.new(image, "logo.png").create_for(-1) theme.set_field(target: :common, name: :logo, upload_id: upload.id, type: :theme_upload_var) theme.set_field(target: :common, name: :scss, value: 'body {background-image: url($logo)}') @@ -210,7 +258,6 @@ HTML context "theme settings" do it "allows values to be used in scss" do - theme = Theme.new(name: "awesome theme", user_id: -1) theme.set_field(target: :settings, name: :yaml, value: "background_color: red\nfont_size: 25px") theme.set_field(target: :common, name: :scss, value: 'body {background-color: $background_color; font-size: $font-size}') theme.save! @@ -227,7 +274,6 @@ HTML end it "allows values to be used in JS" do - theme = Theme.new(name: "awesome theme", user_id: -1) theme.set_field(target: :settings, name: :yaml, value: "name: bob") theme.set_field(target: :common, name: :after_header, value: '') theme.save! @@ -263,26 +309,30 @@ HTML it 'correctly caches theme ids' do Theme.destroy_all - theme = Theme.create!(name: "bob", user_id: -1) + theme + theme2 = Fabricate(:theme) - expect(Theme.theme_ids).to eq(Set.new([theme.id])) - expect(Theme.user_theme_ids).to eq(Set.new([])) + expect(Theme.theme_ids).to contain_exactly(theme.id, theme2.id) + expect(Theme.user_theme_ids).to eq([]) - theme.user_selectable = true - theme.save + theme.update!(user_selectable: true) - expect(Theme.user_theme_ids).to eq(Set.new([theme.id])) + expect(Theme.user_theme_ids).to contain_exactly(theme.id) - theme.user_selectable = false - theme.save + theme2.update!(user_selectable: true) + expect(Theme.user_theme_ids).to contain_exactly(theme.id, theme2.id) + + theme.update!(user_selectable: false) + theme2.update!(user_selectable: false) theme.set_default! - expect(Theme.user_theme_ids).to eq(Set.new([theme.id])) + expect(Theme.user_theme_ids).to contain_exactly(theme.id) theme.destroy + theme2.destroy - expect(Theme.theme_ids).to eq(Set.new([])) - expect(Theme.user_theme_ids).to eq(Set.new([])) + expect(Theme.theme_ids).to eq([]) + expect(Theme.user_theme_ids).to eq([]) end it 'correctly caches user_themes template' do @@ -292,8 +342,7 @@ HTML user_themes = JSON.parse(json)["user_themes"] expect(user_themes).to eq([]) - theme = Theme.create!(name: "bob", user_id: -1, user_selectable: true) - theme.save! + theme = Fabricate(:theme, name: "bob", user_selectable: true) json = Site.json_for(guardian) user_themes = JSON.parse(json)["user_themes"].map { |t| t["name"] } @@ -320,7 +369,6 @@ HTML it 'handles settings cache correctly' do Theme.destroy_all - theme = Theme.create!(name: "awesome theme", user_id: -1) expect(cached_settings(theme.id)).to eq("{}") theme.set_field(target: :settings, name: "yaml", value: "boolean_setting: true") @@ -330,7 +378,6 @@ HTML theme.settings.first.value = "false" expect(cached_settings(theme.id)).to match(/\"boolean_setting\":false/) - child = Theme.create!(name: "child theme", user_id: -1) child.set_field(target: :settings, name: "yaml", value: "integer_setting: 54") child.save! @@ -347,5 +394,4 @@ HTML expect(json).not_to match(/\"integer_setting\":54/) expect(json).to match(/\"boolean_setting\":false/) end - end diff --git a/spec/requests/admin/staff_action_logs_controller_spec.rb b/spec/requests/admin/staff_action_logs_controller_spec.rb index 12acd22244..f970be10c7 100644 --- a/spec/requests/admin/staff_action_logs_controller_spec.rb +++ b/spec/requests/admin/staff_action_logs_controller_spec.rb @@ -30,7 +30,7 @@ describe Admin::StaffActionLogsController do describe '#diff' do it 'can generate diffs for theme changes' do - theme = Theme.new(user_id: -1, name: 'bob') + theme = Fabricate(:theme) theme.set_field(target: :mobile, name: :scss, value: 'body {.up}') theme.set_field(target: :common, name: :scss, value: 'omit-dupe') diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index fc9e8cd5d2..9d5a6de5ba 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -49,7 +49,7 @@ describe Admin::ThemesController do it 'can import a theme with an upload' do upload = Fabricate(:upload) - theme = Theme.new(name: 'with-upload', user_id: -1) + theme = Fabricate(:theme) upload = UploadCreator.new(image, "logo.png").create_for(-1) theme.set_field(target: :common, name: :logo, upload_id: upload.id, type: :theme_upload_var) theme.save! @@ -93,7 +93,7 @@ describe Admin::ThemesController do ColorScheme.destroy_all Theme.destroy_all - theme = Theme.new(name: 'my name', user_id: -1) + theme = Fabricate(:theme) theme.set_field(target: :common, name: :scss, value: '.body{color: black;}') theme.set_field(target: :desktop, name: :after_header, value: 'test') @@ -141,7 +141,7 @@ describe Admin::ThemesController do end describe '#update' do - let(:theme) { Theme.create(name: 'my name', user_id: -1) } + let(:theme) { Fabricate(:theme) } it 'can change default theme' do SiteSetting.default_theme_id = -1 @@ -169,7 +169,7 @@ describe Admin::ThemesController do theme.set_field(target: :common, name: :scss, value: '.body{color: black;}') theme.save - child_theme = Theme.create(name: 'my name', user_id: -1) + child_theme = Fabricate(:theme) upload = Fabricate(:upload) @@ -198,5 +198,17 @@ describe Admin::ThemesController do expect(json["theme"]["child_themes"].length).to eq(1) expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1) end + + it 'returns the right error message' do + parent = Fabricate(:theme) + parent.add_child_theme!(theme) + + put "/admin/themes/#{theme.id}.json", params: { + theme: { default: true } + } + + expect(response.status).to eq(400) + expect(JSON.parse(response.body)["errors"].first).to include(I18n.t("themes.errors.component_no_default")) + end end end diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index f498c807b5..f45f1c4fa5 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -33,4 +33,74 @@ RSpec.describe ApplicationController do end end + describe "#handle_theme" do + let(:theme) { Fabricate(:theme, user_selectable: true) } + let(:theme2) { Fabricate(:theme, user_selectable: true) } + let(:user) { Fabricate(:user) } + let(:admin) { Fabricate(:admin) } + + before do + sign_in(user) + end + + it "selects the theme the user has selected" do + user.user_option.update_columns(theme_ids: [theme.id]) + + get "/" + expect(response.status).to eq(200) + expect(controller.theme_ids).to eq([theme.id]) + + theme.update_attribute(:user_selectable, false) + + get "/" + expect(response.status).to eq(200) + expect(controller.theme_ids).to eq([SiteSetting.default_theme_id]) + end + + it "can be overridden with a cookie" do + user.user_option.update_columns(theme_ids: [theme.id]) + + cookies['theme_ids'] = "#{theme2.id}|#{user.user_option.theme_key_seq}" + + get "/" + expect(response.status).to eq(200) + expect(controller.theme_ids).to eq([theme2.id]) + + theme2.update!(user_selectable: false) + theme.add_child_theme!(theme2) + cookies['theme_ids'] = "#{theme.id},#{theme2.id}|#{user.user_option.theme_key_seq}" + + get "/" + expect(response.status).to eq(200) + expect(controller.theme_ids).to eq([theme.id, theme2.id]) + end + + it "falls back to the default theme when the user has no cookies or preferences" do + user.user_option.update_columns(theme_ids: []) + cookies["theme_ids"] = nil + theme2.set_default! + + get "/" + expect(response.status).to eq(200) + expect(controller.theme_ids).to eq([theme2.id]) + end + + it "can be overridden with preview_theme_id param" do + sign_in(admin) + cookies['theme_ids'] = "#{theme.id},#{theme2.id}|#{admin.user_option.theme_key_seq}" + + get "/", params: { preview_theme_id: theme2.id } + expect(response.status).to eq(200) + expect(controller.theme_ids).to eq([theme2.id]) + end + + it "cookie can fail back to user if out of sync" do + user.user_option.update_columns(theme_ids: [theme.id]) + cookies['theme_ids'] = "#{theme2.id}|#{user.user_option.theme_key_seq - 1}" + + get "/" + expect(response.status).to eq(200) + expect(controller.theme_ids).to eq([theme.id]) + end + end end diff --git a/spec/requests/embed_controller_spec.rb b/spec/requests/embed_controller_spec.rb index f7f5c11eee..85601b0b10 100644 --- a/spec/requests/embed_controller_spec.rb +++ b/spec/requests/embed_controller_spec.rb @@ -83,7 +83,7 @@ describe EmbedController do end it "includes CSS from embedded_scss field" do - theme = Theme.create!(name: "Awesome blog", user_id: -1) + theme = Fabricate(:theme) theme.set_default! ThemeField.create!( diff --git a/spec/requests/stylesheets_controller_spec.rb b/spec/requests/stylesheets_controller_spec.rb index 3ccecc7ab1..138eb202ab 100644 --- a/spec/requests/stylesheets_controller_spec.rb +++ b/spec/requests/stylesheets_controller_spec.rb @@ -27,7 +27,7 @@ describe StylesheetsController do it 'can lookup theme specific css' do scheme = ColorScheme.create_from_base(name: "testing", colors: []) - theme = Theme.create!(name: "test", color_scheme_id: scheme.id, user_id: -1) + theme = Fabricate(:theme, color_scheme_id: scheme.id) builder = Stylesheet::Manager.new(:desktop, theme.id) builder.compile diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index e98a2ab94a..a4e43d0bf3 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1218,48 +1218,6 @@ RSpec.describe TopicsController do expect(response.headers['X-Robots-Tag']).to eq(nil) end - describe "themes" do - let(:theme) { Theme.create!(user_id: -1, name: 'bob', user_selectable: true) } - let(:theme2) { Theme.create!(user_id: -1, name: 'bobbob', user_selectable: true) } - - before do - sign_in(user) - end - - it "selects the theme the user has selected" do - user.user_option.update_columns(theme_ids: [theme.id]) - - get "/t/#{topic.id}" - expect(response).to be_redirect - expect(controller.theme_id).to eq(theme.id) - - theme.update_attribute(:user_selectable, false) - - get "/t/#{topic.id}" - expect(response).to be_redirect - expect(controller.theme_id).not_to eq(theme.id) - end - - it "can be overridden with a cookie" do - user.user_option.update_columns(theme_ids: [theme.id]) - - cookies['theme_ids'] = "#{theme2.id}|#{user.user_option.theme_key_seq}" - - get "/t/#{topic.id}" - expect(response).to be_redirect - expect(controller.theme_id).to eq(theme2.id) - end - - it "cookie can fail back to user if out of sync" do - user.user_option.update_columns(theme_ids: [theme.id]) - cookies['theme_ids'] = "#{theme2.id}|#{user.user_option.theme_key_seq - 1}" - - get "/t/#{topic.id}" - expect(response).to be_redirect - expect(controller.theme_id).to eq(theme.id) - end - end - it "doesn't store an incoming link when there's no referer" do expect { get "/t/#{topic.id}.json" diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index c0aa78e5f1..396957bd2f 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1440,7 +1440,7 @@ describe UsersController do notification_level: TagUser.notification_levels[:watching] ).pluck(:tag_id)).to contain_exactly(tags[0].id, tags[1].id) - theme = Theme.create(name: "test", user_selectable: true, user_id: -1) + theme = Fabricate(:theme, user_selectable: true) put "/u/#{user.username}.json", params: { muted_usernames: "", diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb index 56c8a81c36..41ea44c06c 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/spec/services/staff_action_logger_spec.rb @@ -154,7 +154,7 @@ describe StaffActionLogger do end let :theme do - Theme.new(name: 'bob', user_id: -1) + Fabricate(:theme) end it "logs new site customizations" do @@ -188,7 +188,7 @@ describe StaffActionLogger do end it "creates a new UserHistory record" do - theme = Theme.new(name: 'Banana') + theme = Fabricate(:theme) theme.set_field(target: :common, name: :scss, value: "body{margin: 10px;}") log_record = logger.log_theme_destroy(theme) diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index 5b1dd3df76..65799eb179 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -478,7 +478,7 @@ describe UserMerger do end it "updates themes" do - theme = Theme.create!(name: 'my name', user_id: source_user.id) + theme = Fabricate(:theme, user: source_user) merge_users! expect(theme.reload.user_id).to eq(target_user.id) diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index 5e4e549f88..47e37edc0d 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -82,7 +82,7 @@ describe UserUpdater do updater = UserUpdater.new(acting_user, user) date_of_birth = Time.zone.now - theme = Theme.create!(user_id: -1, name: "test", user_selectable: true) + theme = Fabricate(:theme, user_selectable: true) seq = user.user_option.theme_key_seq