FEATURE: Ability to add components to all themes (#8404)

* FEATURE: Ability to add components to all themes

This is the first and functional step from that topic https://dev.discourse.org/t/adding-a-theme-component-is-too-much-work/15398/16

The idea here is that when a new component is added, the user can easily assign it to all themes (parents).

To achieve that, I needed to change a site-setting component to accept `setDefaultValues` action and `setDefaultValuesLabel` translated label.
Also, I needed to add `allowAny` option to disable that for theme selector.

I also refactored backend to accept both parent and child ids with one method to avoid duplication (Renamed `add_child_theme!` to more general `add_relative_theme!`)

* FIX: Improvement after code review

* FIX: Improvement after code review2

* FIX: use mapBy and filterBy directly

app/assets/javascripts/admin/components/theme-setting-relatives-selector.js.es6

@@ -0,0 +1,26 @@
+import Component from "@ember/component";
+import BufferedContent from "discourse/mixins/buffered-content";
+import SettingComponent from "admin/mixins/setting-component";
+
+export default Component.extend(BufferedContent, SettingComponent, {
+  layoutName: "admin/templates/components/site-setting",
+
+  _save() {
+    return this.model
+      .save({ [this.setting.setting]: this.convertNamesToIds() })
+      .then(() => this.store.findAll("theme"));
+  },
+
+  convertNamesToIds() {
+    return this.get("buffered.value")
+      .split("|")
+      .filter(Boolean)
+      .map(themeName => {
+        if (themeName !== "") {
+          return this.setting.allThemes.find(theme => theme.name === themeName)
+            .id;
+        }
+        return themeName;
+      });
+  }
+});

app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6

@@ -1,5 +1,11 @@
 import { makeArray } from "discourse-common/lib/helpers";
-import { empty, notEmpty, match } from "@ember/object/computed";
+import {
+  empty,
+  filterBy,
+  match,
+  mapBy,
+  notEmpty
+} from "@ember/object/computed";
 import Controller from "@ember/controller";
 import { default as discourseComputed } from "discourse-common/utils/decorators";
 import { url } from "discourse/lib/computed";
@@ -15,6 +21,9 @@ export default Controller.extend({
   previewUrl: url("model.id", "/admin/themes/%@/preview"),
   addButtonDisabled: empty("selectedChildThemeId"),
   editRouteName: "adminCustomizeThemes.edit",
+  parentThemesNames: mapBy("model.parentThemes", "name"),
+  availableParentThemes: filterBy("allThemes", "component", false),
+  availableThemesNames: mapBy("availableParentThemes", "name"),
 
   @discourseComputed("model.editedFields")
   editedFieldsFormatted() {
@@ -50,6 +59,24 @@ export default Controller.extend({
     }
   },
 
+  @discourseComputed("model.parentThemes.[]")
+  relativesSelectorSettings() {
+    return Ember.Object.create({
+      list_type: "compact",
+      type: "list",
+      preview: null,
+      anyValue: false,
+      setting: "parent_theme_ids",
+      label: I18n.t("admin.customize.theme.component_on_themes"),
+      choices: this.availableThemesNames,
+      default: this.parentThemesNames.join("|"),
+      value: this.parentThemesNames.join("|"),
+      defaultValues: this.availableThemesNames.join("|"),
+      allThemes: this.allThemes,
+      setDefaultValuesLabel: I18n.t("admin.customize.theme.add_all_themes")
+    });
+  },
+
   @discourseComputed("allThemes", "model.component", "model")
   availableChildThemes(allThemes) {
     if (!this.get("model.component")) {
@@ -241,7 +268,7 @@ export default Controller.extend({
     addChildTheme() {
       let themeId = parseInt(this.selectedChildThemeId, 10);
       let theme = this.allThemes.findBy("id", themeId);
-      this.model.addChildTheme(theme);
+      this.model.addChildTheme(theme).then(() => this.store.findAll("theme"));
     },
 
     removeUpload(upload) {
@@ -258,7 +285,9 @@ export default Controller.extend({
     },
 
     removeChildTheme(theme) {
-      this.model.removeChildTheme(theme);
+      this.model
+        .removeChildTheme(theme)
+        .then(() => this.store.findAll("theme"));
     },
 
     destroy() {

app/assets/javascripts/admin/mixins/setting-component.js.es6

@@ -51,7 +51,6 @@ export default Mixin.create({
         });
       }
     }
-
     let preview = setting.get("preview");
     if (preview) {
       return new Handlebars.SafeString(
@@ -67,9 +66,9 @@ export default Mixin.create({
     return componentType.replace(/\_/g, "-");
   },
 
-  @discourseComputed("setting.setting")
-  settingName(setting) {
-    return setting.replace(/\_/g, " ");
+  @discourseComputed("setting.setting", "setting.label")
+  settingName(setting, label) {
+    return label || setting.replace(/\_/g, " ");
   },
 
   @discourseComputed("type")
@@ -91,6 +90,11 @@ export default Mixin.create({
     return "site-settings/" + typeClass;
   },
 
+  @discourseComputed("setting.anyValue")
+  allowAny(anyValue) {
+    return anyValue !== false;
+  },
+
   @discourseComputed("setting.default", "buffered.value")
   overridden(settingDefault, bufferedValue) {
     return settingDefault !== bufferedValue;
@@ -209,6 +213,10 @@ export default Mixin.create({
 
     toggleSecret() {
       this.toggleProperty("isSecret");
+    },
+
+    setDefaultValues() {
+      this.set("buffered.value", this.get("setting.defaultValues"));
     }
   }
 });

app/assets/javascripts/admin/models/theme.js.es6

@@ -269,6 +269,15 @@ const Theme = RestModel.extend({
     return this.saveChanges("child_theme_ids");
   },
 
+  addParentTheme(theme) {
+    let parentThemes = this.parentThemes;
+    if (!parentThemes) {
+      parentThemes = [];
+      this.set("parentThemes", parentThemes);
+    }
+    parentThemes.addObject(theme);
+  },
+
   @discourseComputed("name", "default")
   description: function(name, isDefault) {
     if (isDefault) {

app/assets/javascripts/admin/templates/components/site-setting.hbs

@@ -1,8 +1,11 @@
 <div class='setting-label'>
    <h3>{{unbound settingName}}</h3>
+   {{#if setting.defaultValues }}
+     <a onClick={{action 'setDefaultValues'}}>{{setting.setDefaultValuesLabel}}</a>
+   {{/if}}
 </div>
 <div class="setting-value">
-  {{component componentName setting=setting value=buffered.value validationMessage=validationMessage preview=preview isSecret=isSecret}}
+  {{component componentName setting=setting value=buffered.value validationMessage=validationMessage preview=preview isSecret=isSecret allowAny=allowAny}}
 </div>
 {{#if dirty}}
   <div class='setting-controls'>

app/assets/javascripts/admin/templates/components/site-settings/compact-list.hbs

@@ -1,3 +1,3 @@
-{{list-setting settingValue=value choices=setting.choices settingName=setting.setting}}
+{{list-setting settingValue=value choices=setting.choices settingName=setting.setting allowAny=allowAny}}
 {{setting-validation-message message=validationMessage}}
 <div class='desc'>{{{unbound setting.description}}}</div>

app/assets/javascripts/admin/templates/customize-themes-show.hbs

@@ -59,16 +59,16 @@
 
     {{#if model.remote_theme.remote_url}}
       {{#if sourceIsHttp}}
-        <a class="remote-url" href="{{model.remote_theme.remote_url}}">{{i18n "admin.customize.theme.source_url"}} {{d-icon "link"}}</a>
+        <a class="remote-url" href="{{model.remote_theme.remote_url}}">{{i18n "admin.customize.theme.source_url"}}{{d-icon "link"}}</a>
       {{else}}
         <div class="remote-url"><code>{{model.remote_theme.remote_url}}</code></div>
       {{/if}}
     {{/if}}
     {{#if model.remote_theme.about_url}}
-    <a class="url about-url" href="{{model.remote_theme.about_url}}">{{i18n "admin.customize.theme.about_theme"}} {{d-icon "link"}}</a>
+      <a class="url about-url" href="{{model.remote_theme.about_url}}">{{i18n "admin.customize.theme.about_theme"}}{{d-icon "link"}}</a>
     {{/if}}
     {{#if model.remote_theme.license_url}}
-      <a class="url license-url" href="{{model.remote_theme.license_url}}">{{i18n "admin.customize.theme.license"}} {{d-icon "link"}}</a>
+      <a class="url license-url" href="{{model.remote_theme.license_url}}">{{i18n "admin.customize.theme.license"}}{{d-icon "link"}}</a>
     {{/if}}
 
     {{#if model.description}}
@@ -156,6 +156,17 @@
     </div>
   {{/if}}
 
+  {{#if model.component }}
+    <div class="control-unit">
+      <div class="mini-title">{{i18n "admin.customize.theme.title"}}</div>
+      {{#d-section class="form-horizontal theme settings"}}
+        <div class="row setting">
+          {{theme-setting-relatives-selector setting=relativesSelectorSettings model=model class="theme-setting"}}
+        </div>
+      {{/d-section}}
+    </div>
+  {{/if}}
+
   <div class="control-unit">
     <div class="mini-title">{{i18n "admin.customize.theme.css_html"}}</div>
     {{#if model.hasEditedFields}}

app/assets/stylesheets/common/admin/customize.scss

@@ -150,6 +150,9 @@
     a.license-url {
       display: inline-block;
       margin-right: 10px;
+      .d-icon {
+        margin-left: 5px;
+      }
     }
 
     .mini-title {

app/controllers/admin/themes_controller.rb

@@ -176,19 +176,11 @@ class Admin::ThemesController < Admin::AdminController
     end
 
     if theme_params.key?(:child_theme_ids)
-      expected = theme_params[:child_theme_ids].map(&:to_i)
+      add_relative_themes!(:child, theme_params[:child_theme_ids])
+    end
 
-      @theme.child_theme_relation.to_a.each do |child|
-        if expected.include?(child.child_theme_id)
-          expected.reject! { |id| id == child.child_theme_id }
-        else
-          child.destroy
-        end
-      end
-
-      Theme.where(id: expected).each do |theme|
-        @theme.add_child_theme!(theme)
-      end
+    if theme_params.key?(:parent_theme_ids)
+      add_relative_themes!(:parent, theme_params[:parent_theme_ids])
     end
 
     set_fields
@@ -294,6 +286,26 @@ class Admin::ThemesController < Admin::AdminController
 
   private
 
+  def add_relative_themes!(kind, ids)
+    expected = ids.map(&:to_i)
+
+    relation = kind == :child ? @theme.child_theme_relation : @theme.parent_theme_relation
+
+    relation.to_a.each do |relative|
+      if kind == :child && expected.include?(relative.child_theme_id)
+        expected.reject! { |id| id == relative.child_theme_id }
+      elsif kind == :parent && expected.include?(relative.parent_theme_id)
+        expected.reject! { |id| id == relative.parent_theme_id }
+      else
+        relative.destroy
+      end
+    end
+
+    Theme.where(id: expected).each do |theme|
+      @theme.add_relative_theme!(kind, theme)
+    end
+  end
+
   def update_default_theme
     if theme_params.key?(:default)
       is_default = theme_params[:default].to_s == "true"
@@ -310,6 +322,7 @@ class Admin::ThemesController < Admin::AdminController
       begin
         # deep munge is a train wreck, work around it for now
         params[:theme][:child_theme_ids] ||= [] if params[:theme].key?(:child_theme_ids)
+        params[:theme][:parent_theme_ids] ||= [] if params[:theme].key?(:parent_theme_ids)
 
         params.require(:theme).permit(
           :name,
@@ -321,7 +334,8 @@ class Admin::ThemesController < Admin::AdminController
           settings: {},
           translations: {},
           theme_fields: [:name, :target, :value, :upload_id, :type_id],
-          child_theme_ids: []
+          child_theme_ids: [],
+          parent_theme_ids: []
         )
       end
   end

app/models/theme.rb

@@ -376,10 +376,15 @@ class Theme < ActiveRecord::Base
     fields.values
   end
 
-  def add_child_theme!(theme)
-    new_relation = child_theme_relation.new(child_theme_id: theme.id)
+  def add_relative_theme!(kind, theme)
+    new_relation = if kind == :child
+      child_theme_relation.new(child_theme_id: theme.id)
+    else
+      parent_theme_relation.new(parent_theme_id: theme.id)
+    end
     if new_relation.save
       child_themes.reload
+      parent_themes.reload
       save!
       Theme.clear_cache!
     else

config/locales/client.en.yml

@@ -3590,6 +3590,7 @@ en:
           color_scheme_select: "Select colors to be used by theme"
           custom_sections: "Custom sections:"
           theme_components: "Theme Components"
+          add_all_themes: "Add all themes"
           convert: "Convert"
           convert_component_alert: "Are you sure you want to convert this component to theme? It will be removed as a component from %{relatives}."
           convert_component_tooltip: "Convert this component to theme"
@@ -3622,6 +3623,7 @@ en:
           edit_css_html: "Edit CSS/HTML"
           edit_css_html_help: "You have not edited any CSS or HTML"
           delete_upload_confirm: "Delete this upload? (Theme CSS may stop working!)"
+          component_on_themes: "Include component on these themes"
           import_web_tip: "Repository containing theme"
           import_web_advanced: "Advanced..."
           import_file_tip: ".tar.gz, .zip, or .dcstyle.json file containing theme"

spec/components/guardian_spec.rb

@@ -2890,7 +2890,7 @@ describe Guardian do
       expect(user_guardian.allow_themes?([theme.id, theme2.id])).to eq(false)
 
       theme2.update!(user_selectable: false, component: true)
-      theme.add_child_theme!(theme2)
+      theme.add_relative_theme!(:child, theme2)
       expect(user_guardian.allow_themes?([theme.id, theme2.id])).to eq(true)
       expect(user_guardian.allow_themes?([theme2.id])).to eq(false)
     end

spec/components/stylesheet/importer_spec.rb

@@ -74,7 +74,7 @@ describe Stylesheet::Importer do
       t.component = true
       t.set_field(target: :extra_scss, name: "my_files/moremagic", value: child_scss)
       t.save!
-      theme.add_child_theme!(t)
+      theme.add_relative_theme!(:child, t)
     }}
 
     let(:importer) { described_class.new(theme: theme) }

spec/components/stylesheet/manager_spec.rb

@@ -40,7 +40,7 @@ describe Stylesheet::Manager do
     child_theme.set_field(target: :common, name: "embedded_scss", value: ".child_embedded{.scss{color: red;}}")
     child_theme.save!
 
-    theme.add_child_theme!(child_theme)
+    theme.add_relative_theme!(:child, child_theme)
 
     old_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
 
@@ -88,7 +88,7 @@ describe Stylesheet::Manager do
     it "can correctly account for settings in theme's components" do
       theme = Fabricate(:theme)
       child = Fabricate(:theme, component: true)
-      theme.add_child_theme!(child)
+      theme.add_relative_theme!(:child, child)
 
       child.set_field(target: :settings, name: :yaml, value: "childcolor: red")
       child.set_field(target: :common, name: :scss, value: "body {background-color: $childcolor}")

spec/components/svg_sprite/svg_sprite_spec.rb

@@ -138,7 +138,7 @@ describe SvgSprite do
     theme.update(component: true)
     theme.save!
     parent_theme = Fabricate(:theme)
-    parent_theme.add_child_theme!(theme)
+    parent_theme.add_relative_theme!(:child, theme)
     expect(SvgSprite.all_icons([parent_theme.id])).to include("dragon")
   end
 

spec/models/theme_spec.rb

@@ -53,12 +53,17 @@ describe Theme do
 
     parent.save!
 
-    parent.add_child_theme!(child)
+    parent.add_relative_theme!(:child, child)
 
     expect(Theme.lookup_field(parent.id, :mobile, "header")).to eq("Common Parent\nMobile Parent\nWorldie\nMobile")
 
   end
 
+  it 'can support parent themes' do
+    child.add_relative_theme!(:parent, theme)
+    expect(child.parent_themes).to eq([theme])
+  end
+
   it "can automatically disable for mismatching version" do
     expect(theme.supported?).to eq(true)
     theme.create_remote_theme!(remote_url: "", minimum_discourse_version: "99.99.99")
@@ -74,7 +79,7 @@ describe Theme do
   end
 
   it '#transform_ids filters out disabled components' do
-    theme.add_child_theme!(child)
+    theme.add_relative_theme!(:child, child)
     expect(Theme.transform_ids([theme.id], extend: true)).to eq([theme.id, child.id])
     child.update!(enabled: false)
     expect(Theme.transform_ids([theme.id], extend: true)).to eq([theme.id])
@@ -85,11 +90,11 @@ describe Theme do
     grandparent = Fabricate(:theme, user: user)
 
     expect do
-      child.add_child_theme!(grandchild)
+      child.add_relative_theme!(:child, grandchild)
     end.to raise_error(Discourse::InvalidParameters, I18n.t("themes.errors.no_multilevels_components"))
 
     expect do
-      grandparent.add_child_theme!(theme)
+      grandparent.add_relative_theme!(:child, theme)
     end.to raise_error(Discourse::InvalidParameters, I18n.t("themes.errors.no_multilevels_components"))
   end
 
@@ -198,7 +203,7 @@ HTML
 
   describe "#switch_to_component!" do
     it "correctly converts a theme to component" do
-      theme.add_child_theme!(child)
+      theme.add_relative_theme!(:child, child)
       scheme = ColorScheme.create!(name: 'test')
       theme.update!(color_scheme_id: scheme.id, user_selectable: true)
       theme.set_default!
@@ -216,7 +221,7 @@ HTML
 
   describe "#switch_to_theme!" do
     it "correctly converts a component to theme" do
-      theme.add_child_theme!(child)
+      theme.add_relative_theme!(:child, child)
 
       child.switch_to_theme!
       theme.reload
@@ -236,8 +241,8 @@ HTML
     let!(:orphan4) { Fabricate(:theme, component: true) }
 
     before do
-      theme.add_child_theme!(child)
-      theme.add_child_theme!(child2)
+      theme.add_relative_theme!(:child, child)
+      theme.add_relative_theme!(:child, child2)
     end
 
     it "returns an empty array if no ids are passed" do
@@ -575,7 +580,7 @@ HTML
     child.set_field(target: :settings, name: "yaml", value: "integer_setting: 54")
 
     child.save!
-    theme.add_child_theme!(child)
+    theme.add_relative_theme!(:child, child)
 
     json = cached_settings(theme.id)
     expect(json).to match(/\"boolean_setting\":false/)

spec/requests/admin/themes_controller_spec.rb

@@ -321,6 +321,16 @@ describe Admin::ThemesController do
       expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
     end
 
+    it 'updates a child theme' do
+      child_theme = Fabricate(:theme, component: true)
+      put "/admin/themes/#{child_theme.id}.json", params: {
+        theme: {
+          parent_theme_ids: [theme.id],
+        }
+      }
+      expect(child_theme.parent_themes).to eq([theme])
+    end
+
     it 'can update translations' do
       theme.set_field(target: :translations, name: :en, value: { en: { somegroup: { somestring: "defaultstring" } } }.deep_stringify_keys.to_yaml)
       theme.save!

spec/requests/application_controller_spec.rb

@@ -374,7 +374,7 @@ RSpec.describe ApplicationController do
       expect(controller.theme_ids).to eq([theme2.id])
 
       theme2.update!(user_selectable: false, component: true)
-      theme.add_child_theme!(theme2)
+      theme.add_relative_theme!(:child, theme2)
       cookies['theme_ids'] = "#{theme.id},#{theme2.id}|#{user.user_option.theme_key_seq}"
 
       get "/"

spec/services/user_updater_spec.rb

@@ -225,7 +225,7 @@ describe UserUpdater do
 
       theme = Fabricate(:theme)
       child = Fabricate(:theme, component: true)
-      theme.add_child_theme!(child)
+      theme.add_relative_theme!(:child, child)
       theme.set_default!
 
       updater.update(theme_ids: [theme.id.to_s, child.id.to_s, "", nil])