From decf1f27cfbc82b673d31c94858ab22d567f0907 Mon Sep 17 00:00:00 2001 From: OsamaSayegh Date: Thu, 12 Jul 2018 07:18:21 +0300 Subject: [PATCH] FEATURE: Groundwork for user-selectable theme components * Phase 0 for user-selectable theme components - Drops `key` column from the `themes` table - Drops `theme_key` column from the `user_options` table - Adds `theme_ids` (array of ints default []) column to the `user_options` table and migrates data from `theme_key` to the new column. - Removes the `default_theme_key` site setting and adds `default_theme_id` instead. - Replaces `theme_key` cookie with a new one called `theme_ids` - no longer need Theme.settings_for_client --- .../controllers/preferences/interface.js.es6 | 20 +++--- .../initializers/live-development.js.es6 | 22 +++---- .../discourse/lib/theme-selector.js.es6 | 37 ++++++----- .../javascripts/discourse/models/user.js.es6 | 2 +- .../templates/preferences/interface.hbs | 2 +- app/controllers/admin/themes_controller.rb | 4 +- app/controllers/application_controller.rb | 41 ++++++------- app/controllers/stylesheets_controller.rb | 2 +- app/controllers/themes_controller.rb | 10 +-- app/controllers/users_controller.rb | 4 +- app/helpers/application_helper.rb | 14 ++--- app/models/color_scheme.rb | 2 +- app/models/theme.rb | 61 ++++++++----------- app/models/theme_field.rb | 4 +- app/models/user_option.rb | 5 +- app/serializers/site_serializer.rb | 8 +-- app/serializers/theme_serializer.rb | 4 +- app/serializers/user_option_serializer.rb | 6 +- app/services/user_updater.rb | 14 +++-- .../common/_discourse_stylesheet.html.erb | 2 +- app/views/layouts/application.html.erb | 2 +- app/views/layouts/crawler.html.erb | 2 +- app/views/layouts/embed.html.erb | 2 +- .../layouts/finish_installation.html.erb | 2 +- app/views/wizard/index.html.erb | 2 +- config/routes.rb | 2 +- config/site_settings.yml | 4 +- db/fixtures/000_delayed_drops.rb | 22 +++++++ ...80706054922_drop_key_column_from_themes.rb | 32 ++++++++++ lib/guardian.rb | 10 ++- lib/middleware/anonymous_cache.rb | 11 ++-- lib/stylesheet/manager.rb | 30 ++++----- lib/stylesheet/watcher.rb | 10 +-- lib/wizard/builder.rb | 2 +- .../middleware/anonymous_cache_spec.rb | 4 +- spec/components/stylesheet/manager_spec.rb | 18 +++--- spec/components/wizard/step_updater_spec.rb | 4 +- spec/models/color_scheme_spec.rb | 4 +- spec/models/site_spec.rb | 10 +-- spec/models/theme_spec.rb | 55 ++++++++--------- spec/requests/admin/themes_controller_spec.rb | 8 +-- spec/requests/stylesheets_controller_spec.rb | 4 +- spec/requests/topics_controller_spec.rb | 18 +++--- spec/requests/users_controller_spec.rb | 4 +- spec/services/user_updater_spec.rb | 4 +- 45 files changed, 289 insertions(+), 241 deletions(-) create mode 100644 db/migrate/20180706054922_drop_key_column_from_themes.rb diff --git a/app/assets/javascripts/discourse/controllers/preferences/interface.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/interface.js.es6 index 8cc0861dea5..29ee8aa1943 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/interface.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/interface.js.es6 @@ -5,7 +5,7 @@ import { observes } from "ember-addons/ember-computed-decorators"; import { - currentThemeKey, + currentThemeId, listThemes, previewTheme, setLocalTheme @@ -35,7 +35,7 @@ export default Ember.Controller.extend(PreferencesTabController, { ]; if (makeDefault) { - attrs.push("theme_key"); + attrs.push("theme_ids"); } return attrs; @@ -50,8 +50,8 @@ export default Ember.Controller.extend(PreferencesTabController, { }, @computed() - themeKey() { - return currentThemeKey(); + themeId() { + return currentThemeId(); }, userSelectableThemes: function() { @@ -63,10 +63,10 @@ export default Ember.Controller.extend(PreferencesTabController, { return themes && themes.length > 1; }, - @observes("themeKey") - themeKeyChanged() { - let key = this.get("themeKey"); - previewTheme(key); + @observes("themeId") + themeIdChanged() { + const id = this.get("themeId"); + previewTheme(id); }, homeChanged() { @@ -95,7 +95,7 @@ export default Ember.Controller.extend(PreferencesTabController, { this.set("saved", false); const makeThemeDefault = this.get("makeThemeDefault"); if (makeThemeDefault) { - this.set("model.user_option.theme_key", this.get("themeKey")); + this.set("model.user_option.theme_ids", [this.get("themeId")]); } return this.get("model") @@ -105,7 +105,7 @@ export default Ember.Controller.extend(PreferencesTabController, { if (!makeThemeDefault) { setLocalTheme( - this.get("themeKey"), + [this.get("themeId")], this.get("model.user_option.theme_key_seq") ); } diff --git a/app/assets/javascripts/discourse/initializers/live-development.js.es6 b/app/assets/javascripts/discourse/initializers/live-development.js.es6 index c3ab4b7d6fb..994e4e36e63 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 { currentThemeKey, refreshCSS } from "discourse/lib/theme-selector"; +import { currentThemeId, refreshCSS } from "discourse/lib/theme-selector"; // Use the message bus for live reloading of components for faster development. export default { @@ -9,18 +9,18 @@ export default { if ( window.history && - window.location.search.indexOf("?preview_theme_key=") === 0 + window.location.search.indexOf("?preview_theme_id=") === 0 ) { - // force preview theme key to always be carried along - const themeKey = window.location.search.slice(19).split("&")[0]; - if (themeKey.match(/^[a-z0-9-]+$/i)) { + // force preview theme id to always be carried along + const themeId = window.location.search.slice(19).split("&")[0]; + if (themeId.match(/^[a-z0-9-]+$/i)) { const patchState = function(f) { const patched = window.history[f]; window.history[f] = function(stateObj, name, url) { - if (url.indexOf("preview_theme_key=") === -1) { + if (url.indexOf("preview_theme_id=") === -1) { const joiner = url.indexOf("?") === -1 ? "?" : "&"; - url = `${url}${joiner}preview_theme_key=${themeKey}`; + url = `${url}${joiner}preview_theme_id=${themeId}`; } return patched.call(window.history, stateObj, name, url); @@ -35,7 +35,7 @@ export default { $("header.custom").each(function() { const header = $(this); return messageBus.subscribe( - "/header-change/" + $(this).data("key"), + "/header-change/" + $(this).data("id"), function(data) { return header.html(data); } @@ -58,12 +58,12 @@ export default { // Refresh if necessary document.location.reload(true); } else { - let themeKey = currentThemeKey(); + let themeId = currentThemeId(); $("link").each(function() { - if (me.hasOwnProperty("theme_key") && me.new_href) { + if (me.hasOwnProperty("theme_id") && me.new_href) { let target = $(this).data("target"); - if (me.theme_key === themeKey && target === me.target) { + if (me.theme_id === themeId && target === me.target) { 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 2634930ffc8..53b8c00d46a 100644 --- a/app/assets/javascripts/discourse/lib/theme-selector.js.es6 +++ b/app/assets/javascripts/discourse/lib/theme-selector.js.es6 @@ -1,23 +1,28 @@ import { ajax } from "discourse/lib/ajax"; -const keySelector = "meta[name=discourse_theme_key]"; +const keySelector = "meta[name=discourse_theme_id]"; -export function currentThemeKey() { - let themeKey = null; +export function currentThemeId() { + let themeId = null; let elem = _.first($(keySelector)); if (elem) { - themeKey = elem.content; - if (_.isEmpty(themeKey)) { - themeKey = null; + themeId = elem.content; + if (_.isEmpty(themeId)) { + themeId = null; + } else { + themeId = parseInt(themeId); } } - return themeKey; + return themeId; } -export function setLocalTheme(key, themeSeq) { - if (key) { - $.cookie("theme_key", `${key},${themeSeq}`, { path: "/", expires: 9999 }); +export function setLocalTheme(ids, themeSeq) { + if (ids && ids.length > 0) { + $.cookie("theme_ids", `${ids.join(",")}|${themeSeq}`, { + path: "/", + expires: 9999 + }); } else { - $.cookie("theme_key", null, { path: "/", expires: 1 }); + $.cookie("theme_ids", null, { path: "/", expires: 1 }); } } @@ -60,14 +65,14 @@ export function refreshCSS(node, hash, newHref, options) { $orig.data("copy", reloaded); } -export function previewTheme(key) { - if (currentThemeKey() !== key) { +export function previewTheme(id) { + if (currentThemeId() !== id) { Discourse.set("assetVersion", "forceRefresh"); - ajax(`/themes/assets/${key ? key : "default"}`).then(results => { + ajax(`/themes/assets/${id ? id : "default"}`).then(results => { let elem = _.first($(keySelector)); if (elem) { - elem.content = key; + elem.content = id; } results.themes.forEach(theme => { @@ -95,7 +100,7 @@ export function listThemes(site) { } themes.forEach(t => { - results.push({ name: t.name, id: t.theme_key }); + results.push({ name: t.name, id: t.theme_id }); }); return results.length === 0 ? null : results; diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 964b37be865..96f078baeb3 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -271,7 +271,7 @@ const User = RestModel.extend({ "notification_level_when_replying", "like_notification_frequency", "include_tl0_in_digests", - "theme_key", + "theme_ids", "allow_private_messages", "homepage_id" ]; diff --git a/app/assets/javascripts/discourse/templates/preferences/interface.hbs b/app/assets/javascripts/discourse/templates/preferences/interface.hbs index 9954bd40092..968737b6b60 100644 --- a/app/assets/javascripts/discourse/templates/preferences/interface.hbs +++ b/app/assets/javascripts/discourse/templates/preferences/interface.hbs @@ -2,7 +2,7 @@
- {{combo-box content=userSelectableThemes value=themeKey}} + {{combo-box content=userSelectableThemes value=themeId}}
{{preference-checkbox labelKey="user.theme_default_on_all_devices" checked=makeThemeDefault}} diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index ebc74980e09..857fcca970c 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -7,7 +7,7 @@ class Admin::ThemesController < Admin::AdminController def preview @theme = Theme.find(params[:id]) - redirect_to path("/?preview_theme_key=#{@theme.key}") + redirect_to path("/?preview_theme_id=#{@theme.id}") end def upload_asset @@ -226,7 +226,7 @@ class Admin::ThemesController < Admin::AdminController def update_default_theme if theme_params.key?(:default) is_default = theme_params[:default].to_s == "true" - if @theme.key == SiteSetting.default_theme_key && !is_default + if @theme.id == SiteSetting.default_theme_id && !is_default Theme.clear_default! elsif is_default @theme.set_default! diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 83f2feb4599..3972fa18c0d 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_key + attr_reader :theme_id serialization_scope :guardian @@ -59,11 +59,11 @@ class ApplicationController < ActionController::Base layout :set_layout if Rails.env == "development" - after_action :remember_theme_key + after_action :remember_theme_id - def remember_theme_key - if @theme_key - Stylesheet::Watcher.theme_key = @theme_key if defined? Stylesheet::Watcher + def remember_theme_id + if @theme_id + Stylesheet::Watcher.theme_id = @theme_id if defined? Stylesheet::Watcher end end end @@ -325,34 +325,34 @@ class ApplicationController < ActionController::Base end def handle_theme - return if request.xhr? || request.format == "json" || request.format == "js" return if request.method != "GET" resolve_safe_mode return if request.env[NO_CUSTOM] - theme_key = request[:preview_theme_key] + theme_id = request[:preview_theme_id]&.to_i user_option = current_user&.user_option - unless theme_key - key, seq = cookies[:theme_key]&.split(",") - if key && seq && seq.to_i == user_option&.theme_key_seq.to_i - theme_key = key + unless theme_id + 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 end end - theme_key ||= user_option&.theme_key + theme_id ||= user_option&.theme_ids&.first - if theme_key && !guardian.allow_theme?(theme_key) - theme_key = nil + if theme_id && !guardian.allow_themes?(theme_id) + theme_id = nil end - theme_key ||= SiteSetting.default_theme_key - theme_key = nil if theme_key.blank? + theme_id ||= SiteSetting.default_theme_id + theme_id = nil if theme_id.blank? || theme_id == -1 - @theme_key = request.env[:resolved_theme_key] = theme_key + @theme_id = request.env[:resolved_theme_id] = theme_id end def guardian @@ -485,7 +485,6 @@ class ApplicationController < ActionController::Base def preload_anonymous_data store_preloaded("site", Site.json_for(guardian)) store_preloaded("siteSettings", SiteSetting.client_settings_json) - store_preloaded("themeSettings", Theme.settings_for_client(@theme_key)) store_preloaded("customHTML", custom_html_json) store_preloaded("banner", banner_json) store_preloaded("customEmoji", custom_emoji) @@ -503,10 +502,10 @@ class ApplicationController < ActionController::Base target = view_context.mobile_view? ? :mobile : :desktop data = - if @theme_key + if @theme_id { - top: Theme.lookup_field(@theme_key, target, "after_header"), - footer: Theme.lookup_field(@theme_key, target, "footer") + top: Theme.lookup_field(@theme_id, target, "after_header"), + footer: Theme.lookup_field(@theme_id, target, "footer") } else {} diff --git a/app/controllers/stylesheets_controller.rb b/app/controllers/stylesheets_controller.rb index f7344a33930..6b0fe317cf0 100644 --- a/app/controllers/stylesheets_controller.rb +++ b/app/controllers/stylesheets_controller.rb @@ -34,7 +34,7 @@ class StylesheetsController < ApplicationController split_target, color_scheme_id = target.split(/_(-?[0-9]+)/) theme = Theme.find_by(color_scheme_id: color_scheme_id) end - Stylesheet::Manager.stylesheet_link_tag(split_target, nil, theme&.key) + Stylesheet::Manager.stylesheet_link_tag(split_target, nil, theme&.id) end cache_time = request.env["HTTP_IF_MODIFIED_SINCE"] diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb index a5f26c5cc4d..22c29251077 100644 --- a/app/controllers/themes_controller.rb +++ b/app/controllers/themes_controller.rb @@ -1,15 +1,15 @@ class ThemesController < ::ApplicationController def assets - theme_key = params[:key].to_s + theme_id = params[:id].to_i - if theme_key == "default" - theme_key = nil + if params[:id] == "default" + theme_id = nil else - raise Discourse::NotFound unless Theme.where(key: theme_key).exists? + raise Discourse::NotFound unless Theme.where(id: theme_id).exists? end object = [:mobile, :desktop, :desktop_theme, :mobile_theme].map do |target| - link = Stylesheet::Manager.stylesheet_link_tag(target, 'all', params[:key]) + link = Stylesheet::Manager.stylesheet_link_tag(target, 'all', params[:id]) if link href = link.split(/["']/)[1] if Rails.env.development? diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index cf047ce31ee..187d859e793 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1072,7 +1072,7 @@ class UsersController < ApplicationController :title, :date_of_birth, :muted_usernames, - :theme_key, + :theme_ids, :locale, :bio_raw, :location, @@ -1087,7 +1087,7 @@ class UsersController < ApplicationController permitted.concat UserUpdater::TAG_NAMES.keys result = params - .permit(permitted) + .permit(permitted, theme_ids: []) .reverse_merge( ip_address: request.remote_ip, registration_ip_address: request.remote_ip, diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index b461dfdafcc..098d540a0de 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -346,11 +346,11 @@ module ApplicationHelper end end - def theme_key + def theme_id if customization_disabled? nil else - request.env[:resolved_theme_key] + request.env[:resolved_theme_id] end end @@ -374,17 +374,17 @@ module ApplicationHelper end def theme_lookup(name) - lookup = Theme.lookup_field(theme_key, mobile_view? ? :mobile : :desktop, name) + lookup = Theme.lookup_field(theme_id, mobile_view? ? :mobile : :desktop, name) lookup.html_safe if lookup end def discourse_stylesheet_link_tag(name, opts = {}) - if opts.key?(:theme_key) - key = opts[:theme_key] unless customization_disabled? + if opts.key?(:theme_id) + id = opts[:theme_id] unless customization_disabled? else - key = theme_key + id = theme_id end - Stylesheet::Manager.stylesheet_link_tag(name, 'all', key) + Stylesheet::Manager.stylesheet_link_tag(name, 'all', id) end end diff --git a/app/models/color_scheme.rb b/app/models/color_scheme.rb index 8b55f896eb6..d03b865c7ea 100644 --- a/app/models/color_scheme.rb +++ b/app/models/color_scheme.rb @@ -109,7 +109,7 @@ class ColorScheme < ActiveRecord::Base end def self.lookup_hex_for_name(name) - enabled_color_scheme = Theme.where(key: SiteSetting.default_theme_key).first&.color_scheme + enabled_color_scheme = Theme.where(id: SiteSetting.default_theme_id).first&.color_scheme (enabled_color_scheme || base).colors.find { |c| c.name == name }.try(:hex) || "nil" end diff --git a/app/models/theme.rb b/app/models/theme.rb index 5dfcd54b3e9..dcc99f3ce34 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -6,6 +6,9 @@ require_dependency 'theme_settings_manager' class Theme < ActiveRecord::Base + # TODO: remove in 2019 + self.ignored_columns = ["key"] + @cache = DistributedCache.new('theme') belongs_to :user @@ -17,11 +20,6 @@ class Theme < ActiveRecord::Base has_many :color_schemes belongs_to :remote_theme - before_create do - self.key ||= SecureRandom.uuid - true - end - def notify_color_change(color) changed_colors << color end @@ -53,7 +51,7 @@ class Theme < ActiveRecord::Base after_destroy do remove_from_cache! clear_cached_settings! - if SiteSetting.default_theme_key == self.key + if SiteSetting.default_theme_id == self.id Theme.clear_default! end @@ -75,21 +73,21 @@ class Theme < ActiveRecord::Base theme.notify_theme_change end, on: :update - def self.theme_keys - if keys = @cache["theme_keys"] - return keys + def self.theme_ids + if ids = @cache["theme_ids"] + return ids end - @cache["theme_keys"] = Set.new(Theme.pluck(:key)) + @cache["theme_ids"] = Set.new(Theme.pluck(:id)) end - def self.user_theme_keys - if keys = @cache["user_theme_keys"] - return keys + def self.user_theme_ids + if ids = @cache["user_theme_ids"] + return ids end - @cache["user_theme_keys"] = Set.new( + @cache["user_theme_ids"] = Set.new( Theme - .where('user_selectable OR key = ?', SiteSetting.default_theme_key) - .pluck(:key) + .where('user_selectable OR id = ?', SiteSetting.default_theme_id) + .pluck(:id) ) end @@ -99,28 +97,28 @@ class Theme < ActiveRecord::Base end def self.clear_default! - SiteSetting.default_theme_key = "" + SiteSetting.default_theme_id = -1 expire_site_cache! end def set_default! - SiteSetting.default_theme_key = key + SiteSetting.default_theme_id = id Theme.expire_site_cache! end def default? - SiteSetting.default_theme_key == key + SiteSetting.default_theme_id == id end - def self.lookup_field(key, target, field) - return if key.blank? + def self.lookup_field(theme_id, target, field) + return if theme_id.blank? - cache_key = "#{key}:#{target}:#{field}:#{ThemeField::COMPILER_VERSION}" + cache_key = "#{theme_id}:#{target}:#{field}:#{ThemeField::COMPILER_VERSION}" lookup = @cache[cache_key] return lookup.html_safe if lookup target = target.to_sym - theme = find_by(key: key) + theme = find_by(id: theme_id) val = theme.resolve_baked_field(target, field) if theme @@ -162,12 +160,12 @@ class Theme < ActiveRecord::Base def refresh_message_for_targets(targets, theme) targets.map do |target| - href = Stylesheet::Manager.stylesheet_href(target.to_sym, theme.key) + href = Stylesheet::Manager.stylesheet_href(target.to_sym, theme.id) if href { target: target, new_href: href, - theme_key: theme.key + theme_id: theme.id } end end @@ -319,7 +317,7 @@ class Theme < ActiveRecord::Base end def cached_settings - Rails.cache.fetch("settings_for_theme_#{self.key}", expires_in: 30.minutes) do + Rails.cache.fetch("settings_for_theme_#{self.id}", expires_in: 30.minutes) do hash = {} self.settings.each do |setting| hash[setting.name] = setting.value @@ -329,7 +327,7 @@ class Theme < ActiveRecord::Base end def clear_cached_settings! - Rails.cache.delete("settings_for_theme_#{self.key}") + Rails.cache.delete("settings_for_theme_#{self.id}") end def included_settings @@ -343,13 +341,6 @@ class Theme < ActiveRecord::Base hash end - def self.settings_for_client(key) - theme = Theme.find_by(key: key) - return {}.to_json unless theme - - theme.included_settings.to_json - end - def update_setting(setting_name, new_value) target_setting = settings.find { |setting| setting.name == setting_name } raise Discourse::NotFound unless target_setting @@ -365,7 +356,6 @@ end # id :integer not null, primary key # name :string not null # user_id :integer not null -# key :string not null # created_at :datetime not null # updated_at :datetime not null # compiler_version :integer default(0), not null @@ -376,6 +366,5 @@ end # # Indexes # -# index_themes_on_key (key) # index_themes_on_remote_theme_id (remote_theme_id) UNIQUE # diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index eb87ce5f14e..da1ab9d2a29 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -221,8 +221,8 @@ COMPILED Stylesheet::Manager.clear_theme_cache! if self.name.include?("scss") # TODO message for mobile vs desktop - MessageBus.publish "/header-change/#{theme.key}", self.value if theme && self.name == "header" - MessageBus.publish "/footer-change/#{theme.key}", self.value if theme && self.name == "footer" + MessageBus.publish "/header-change/#{theme.id}", self.value if theme && self.name == "header" + MessageBus.publish "/footer-change/#{theme.id}", self.value if theme && self.name == "footer" end end diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 95c34d51f6a..a447a8eae96 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -1,4 +1,7 @@ class UserOption < ActiveRecord::Base + # TODO: remove in 2019 + self.ignored_columns = ["theme_key"] + self.primary_key = :user_id belongs_to :user before_create :set_defaults @@ -177,10 +180,10 @@ end # mailing_list_mode_frequency :integer default(1), not null # include_tl0_in_digests :boolean default(FALSE) # notification_level_when_replying :integer -# theme_key :string # theme_key_seq :integer default(0), not null # allow_private_messages :boolean default(TRUE), not null # homepage_id :integer +# theme_ids :integer default([]), not null, is an Array # # Indexes # diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index bd52b8d2e2d..467542ab361 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -39,11 +39,11 @@ class SiteSerializer < ApplicationSerializer def user_themes cache_fragment("user_themes") do - Theme.where('key = :default OR user_selectable', - default: SiteSetting.default_theme_key) + Theme.where('id = :default OR user_selectable', + default: SiteSetting.default_theme_id) .order(:name) - .pluck(:key, :name) - .map { |k, n| { theme_key: k, name: n, default: k == SiteSetting.default_theme_key } } + .pluck(:id, :name) + .map { |id, n| { theme_id: id, name: n, default: id == SiteSetting.default_theme_id } } .as_json end end diff --git a/app/serializers/theme_serializer.rb b/app/serializers/theme_serializer.rb index 4938b5fac99..47e0e1222bb 100644 --- a/app/serializers/theme_serializer.rb +++ b/app/serializers/theme_serializer.rb @@ -33,10 +33,10 @@ class ThemeFieldSerializer < ApplicationSerializer end class ChildThemeSerializer < ApplicationSerializer - attributes :id, :name, :key, :created_at, :updated_at, :default + attributes :id, :name, :created_at, :updated_at, :default def include_default? - object.key == SiteSetting.default_theme_key + object.id == SiteSetting.default_theme_id end def default diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb index 6bccbe81766..23b86fddd2a 100644 --- a/app/serializers/user_option_serializer.rb +++ b/app/serializers/user_option_serializer.rb @@ -19,7 +19,7 @@ class UserOptionSerializer < ApplicationSerializer :email_in_reply_to, :like_notification_frequency, :include_tl0_in_digests, - :theme_key, + :theme_ids, :theme_key_seq, :allow_private_messages, :homepage_id, @@ -36,8 +36,8 @@ class UserOptionSerializer < ApplicationSerializer object.new_topic_duration_minutes || SiteSetting.default_other_new_topic_duration_minutes end - def theme_key - object.theme_key || SiteSetting.default_theme_key + def theme_ids + object.theme_ids.presence || [SiteSetting.default_theme_id] end end diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index eb9011cc21f..a580e968da6 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -34,7 +34,7 @@ class UserUpdater :email_in_reply_to, :like_notification_frequency, :include_tl0_in_digests, - :theme_key, + :theme_ids, :allow_private_messages, :homepage_id, ] @@ -85,9 +85,15 @@ class UserUpdater save_options = false - # special handling for theme_key cause we need to bump a sequence number - if attributes.key?(:theme_key) && user.user_option.theme_key != attributes[:theme_key] - user.user_option.theme_key_seq += 1 + # special handling for theme_id cause we need to bump a sequence number + if attributes.key?(:theme_ids) + user_guardian = Guardian.new(user) + attributes[:theme_ids].map!(&:to_i) + if user_guardian.allow_themes?(attributes[:theme_ids]) + user.user_option.theme_key_seq += 1 if user.user_option.theme_ids != attributes[:theme_ids] + else + attributes.delete(:theme_ids) + end end OPTION_ATTR.each do |attribute| diff --git a/app/views/common/_discourse_stylesheet.html.erb b/app/views/common/_discourse_stylesheet.html.erb index 044ce2fc767..5e66ec2ab2f 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_key %> +<%- if theme_id %> <%= 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 60d30d0fd49..934695067b0 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 84195992f50..9e2c662f800 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_key %> + <%- if theme_id %> <%= 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 a0de03682a9..b1faf885ed1 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_key: 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 abf274f54fa..f573616b19f 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_key: nil %> + <%= discourse_stylesheet_link_tag 'wizard', theme_id: 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 b090b4539cc..fb4f420bd1f 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_key: nil %> + <%= discourse_stylesheet_link_tag :wizard, theme_id: nil %> <%= preload_script 'ember_jquery' %> <%= preload_script 'wizard-vendor' %> <%= preload_script 'wizard-application' %> diff --git a/config/routes.rb b/config/routes.rb index 2bee8f0601d..bf5199f450f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -803,7 +803,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/:key" => "themes#assets" + get "/themes/assets/:id" => "themes#assets" if Rails.env == "test" || Rails.env == "development" get "/qunit" => "qunit#index" diff --git a/config/site_settings.yml b/config/site_settings.yml index f51e6998c08..27ea51c6dc9 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -192,8 +192,8 @@ basic: enable_mobile_theme: client: true default: true - default_theme_key: - default: '' + default_theme_id: + default: -1 hidden: true relative_date_duration: client: true diff --git a/db/fixtures/000_delayed_drops.rb b/db/fixtures/000_delayed_drops.rb index b5d2ab446ea..2cc74cc605d 100644 --- a/db/fixtures/000_delayed_drops.rb +++ b/db/fixtures/000_delayed_drops.rb @@ -217,4 +217,26 @@ Migration::TableDropper.delayed_drop( } ) +Migration::ColumnDropper.drop( + table: 'user_options', + after_migration: 'DropKeyColumnFromThemes', + columns: %w[ + theme_key + ], + on_drop: ->() { + STDERR.puts 'Removing theme_key column from user_options table!' + } +) + +Migration::ColumnDropper.drop( + table: 'themes', + after_migration: 'DropKeyColumnFromThemes', + columns: %w[ + key + ], + on_drop: ->() { + STDERR.puts 'Removing key column from themes table!' + } +) + Discourse.reset_active_record_cache diff --git a/db/migrate/20180706054922_drop_key_column_from_themes.rb b/db/migrate/20180706054922_drop_key_column_from_themes.rb new file mode 100644 index 00000000000..60295200099 --- /dev/null +++ b/db/migrate/20180706054922_drop_key_column_from_themes.rb @@ -0,0 +1,32 @@ +class DropKeyColumnFromThemes < ActiveRecord::Migration[5.2] + def up + add_column :user_options, :theme_ids, :integer, array: true, null: false, default: [] + + execute( + "UPDATE user_options AS uo + SET theme_ids = ( + SELECT array_agg(themes.id) + FROM themes + INNER JOIN user_options + ON themes.key = user_options.theme_key + WHERE user_options.user_id = uo.user_id + ) WHERE uo.theme_key IN (SELECT key FROM themes)" + ) + + execute( + "INSERT INTO site_settings (name, data_type, value, created_at, updated_at) + SELECT 'default_theme_id', 3, id, now(), now() + FROM themes + WHERE key = (SELECT value FROM site_settings WHERE name = 'default_theme_key')" + ) + + execute("DELETE FROM site_settings WHERE name = 'default_theme_key'") + + # delayed drop for theme_key on user_options table + # delayed drop for key on themes table + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/guardian.rb b/lib/guardian.rb index 528a190ff79..9c70f4b9018 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -358,12 +358,10 @@ class Guardian UserExport.where(user_id: @user.id, created_at: (Time.zone.now.beginning_of_day..Time.zone.now.end_of_day)).count == 0 end - def allow_theme?(theme_key) - if is_staff? - Theme.theme_keys.include?(theme_key) - else - Theme.user_theme_keys.include?(theme_key) - 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? end private diff --git a/lib/middleware/anonymous_cache.rb b/lib/middleware/anonymous_cache.rb index 9e29f06854c..4cd9cd53586 100644 --- a/lib/middleware/anonymous_cache.rb +++ b/lib/middleware/anonymous_cache.rb @@ -65,13 +65,14 @@ 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_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}" end - def theme_key - key, _ = @request.cookies['theme_key']&.split(',') - if key && Guardian.new.allow_theme?(key) - key + def theme_id + ids, _ = @request.cookies['theme_ids']&.split('|') + ids = ids&.split(",")&.map(&:to_i) + if ids && Guardian.new.allow_themes?(ids) + ids.first else nil end diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index fb7d5e18ecd..a70ccae052b 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -19,29 +19,29 @@ class Stylesheet::Manager cache.hash.keys.select { |k| k =~ /theme/ }.each { |k|cache.delete(k) } end - def self.stylesheet_href(target = :desktop, theme_key = :missing) - href = stylesheet_link_tag(target, 'all', theme_key) + def self.stylesheet_href(target = :desktop, theme_id = :missing) + href = stylesheet_link_tag(target, 'all', theme_id) if href href.split(/["']/)[1] end end - def self.stylesheet_link_tag(target = :desktop, media = 'all', theme_key = :missing) + def self.stylesheet_link_tag(target = :desktop, media = 'all', theme_id = :missing) target = target.to_sym - if theme_key == :missing - theme_key = SiteSetting.default_theme_key + if theme_id == :missing + theme_id = SiteSetting.default_theme_id end current_hostname = Discourse.current_hostname - cache_key = "#{target}_#{theme_key}_#{current_hostname}" + cache_key = "#{target}_#{theme_id}_#{current_hostname}" tag = cache[cache_key] return tag.dup.html_safe if tag @lock.synchronize do - builder = self.new(target, theme_key) + builder = self.new(target, theme_id) if builder.is_theme? && !builder.theme tag = "" else @@ -55,15 +55,15 @@ class Stylesheet::Manager end def self.precompile_css - themes = Theme.where('user_selectable OR key = ?', SiteSetting.default_theme_key).pluck(:key, :name) + themes = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:id, :name) themes << nil - themes.each do |key, name| + themes.each do |id, name| [:desktop, :mobile, :desktop_rtl, :mobile_rtl].each do |target| - theme_key = key || SiteSetting.default_theme_key - cache_key = "#{target}_#{theme_key}" + theme_id = id || SiteSetting.default_theme_id + cache_key = "#{target}_#{theme_id}" STDERR.puts "precompile target: #{target} #{name}" - builder = self.new(target, theme_key) + builder = self.new(target, theme_id) builder.compile(force: true) cache[cache_key] = nil end @@ -100,9 +100,9 @@ class Stylesheet::Manager end.compact.max.to_i end - def initialize(target = :desktop, theme_key) + def initialize(target = :desktop, theme_id) @target = target - @theme_key = theme_key + @theme_id = theme_id end def compile(opts = {}) @@ -240,7 +240,7 @@ class Stylesheet::Manager end def theme - @theme ||= (Theme.find_by(key: @theme_key) || :nil) + @theme ||= (Theme.find_by(id: @theme_id) || :nil) @theme == :nil ? nil : @theme end diff --git a/lib/stylesheet/watcher.rb b/lib/stylesheet/watcher.rb index a1150e734f9..91b13343d81 100644 --- a/lib/stylesheet/watcher.rb +++ b/lib/stylesheet/watcher.rb @@ -3,12 +3,12 @@ require 'listen' module Stylesheet class Watcher - def self.theme_key=(v) - @theme_key = v + def self.theme_id=(v) + @theme_id = v end - def self.theme_key - @theme_key || SiteSetting.default_theme_key + def self.theme_id + @theme_id || SiteSetting.default_theme_id end def self.watch(paths = nil) @@ -79,7 +79,7 @@ module Stylesheet { target: name, new_href: Stylesheet::Manager.stylesheet_href(name.to_sym), - theme_key: Stylesheet::Watcher.theme_key + theme_id: Stylesheet::Watcher.theme_id } end MessageBus.publish '/file-change', message diff --git a/lib/wizard/builder.rb b/lib/wizard/builder.rb index f4d3ca66c7e..eb5f7d04b1e 100644 --- a/lib/wizard/builder.rb +++ b/lib/wizard/builder.rb @@ -117,7 +117,7 @@ class Wizard end @wizard.append_step('colors') do |step| - default_theme = Theme.find_by(key: SiteSetting.default_theme_key) + default_theme = Theme.find_by(id: SiteSetting.default_theme_id) scheme_id = default_theme&.color_scheme&.base_scheme_id || 'default' themes = step.add_field(id: 'base_scheme_id', type: 'dropdown', required: true, value: scheme_id) diff --git a/spec/components/middleware/anonymous_cache_spec.rb b/spec/components/middleware/anonymous_cache_spec.rb index 0ee3bc46b77..6c153115ab3 100644 --- a/spec/components/middleware/anonymous_cache_spec.rb +++ b/spec/components/middleware/anonymous_cache_spec.rb @@ -34,12 +34,12 @@ describe Middleware::AnonymousCache::Helper do it "handles theme keys" do theme = Theme.create(name: "test", user_id: -1, user_selectable: true) - with_bad_theme_key = new_helper("HTTP_COOKIE" => "theme_key=abc").cache_key + with_bad_theme_key = new_helper("HTTP_COOKIE" => "theme_ids=abc").cache_key with_no_theme_key = new_helper().cache_key expect(with_bad_theme_key).to eq(with_no_theme_key) - with_good_theme_key = new_helper("HTTP_COOKIE" => "theme_key=#{theme.key}").cache_key + with_good_theme_key = new_helper("HTTP_COOKIE" => "theme_ids=#{theme.id}").cache_key expect(with_good_theme_key).not_to eq(with_no_theme_key) end diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index b534133b9e2..1d4c19fd90f 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -9,7 +9,7 @@ describe Stylesheet::Manager do expect(link).to eq("") theme = Theme.create(name: "embedded", user_id: -1) - SiteSetting.default_theme_key = theme.key + SiteSetting.default_theme_id = theme.id link = Stylesheet::Manager.stylesheet_link_tag(:embedded_theme) expect(link).not_to eq("") @@ -41,9 +41,9 @@ describe Stylesheet::Manager do theme.add_child_theme!(child_theme) - old_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.key) + old_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id) - manager = Stylesheet::Manager.new(:desktop_theme, theme.key) + manager = Stylesheet::Manager.new(:desktop_theme, theme.id) manager.compile(force: true) css = File.read(manager.stylesheet_fullpath) @@ -57,7 +57,7 @@ describe Stylesheet::Manager do child_theme.set_field(target: :desktop, name: :scss, value: ".nothing{color: green;}") child_theme.save! - new_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.key) + new_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id) expect(new_link).not_to eq(old_link) @@ -77,12 +77,12 @@ describe Stylesheet::Manager do user_id: -1 ) - manager = Stylesheet::Manager.new(:desktop_theme, theme.key) + manager = Stylesheet::Manager.new(:desktop_theme, theme.id) digest1 = manager.digest DiscoursePluginRegistry.stylesheets.add "fake_file" - manager = Stylesheet::Manager.new(:desktop_theme, theme.key) + manager = Stylesheet::Manager.new(:desktop_theme, theme.id) digest2 = manager.digest expect(digest1).not_to eq(digest2) @@ -107,7 +107,7 @@ describe Stylesheet::Manager do type_id: ThemeField.types[:theme_upload_var] ) - manager = Stylesheet::Manager.new(:desktop_theme, theme.key) + manager = Stylesheet::Manager.new(:desktop_theme, theme.id) digest1 = manager.digest field.destroy! @@ -121,7 +121,7 @@ describe Stylesheet::Manager do type_id: ThemeField.types[:theme_upload_var] ) - manager = Stylesheet::Manager.new(:desktop_theme, theme.key) + manager = Stylesheet::Manager.new(:desktop_theme, theme.id) digest2 = manager.digest expect(digest1).not_to eq(digest2) @@ -137,7 +137,7 @@ describe Stylesheet::Manager do category1 = Fabricate(:category, uploaded_background_id: 123, updated_at: 1.week.ago) category2 = Fabricate(:category, uploaded_background_id: 456, updated_at: 2.days.ago) - manager = Stylesheet::Manager.new(:desktop_theme, theme.key) + manager = Stylesheet::Manager.new(:desktop_theme, theme.id) digest1 = manager.color_scheme_digest diff --git a/spec/components/wizard/step_updater_spec.rb b/spec/components/wizard/step_updater_spec.rb index b57d3000db3..d10c2120e9e 100644 --- a/spec/components/wizard/step_updater_spec.rb +++ b/spec/components/wizard/step_updater_spec.rb @@ -155,7 +155,7 @@ describe Wizard::StepUpdater do updater.update expect(updater.success?).to eq(true) expect(wizard.completed_steps?('colors')).to eq(true) - theme = Theme.find_by(key: SiteSetting.default_theme_key) + theme = Theme.find_by(id: SiteSetting.default_theme_id) expect(theme.color_scheme.base_scheme_id).to eq('dark') end end @@ -203,7 +203,7 @@ describe Wizard::StepUpdater do expect(color_scheme).to be_present expect(color_scheme.colors).to be_present - theme = Theme.find_by(key: SiteSetting.default_theme_key) + theme = Theme.find_by(id: SiteSetting.default_theme_id) expect(theme.color_scheme_id).to eq(color_scheme.id) expect(Theme.where(user_selectable: true).count).to eq(2) diff --git a/spec/models/color_scheme_spec.rb b/spec/models/color_scheme_spec.rb index 270c848a393..25121d4997a 100644 --- a/spec/models/color_scheme_spec.rb +++ b/spec/models/color_scheme_spec.rb @@ -14,11 +14,11 @@ describe ColorScheme do theme.set_field(name: :scss, target: :desktop, value: '.bob {color: $primary;}') theme.save! - href = Stylesheet::Manager.stylesheet_href(:desktop_theme, theme.key) + href = Stylesheet::Manager.stylesheet_href(:desktop_theme, theme.id) ColorSchemeRevisor.revise(scheme, colors: [{ name: 'primary', hex: 'bbb' }]) - href2 = Stylesheet::Manager.stylesheet_href(:desktop_theme, theme.key) + href2 = Stylesheet::Manager.stylesheet_href(:desktop_theme, theme.id) expect(href).not_to eq(href2) end diff --git a/spec/models/site_spec.rb b/spec/models/site_spec.rb index 2463fddd796..6a08586b69e 100644 --- a/spec/models/site_spec.rb +++ b/spec/models/site_spec.rb @@ -7,18 +7,18 @@ describe Site do json = Site.json_for(guardian) parsed = JSON.parse(json) - expected = Theme.where('key = :default OR user_selectable', - default: SiteSetting.default_theme_key) + expected = Theme.where('id = :default OR user_selectable', + default: SiteSetting.default_theme_id) .order(:name) - .pluck(:key, :name) - .map { |k, n| { "theme_key" => k, "name" => n, "default" => k == SiteSetting.default_theme_key } } + .pluck(:id, :name) + .map { |id, n| { "theme_id" => id, "name" => n, "default" => id == SiteSetting.default_theme_id } } expect(parsed["user_themes"]).to eq(expected) end it "includes user themes and expires them as needed" do default_theme = Theme.create!(user_id: -1, name: 'default') - SiteSetting.default_theme_key = default_theme.key + SiteSetting.default_theme_id = default_theme.id user_theme = Theme.create!(user_id: -1, name: 'user theme', user_selectable: true) anon_guardian = Guardian.new diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 352147e3680..5802de76120 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -22,11 +22,6 @@ describe Theme do Theme.create!(customization_params) end - it 'should set default key when creating a new customization' do - s = Theme.create!(name: 'my name', user_id: user.id) - expect(s.key).not_to eq(nil) - end - 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') @@ -51,13 +46,13 @@ describe Theme do child.save! - expect(Theme.lookup_field(child.key, :desktop, "header")).to eq("World\nDesktop") - expect(Theme.lookup_field(child.key, "mobile", :header)).to eq("World\nMobile") + expect(Theme.lookup_field(child.id, :desktop, "header")).to eq("World\nDesktop") + expect(Theme.lookup_field(child.id, "mobile", :header)).to eq("World\nMobile") child.set_field(target: :common, name: "header", value: "Worldie") child.save! - expect(Theme.lookup_field(child.key, :mobile, :header)).to eq("Worldie\nMobile") + expect(Theme.lookup_field(child.id, :mobile, :header)).to eq("Worldie\nMobile") parent = Theme.new(name: '1', user_id: user.id) @@ -68,7 +63,7 @@ describe Theme do parent.add_child_theme!(child) - expect(Theme.lookup_field(parent.key, :mobile, "header")).to eq("Common Parent\nMobile Parent\nWorldie\nMobile") + expect(Theme.lookup_field(parent.id, :mobile, "header")).to eq("Common Parent\nMobile Parent\nWorldie\nMobile") end @@ -88,7 +83,7 @@ describe Theme do theme.set_field(target: :common, name: "head_tag", value: "I am bold") theme.save! - expect(Theme.lookup_field(theme.key, :desktop, "head_tag")).to eq("I am bold") + expect(Theme.lookup_field(theme.id, :desktop, "head_tag")).to eq("I am bold") end it 'should precompile fragments in body and head tags' do @@ -104,7 +99,7 @@ HTML theme.set_field(target: :common, name: "header", value: with_template) theme.save! - baked = Theme.lookup_field(theme.key, :mobile, "header") + baked = Theme.lookup_field(theme.id, :mobile, "header") expect(baked).to match(/HTMLBars/) expect(baked).to match(/raw-handlebars/) @@ -118,7 +113,7 @@ HTML ThemeField.update_all(value_baked: nil) - expect(Theme.lookup_field(theme.key, :desktop, :body_tag)).to match(/test<\/b>/) + expect(Theme.lookup_field(theme.id, :desktop, :body_tag)).to match(/test<\/b>/) end context "plugin api" do @@ -244,7 +239,7 @@ HTML }); HTML - expect(Theme.lookup_field(theme.key, :desktop, :after_header)).to eq(transpiled.strip) + expect(Theme.lookup_field(theme.id, :desktop, :after_header)).to eq(transpiled.strip) setting = theme.settings.find { |s| s.name == :name } setting.value = 'bill' @@ -255,35 +250,35 @@ HTML alert(settings.name);var a = function a() {}; }); HTML - expect(Theme.lookup_field(theme.key, :desktop, :after_header)).to eq(transpiled.strip) + expect(Theme.lookup_field(theme.id, :desktop, :after_header)).to eq(transpiled.strip) end end - it 'correctly caches theme keys' do + it 'correctly caches theme ids' do Theme.destroy_all theme = Theme.create!(name: "bob", user_id: -1) - expect(Theme.theme_keys).to eq(Set.new([theme.key])) - expect(Theme.user_theme_keys).to eq(Set.new([])) + expect(Theme.theme_ids).to eq(Set.new([theme.id])) + expect(Theme.user_theme_ids).to eq(Set.new([])) theme.user_selectable = true theme.save - expect(Theme.user_theme_keys).to eq(Set.new([theme.key])) + expect(Theme.user_theme_ids).to eq(Set.new([theme.id])) theme.user_selectable = false theme.save theme.set_default! - expect(Theme.user_theme_keys).to eq(Set.new([theme.key])) + expect(Theme.user_theme_ids).to eq(Set.new([theme.id])) theme.destroy - expect(Theme.theme_keys).to eq(Set.new([])) - expect(Theme.user_theme_keys).to eq(Set.new([])) + expect(Theme.theme_ids).to eq(Set.new([])) + expect(Theme.user_theme_ids).to eq(Set.new([])) end it 'correctly caches user_themes template' do @@ -314,24 +309,22 @@ HTML expect(user_themes).to eq([]) end - def cached_settings(key) - Theme.settings_for_client(key) # returns json + def cached_settings(id) + Theme.find_by(id: id).included_settings.to_json end it 'handles settings cache correctly' do Theme.destroy_all - expect(cached_settings(nil)).to eq("{}") theme = Theme.create!(name: "awesome theme", user_id: -1) - theme.save! - expect(cached_settings(theme.key)).to eq("{}") + expect(cached_settings(theme.id)).to eq("{}") theme.set_field(target: :settings, name: "yaml", value: "boolean_setting: true") theme.save! - expect(cached_settings(theme.key)).to match(/\"boolean_setting\":true/) + expect(cached_settings(theme.id)).to match(/\"boolean_setting\":true/) theme.settings.first.value = "false" - expect(cached_settings(theme.key)).to match(/\"boolean_setting\":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") @@ -339,14 +332,14 @@ HTML child.save! theme.add_child_theme!(child) - json = cached_settings(theme.key) + json = cached_settings(theme.id) expect(json).to match(/\"boolean_setting\":false/) expect(json).to match(/\"integer_setting\":54/) - expect(cached_settings(child.key)).to eq("{\"integer_setting\":54}") + expect(cached_settings(child.id)).to eq("{\"integer_setting\":54}") child.destroy! - json = cached_settings(theme.key) + json = cached_settings(theme.id) expect(json).not_to match(/\"integer_setting\":54/) expect(json).to match(/\"boolean_setting\":false/) end diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index e0416191afa..fc9e8cd5d28 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -144,25 +144,25 @@ describe Admin::ThemesController do let(:theme) { Theme.create(name: 'my name', user_id: -1) } it 'can change default theme' do - SiteSetting.default_theme_key = nil + SiteSetting.default_theme_id = -1 put "/admin/themes/#{theme.id}.json", params: { id: theme.id, theme: { default: true } } expect(response.status).to eq(200) - expect(SiteSetting.default_theme_key).to eq(theme.key) + expect(SiteSetting.default_theme_id).to eq(theme.id) end it 'can unset default theme' do - SiteSetting.default_theme_key = theme.key + SiteSetting.default_theme_id = theme.id put "/admin/themes/#{theme.id}.json", params: { theme: { default: false } } expect(response.status).to eq(200) - expect(SiteSetting.default_theme_key).to be_blank + expect(SiteSetting.default_theme_id).to eq(-1) end it 'updates a theme' do diff --git a/spec/requests/stylesheets_controller_spec.rb b/spec/requests/stylesheets_controller_spec.rb index d33961fa75d..3ccecc7ab1a 100644 --- a/spec/requests/stylesheets_controller_spec.rb +++ b/spec/requests/stylesheets_controller_spec.rb @@ -29,7 +29,7 @@ describe StylesheetsController do scheme = ColorScheme.create_from_base(name: "testing", colors: []) theme = Theme.create!(name: "test", color_scheme_id: scheme.id, user_id: -1) - builder = Stylesheet::Manager.new(:desktop, theme.key) + builder = Stylesheet::Manager.new(:desktop, theme.id) builder.compile `rm #{Stylesheet::Manager.cache_fullpath}/*` @@ -42,7 +42,7 @@ describe StylesheetsController do expect(response.status).to eq(200) - builder = Stylesheet::Manager.new(:desktop_theme, theme.key) + builder = Stylesheet::Manager.new(:desktop_theme, theme.id) builder.compile `rm #{Stylesheet::Manager.cache_fullpath}/*` diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 519081e56d9..a8157aecfe7 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1216,36 +1216,36 @@ RSpec.describe TopicsController do end it "selects the theme the user has selected" do - user.user_option.update_columns(theme_key: theme.key) + user.user_option.update_columns(theme_ids: [theme.id]) get "/t/#{topic.id}" expect(response).to be_redirect - expect(controller.theme_key).to eq(theme.key) + 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_key).not_to eq(theme.key) + expect(controller.theme_id).not_to eq(theme.id) end it "can be overridden with a cookie" do - user.user_option.update_columns(theme_key: theme.key) + user.user_option.update_columns(theme_ids: [theme.id]) - cookies['theme_key'] = "#{theme2.key},#{user.user_option.theme_key_seq}" + cookies['theme_ids'] = "#{theme2.id}|#{user.user_option.theme_key_seq}" get "/t/#{topic.id}" expect(response).to be_redirect - expect(controller.theme_key).to eq(theme2.key) + 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_key: theme.key) - cookies['theme_key'] = "#{theme2.key},#{user.user_option.theme_key_seq - 1}" + 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_key).to eq(theme.key) + expect(controller.theme_id).to eq(theme.id) end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 9cd7f4bb8ba..9f86db38a7d 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1444,14 +1444,14 @@ describe UsersController do put "/u/#{user.username}.json", params: { muted_usernames: "", - theme_key: theme.key, + theme_ids: [theme.id], email_direct: false } user.reload expect(user.muted_users.pluck(:username).sort).to be_empty - expect(user.user_option.theme_key).to eq(theme.key) + expect(user.user_option.theme_ids).to eq([theme.id]) expect(user.user_option.email_direct).to eq(false) end diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index 14d820d488e..5e4e549f881 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -95,7 +95,7 @@ describe UserUpdater do notification_level_when_replying: 3, email_in_reply_to: false, date_of_birth: date_of_birth, - theme_key: theme.key, + theme_ids: [theme.id], allow_private_messages: false) expect(val).to be_truthy @@ -110,7 +110,7 @@ describe UserUpdater do expect(user.user_option.auto_track_topics_after_msecs).to eq 101 expect(user.user_option.notification_level_when_replying).to eq 3 expect(user.user_option.email_in_reply_to).to eq false - expect(user.user_option.theme_key).to eq theme.key + expect(user.user_option.theme_ids.first).to eq theme.id expect(user.user_option.theme_key_seq).to eq(seq + 1) expect(user.user_option.allow_private_messages).to eq(false) expect(user.date_of_birth).to eq(date_of_birth.to_date)