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
This commit is contained in:
OsamaSayegh 2018-07-12 07:18:21 +03:00 committed by Sam
parent f13a7226db
commit decf1f27cf
45 changed files with 289 additions and 241 deletions

View File

@ -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")
);
}

View File

@ -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)) {

View File

@ -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;

View File

@ -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"
];

View File

@ -2,7 +2,7 @@
<div class="control-group theme">
<label class="control-label">{{i18n 'user.theme'}}</label>
<div class="controls">
{{combo-box content=userSelectableThemes value=themeKey}}
{{combo-box content=userSelectableThemes value=themeId}}
</div>
<div class="controls">
{{preference-checkbox labelKey="user.theme_default_on_all_devices" checked=makeThemeDefault}}

View File

@ -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!

View File

@ -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
{}

View File

@ -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"]

View File

@ -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?

View File

@ -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,

View File

@ -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

View File

@ -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

View File

@ -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
#

View File

@ -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

View File

@ -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
#

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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|

View File

@ -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 %>

View File

@ -4,7 +4,7 @@
<meta charset="utf-8">
<title><%= content_for?(:title) ? yield(:title) : SiteSetting.title %></title>
<meta name="description" content="<%= @description_meta || SiteSetting.site_description %>">
<meta name="discourse_theme_key" content="<%= theme_key %>">
<meta name="discourse_theme_id" content="<%= theme_id %>">
<meta name="discourse_current_homepage" content="<%= current_homepage %>">
<%= render partial: "layouts/head" %>
<%= discourse_csrf_tags %>

View File

@ -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") %>

View File

@ -3,7 +3,7 @@
<head>
<meta charset="utf-8">
<%= 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 %>

View File

@ -1,6 +1,6 @@
<html>
<head>
<%= 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' %>

View File

@ -1,6 +1,6 @@
<html>
<head>
<%= 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' %>

View File

@ -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"

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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

View File

@ -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: "<b>I am bold")
theme.save!
expect(Theme.lookup_field(theme.key, :desktop, "head_tag")).to eq("<b>I am bold</b>")
expect(Theme.lookup_field(theme.id, :desktop, "head_tag")).to eq("<b>I am bold</b>")
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(/<b>test<\/b>/)
expect(Theme.lookup_field(theme.id, :desktop, :body_tag)).to match(/<b>test<\/b>/)
end
context "plugin api" do
@ -244,7 +239,7 @@ HTML
});</script>
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() {};
});</script>
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

View File

@ -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

View File

@ -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}/*`

View File

@ -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

View File

@ -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

View File

@ -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)