FEATURE: Add endpoint to individually update a theme setting (#7789)

- also adds a new staff action type called "change theme setting" for easier logging of staff changes to theme settings

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

@@ -1,12 +1,17 @@
 import BufferedContent from "discourse/mixins/buffered-content";
 import SettingComponent from "admin/mixins/setting-component";
+import { ajax } from "discourse/lib/ajax";
+import { popupAjaxError } from "discourse/lib/ajax-error";
 
 export default Ember.Component.extend(BufferedContent, SettingComponent, {
   layoutName: "admin/templates/components/site-setting",
   _save() {
-    return this.model.saveSettings(
-      this.get("setting.setting"),
-      this.get("buffered.value")
-    );
+    return ajax(`/admin/themes/${this.model.id}/setting`, {
+      type: "PUT",
+      data: {
+        name: this.setting.setting,
+        value: this.get("buffered.value")
+      }
+    }).catch(popupAjaxError);
   }
 });

app/controllers/admin/themes_controller.rb

@@ -261,6 +261,24 @@ class Admin::ThemesController < Admin::AdminController
     end
   end
 
+  def update_single_setting
+    params.require("name")
+    @theme = Theme.find_by(id: params[:id])
+    raise Discourse::InvalidParameters.new(:id) unless @theme
+
+    setting_name = params[:name].to_sym
+    new_value = params[:value] || nil
+
+    previous_value = @theme.included_settings[setting_name]
+    @theme.update_setting(setting_name, new_value)
+    @theme.save
+
+    log_theme_setting_change(setting_name, previous_value, new_value)
+
+    updated_setting = @theme.included_settings.select { |key, val| key == setting_name }
+    render json: updated_setting, status: :ok
+  end
+
   private
 
   def update_default_theme
@@ -328,6 +346,10 @@ class Admin::ThemesController < Admin::AdminController
     StaffActionLogger.new(current_user).log_theme_change(old_record, new_record)
   end
 
+  def log_theme_setting_change(setting_name, previous_value, new_value)
+    StaffActionLogger.new(current_user).log_theme_setting_change(setting_name, previous_value, new_value, @theme)
+  end
+
   def handle_switch
     param = theme_params[:component]
     if param.to_s == "false" && @theme.component?

app/models/user_history.rb

@@ -94,7 +94,8 @@ class UserHistory < ActiveRecord::Base
       embeddable_host_create: 73,
       embeddable_host_update: 74,
       embeddable_host_destroy: 75,
-      web_hook_deactivate: 76
+      web_hook_deactivate: 76,
+      change_theme_setting: 77
     )
   end
 
@@ -165,7 +166,8 @@ class UserHistory < ActiveRecord::Base
       :web_hook_deactivate,
       :embeddable_host_create,
       :embeddable_host_update,
-      :embeddable_host_destroy
+      :embeddable_host_destroy,
+      :change_theme_setting
     ]
   end
 

app/services/staff_action_logger.rb

@@ -206,6 +206,18 @@ class StaffActionLogger
     ))
   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)
+
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:change_theme_setting],
+      subject: "#{theme.name}: #{setting_name.to_s}",
+      previous_value: previous_value,
+      new_value: new_value
+    ))
+  end
+
   def log_site_text_change(subject, new_text = nil, old_text = nil, opts = {})
     raise Discourse::InvalidParameters.new(:subject) unless subject.present?
     UserHistory.create!(params(opts).merge(

config/locales/client.en.yml

@@ -3783,6 +3783,7 @@ en:
             embeddable_host_create: "embeddable host create"
             embeddable_host_update: "embeddable host update"
             embeddable_host_destroy: "embeddable host destroy"
+            change_theme_setting: "change theme setting"
         screened_emails:
           title: "Screened Emails"
           description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."

config/routes.rb

@@ -209,6 +209,7 @@ Discourse::Application.routes.draw do
     post "themes/generate_key_pair" => "themes#generate_key_pair"
     get "themes/:id/preview" => "themes#preview"
     get "themes/:id/diff_local_changes" => "themes#diff_local_changes"
+    put "themes/:id/setting" => "themes#update_single_setting"
 
     scope "/customize", constraints: AdminConstraint.new do
       resources :user_fields, constraints: AdminConstraint.new

spec/requests/admin/themes_controller_spec.rb

@@ -387,4 +387,39 @@ describe Admin::ThemesController do
       expect(response.body).to eq("{}")
     end
   end
+
+  describe '#update_single_setting' do
+    let(:theme) { Fabricate(:theme) }
+
+    before do
+      theme.set_field(target: :settings, name: :yaml, value: "bg: red")
+      theme.save!
+    end
+
+    it "should update a theme setting" do
+      put "/admin/themes/#{theme.id}/setting.json", params: {
+        name: "bg",
+        value: "green"
+      }
+
+      expect(response.status).to eq(200)
+      expect(JSON.parse(response.body)["bg"]).to eq("green")
+
+      theme.reload
+      expect(theme.included_settings[:bg]).to eq("green")
+      user_history = UserHistory.last
+
+      expect(user_history.action).to eq(
+        UserHistory.actions[:change_theme_setting]
+      )
+    end
+
+    it "should clear a theme setting" do
+      put "/admin/themes/#{theme.id}/setting.json", params: { name: "bg" }
+      theme.reload
+
+      expect(response.status).to eq(200)
+      expect(theme.included_settings[:bg]).to eq("")
+    end
+  end
 end

spec/services/staff_action_logger_spec.rb

@@ -202,6 +202,33 @@ describe StaffActionLogger do
     end
   end
 
+  describe "log_theme_setting_change" do
+
+    it "raises an error when params are invalid" do
+      expect { logger.log_theme_setting_change(nil, nil, nil, nil) }.to raise_error(Discourse::InvalidParameters)
+    end
+
+    let! :theme do
+      Fabricate(:theme)
+    end
+
+    before do
+      theme.set_field(target: :settings, name: :yaml, value: "custom_setting: special")
+      theme.save!
+    end
+
+    it "raises an error when theme setting is invalid" do
+      expect { logger.log_theme_setting_change(:inexistent_setting, nil, nil, theme) }.to raise_error(Discourse::InvalidParameters)
+    end
+
+    it "logs theme setting changes" do
+      log_record = logger.log_theme_setting_change(:custom_setting, "special", "notsospecial", theme)
+      expect(log_record.subject).to eq("#{theme.name}: custom_setting")
+      expect(log_record.previous_value).to eq("special")
+      expect(log_record.new_value).to eq("notsospecial")
+    end
+  end
+
   describe "log_site_text_change" do
     it "raises an error when params are invalid" do
       expect { logger.log_site_text_change(nil, 'new text', 'old text') }.to raise_error(Discourse::InvalidParameters)