diff --git a/app/assets/javascripts/admin/components/themes-list-item.js.es6 b/app/assets/javascripts/admin/components/themes-list-item.js.es6 index 5043f6f7437..a3a84ea3d89 100644 --- a/app/assets/javascripts/admin/components/themes-list-item.js.es6 +++ b/app/assets/javascripts/admin/components/themes-list-item.js.es6 @@ -2,6 +2,8 @@ import { default as computed, observes } from "ember-addons/ember-computed-decorators"; +import { iconHTML } from "discourse-common/lib/icon-library"; +import { escape } from "pretty-text/sanitizer"; const MAX_COMPONENTS = 4; @@ -64,7 +66,10 @@ export default Ember.Component.extend({ children = this.childrenExpanded ? children : children.slice(0, MAX_COMPONENTS); - return children.map(t => t.get("name")); + return children.map(t => { + const name = escape(t.name); + return t.enabled ? name : `${iconHTML("ban")} ${name}`; + }); }, @computed("children") diff --git a/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 b/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 index 552a1d3aab7..fc211ce628c 100644 --- a/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 @@ -301,6 +301,20 @@ export default Ember.Controller.extend({ } else { this.commitSwitchType(); } + }, + + enableComponent() { + this.model.set("enabled", true); + this.model + .saveChanges("enabled") + .catch(() => this.model.set("enabled", false)); + }, + + disableComponent() { + this.model.set("enabled", false); + this.model + .saveChanges("enabled") + .catch(() => this.model.set("enabled", true)); } } }); diff --git a/app/assets/javascripts/admin/templates/components/themes-list-item.hbs b/app/assets/javascripts/admin/templates/components/themes-list-item.hbs index 663488607fe..aba96d1917b 100644 --- a/app/assets/javascripts/admin/templates/components/themes-list-item.hbs +++ b/app/assets/javascripts/admin/templates/components/themes-list-item.hbs @@ -17,6 +17,9 @@ {{#if theme.isBroken}} {{d-icon "exclamation-circle" class="broken-indicator" title="admin.customize.theme.broken_theme_tooltip"}} {{/if}} + {{#unless theme.enabled}} + {{d-icon "ban" class="light-grey-icon" title="admin.customize.theme.disabled_component_tooltip"}} + {{/unless}} {{else}} {{d-icon "caret-right"}} {{/unless}} @@ -25,7 +28,7 @@ {{#if displayComponents}}
- {{childrenString}} + {{{childrenString}}} {{#if displayHasMore}} diff --git a/app/assets/javascripts/admin/templates/customize-themes-show.hbs b/app/assets/javascripts/admin/templates/customize-themes-show.hbs index e4796c8cdcb..f7e198d39da 100644 --- a/app/assets/javascripts/admin/templates/customize-themes-show.hbs +++ b/app/assets/javascripts/admin/templates/customize-themes-show.hbs @@ -16,7 +16,7 @@
{{/each}} - {{#unless model.enabled}} + {{#unless model.supported}}
{{i18n "admin.customize.theme.required_version.error"}} {{#if model.remote_theme.minimum_discourse_version}} @@ -28,6 +28,26 @@
{{/unless}} + {{#unless model.enabled}} +
+ {{#if model.disabled_by}} + {{i18n "admin.customize.theme.disabled_by"}} + {{#user-link user=model.disabled_by}} + {{avatar model.disabled_by imageSize="tiny"}} + {{model.disabled_by.username}} + {{/user-link}} + {{format-date model.disabled_at leaveAgo="true"}} + {{else}} + {{i18n "admin.customize.theme.disabled"}} + {{/if}} + {{d-button + class='btn-default' + action=(action "enableComponent") + icon="check" + label="admin.customize.theme.enable"}} +
+ {{/unless}} + {{#unless model.component}}
{{inline-edit-checkbox action=(action "applyDefault") labelKey="admin.customize.theme.is_default" checked=model.default}} @@ -204,7 +224,13 @@ {{#if model.childThemes.length}} {{/if}} @@ -221,5 +247,22 @@ {{d-icon "download"}} {{i18n 'admin.export_json.button_text'}} {{d-button action=(action "switchType") label="admin.customize.theme.convert" icon=convertIcon class="btn-default btn-normal" title=convertTooltip}} + + {{#if model.component}} + {{#if model.enabled}} + {{d-button + class='btn-default' + action=(action "disableComponent") + icon="ban" + label="admin.customize.theme.disable"}} + {{else}} + {{d-button + class='btn-default' + action=(action "enableComponent") + icon="check" + label="admin.customize.theme.enable"}} + {{/if}} + {{/if}} + {{d-button action=(action "destroy") label="admin.customize.delete" icon="trash-alt" class="btn-danger"}}
diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index 15136dd1a12..86290bcf031 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/app/assets/stylesheets/common/admin/customize.scss @@ -513,8 +513,19 @@ list-style: none; margin-left: 0; li { + &.disabled-child { + .child-link { + color: $primary-medium; + &:hover { + text-decoration: underline; + } + } + } + .btn { + margin-left: 5px; + } display: table-row; - .col:first-child { + .col.child-link { padding-right: 10px; padding-bottom: 10px; min-width: 80px; diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index 4684ebeebbd..5389269af87 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -156,8 +156,10 @@ class Admin::ThemesController < Admin::AdminController raise Discourse::InvalidParameters.new(:id) unless @theme original_json = ThemeSerializer.new(@theme, root: false).to_json + disables_component = [false, "false"].include?(theme_params[:enabled]) + enables_component = [true, "true"].include?(theme_params[:enabled]) - [:name, :color_scheme_id, :user_selectable].each do |field| + [:name, :color_scheme_id, :user_selectable, :enabled].each do |field| if theme_params.key?(field) @theme.public_send("#{field}=", theme_params[field]) end @@ -203,7 +205,13 @@ class Admin::ThemesController < Admin::AdminController update_default_theme @theme.reload - log_theme_change(original_json, @theme) + + if (!disables_component && !enables_component) || theme_params.keys.size > 1 + log_theme_change(original_json, @theme) + end + log_theme_component_disabled if disables_component + log_theme_component_enabled if enables_component + format.json { render json: @theme, status: :ok } else format.json do @@ -304,6 +312,7 @@ class Admin::ThemesController < Admin::AdminController :default, :user_selectable, :component, + :enabled, settings: {}, translations: {}, theme_fields: [:name, :target, :value, :upload_id, :type_id], @@ -350,6 +359,14 @@ class Admin::ThemesController < Admin::AdminController StaffActionLogger.new(current_user).log_theme_setting_change(setting_name, previous_value, new_value, @theme) end + def log_theme_component_disabled + StaffActionLogger.new(current_user).log_theme_component_disabled(@theme) + end + + def log_theme_component_enabled + StaffActionLogger.new(current_user).log_theme_component_enabled(@theme) + end + def handle_switch param = theme_params[:component] if param.to_s == "false" && @theme.component? diff --git a/app/models/theme.rb b/app/models/theme.rb index 167f01daadf..81a347861c7 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -156,8 +156,10 @@ class Theme < ActiveRecord::Base all_ids = [parent, *components] - disabled_ids = Theme.where(id: all_ids).includes(:remote_theme) - .reject(&:enabled?).pluck(:id) + disabled_ids = Theme.where(id: all_ids) + .includes(:remote_theme) + .select { |t| !t.supported? || !t.enabled? } + .pluck(:id) all_ids - disabled_ids end @@ -177,7 +179,7 @@ class Theme < ActiveRecord::Base SiteSetting.default_theme_id == id end - def enabled? + def supported? if minimum_version = remote_theme&.minimum_discourse_version return false unless Discourse.has_needed_version?(Discourse::VERSION::STRING, minimum_version) end @@ -216,6 +218,7 @@ class Theme < ActiveRecord::Base return unless component Theme.transaction do + self.enabled = true self.component = false ChildTheme.where("child_theme_id = ?", id).destroy_all self.save! @@ -489,6 +492,22 @@ class Theme < ActiveRecord::Base end end + + def disabled_by + find_disable_action_log&.acting_user + end + + def disabled_at + find_disable_action_log&.created_at + end + + private + + def find_disable_action_log + if component? && !enabled? + @disable_log ||= UserHistory.where(context: id.to_s, action: UserHistory.actions[:disable_theme_component]).order("created_at DESC").first + end + end end # == Schema Information @@ -506,6 +525,7 @@ end # color_scheme_id :integer # remote_theme_id :integer # component :boolean default(FALSE), not null +# enabled :boolean default(TRUE), not null # # Indexes # diff --git a/app/models/user_history.rb b/app/models/user_history.rb index ac9d83e670a..c632a3c1800 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -95,7 +95,9 @@ class UserHistory < ActiveRecord::Base embeddable_host_update: 74, embeddable_host_destroy: 75, web_hook_deactivate: 76, - change_theme_setting: 77 + change_theme_setting: 77, + disable_theme_component: 78, + enable_theme_component: 79 ) end diff --git a/app/serializers/theme_serializer.rb b/app/serializers/theme_serializer.rb index fce406ef257..0682f694877 100644 --- a/app/serializers/theme_serializer.rb +++ b/app/serializers/theme_serializer.rb @@ -63,9 +63,11 @@ class RemoteThemeSerializer < ApplicationSerializer end class ThemeSerializer < BasicThemeSerializer - attributes :color_scheme, :color_scheme_id, :user_selectable, :remote_theme_id, :settings, :errors, :enabled?, :description + attributes :color_scheme, :color_scheme_id, :user_selectable, :remote_theme_id, + :settings, :errors, :supported?, :description, :enabled?, :disabled_at has_one :user, serializer: UserNameSerializer, embed: :object + has_one :disabled_by, serializer: UserNameSerializer, embed: :object has_many :theme_fields, serializer: ThemeFieldSerializer, embed: :objects has_many :child_themes, serializer: BasicThemeSerializer, embed: :objects @@ -108,4 +110,12 @@ class ThemeSerializer < BasicThemeSerializer def description object.internal_translations.find { |t| t.key == "theme_metadata.description" } &.value end + + def include_disabled_at? + object.component? && !object.enabled? + end + + def include_disabled_by? + include_disabled_at? + end end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 3967ffceac9..765a05b5420 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -206,6 +206,22 @@ class StaffActionLogger )) end + def log_theme_component_disabled(component) + UserHistory.create!(params.merge( + action: UserHistory.actions[:disable_theme_component], + subject: component.name, + context: component.id + )) + end + + def log_theme_component_enabled(component) + UserHistory.create!(params.merge( + action: UserHistory.actions[:enable_theme_component], + subject: component.name, + context: component.id + )) + end + def log_theme_setting_change(setting_name, previous_value, new_value, theme, opts = {}) raise Discourse::InvalidParameters.new(:theme) unless theme raise Discourse::InvalidParameters.new(:setting_name) unless theme.included_settings.has_key?(setting_name) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5eff6497bc5..2c45ac584f8 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3481,6 +3481,7 @@ en: inactive_themes: "Inactive themes:" inactive_components: "Unused components:" broken_theme_tooltip: "This theme has errors in its CSS, HTML or YAML" + disabled_component_tooltip: "This component has been disabled" default_theme_tooltip: "This theme is the site's default theme" updates_available_tooltip: "Updates are available for this theme" and_x_more: "and {{count}} more." @@ -3521,6 +3522,10 @@ en: version: "Version:" authors: "Authored by:" source_url: "Source" + enable: "Enable" + disable: "Disable" + disabled: "This component has been disabled." + disabled_by: "This component has been disabled by" required_version: error: "This theme has been automatically disabled because it is not compatible with this version of Discourse." minimum: "Requires Discourse version {{version}} or above." diff --git a/db/migrate/20190630165003_add_enabled_to_themes.rb b/db/migrate/20190630165003_add_enabled_to_themes.rb new file mode 100644 index 00000000000..c1cfdf785e7 --- /dev/null +++ b/db/migrate/20190630165003_add_enabled_to_themes.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddEnabledToThemes < ActiveRecord::Migration[5.2] + def change + add_column :themes, :enabled, :boolean, null: false, default: true + end +end diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 149badb6035..0431413867b 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -60,9 +60,9 @@ describe Theme do end it "can automatically disable for mismatching version" do - expect(theme.enabled?).to eq(true) + expect(theme.supported?).to eq(true) theme.create_remote_theme!(remote_url: "", minimum_discourse_version: "99.99.99") - expect(theme.enabled?).to eq(false) + expect(theme.supported?).to eq(false) expect(Theme.transform_ids([theme.id])).to be_empty end @@ -72,6 +72,13 @@ describe Theme do expect(Theme.transform_ids([nil])).to eq([nil]) end + it '#transform_ids filters out disabled components' do + theme.add_child_theme!(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]) + end + it "doesn't allow multi-level theme components" do grandchild = Fabricate(:theme, user: user) grandparent = Fabricate(:theme, user: user) diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 0afd56775c9..4f32a63e013 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -322,6 +322,65 @@ describe Admin::ThemesController do expect(theme.theme_translation_overrides.count).to eq(0) end + it 'can disable component' do + child = Fabricate(:theme, component: true) + + put "/admin/themes/#{child.id}.json", params: { + theme: { + enabled: false + } + } + expect(response.status).to eq(200) + json = JSON.parse(response.body) + expect(json["theme"]["enabled"]).to eq(false) + expect(UserHistory.where( + context: child.id.to_s, + action: UserHistory.actions[:disable_theme_component] + ).size).to eq(1) + expect(json["theme"]["disabled_by"]["id"]).to eq(admin.id) + end + + it "enabling/disabling a component creates the correct staff action log" do + child = Fabricate(:theme, component: true) + UserHistory.destroy_all + + put "/admin/themes/#{child.id}.json", params: { + theme: { + enabled: false + } + } + expect(response.status).to eq(200) + + expect(UserHistory.where( + context: child.id.to_s, + action: UserHistory.actions[:disable_theme_component] + ).size).to eq(1) + expect(UserHistory.where( + context: child.id.to_s, + action: UserHistory.actions[:enable_theme_component] + ).size).to eq(0) + + put "/admin/themes/#{child.id}.json", params: { + theme: { + enabled: true + } + } + expect(response.status).to eq(200) + json = JSON.parse(response.body) + + expect(UserHistory.where( + context: child.id.to_s, + action: UserHistory.actions[:disable_theme_component] + ).size).to eq(1) + expect(UserHistory.where( + context: child.id.to_s, + action: UserHistory.actions[:enable_theme_component] + ).size).to eq(1) + + expect(json["theme"]["disabled_by"]).to eq(nil) + expect(json["theme"]["enabled"]).to eq(true) + end + it 'handles import errors on update' do theme.create_remote_theme!(remote_url: "https://example.com/repository") diff --git a/test/javascripts/admin/components/themes-list-item-test.js.es6 b/test/javascripts/admin/components/themes-list-item-test.js.es6 index d7506dc4c4c..ff94859bb2d 100644 --- a/test/javascripts/admin/components/themes-list-item-test.js.es6 +++ b/test/javascripts/admin/components/themes-list-item-test.js.es6 @@ -71,11 +71,14 @@ componentTest("with children", { assert.deepEqual( find(".components") .text() - .trim(), + .trim() + .split(",") + .map(n => n.trim()) + .join(","), childrenList .splice(0, 4) .map(theme => theme.get("name")) - .join(", "), + .join(","), "lists the first 4 children" ); assert.deepEqual(