FIX: reset embedding settings when no embeddable host, log host changes (#7264)

app/controllers/admin/embeddable_hosts_controller.rb

@@ -1,23 +1,24 @@
 class Admin::EmbeddableHostsController < Admin::AdminController
 
   def create
-    save_host(EmbeddableHost.new)
+    save_host(EmbeddableHost.new, :create)
   end
 
   def update
     host = EmbeddableHost.where(id: params[:id]).first
-    save_host(host)
+    save_host(host, :update)
   end
 
   def destroy
     host = EmbeddableHost.where(id: params[:id]).first
     host.destroy
+    StaffActionLogger.new(current_user).log_embeddable_host(host, UserHistory.actions[:embeddable_host_destroy])
     render json: success_json
   end
 
   protected
 
-  def save_host(host)
+  def save_host(host, action)
     host.host = params[:embeddable_host][:host]
     host.path_whitelist = params[:embeddable_host][:path_whitelist]
     host.class_name =  params[:embeddable_host][:class_name]
@@ -25,6 +26,8 @@ class Admin::EmbeddableHostsController < Admin::AdminController
     host.category_id = SiteSetting.uncategorized_category_id if host.category_id.blank?
 
     if host.save
+      changes = host.saved_changes if action == :update
+      StaffActionLogger.new(current_user).log_embeddable_host(host, UserHistory.actions[:"embeddable_host_#{action}"], changes: changes)
       render_serialized(host, EmbeddableHostSerializer, root: 'embeddable_host', rest_serializer: true)
     else
       render_json_error(host)

app/models/embeddable_host.rb

@@ -3,6 +3,7 @@ require_dependency 'url_helper'
 class EmbeddableHost < ActiveRecord::Base
   validate :host_must_be_valid
   belongs_to :category
+  after_destroy :reset_embedding_settings
 
   before_validation do
     self.host.sub!(/^https?:\/\//, '')
@@ -53,6 +54,12 @@ class EmbeddableHost < ActiveRecord::Base
 
   private
 
+  def reset_embedding_settings
+    unless EmbeddableHost.exists?
+      Embedding.settings.each { |s| SiteSetting.set(s.to_s, SiteSetting.defaults[s]) }
+    end
+  end
+
   def host_must_be_valid
     if host !~ /\A[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,10}(:[0-9]{1,5})?(\/.*)?\Z/i &&
        host !~ /\A(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})(:[0-9]{1,5})?(\/.*)?\Z/ &&

app/models/user_history.rb

@@ -88,7 +88,10 @@ class UserHistory < ActiveRecord::Base
       approve_user: 69,
       web_hook_create: 70,
       web_hook_update: 71,
-      web_hook_destroy: 72
+      web_hook_destroy: 72,
+      embeddable_host_create: 73,
+      embeddable_host_update: 74,
+      embeddable_host_destroy: 75
     )
   end
 
@@ -155,7 +158,10 @@ class UserHistory < ActiveRecord::Base
       :approve_user,
       :web_hook_create,
       :web_hook_update,
-      :web_hook_destroy
+      :web_hook_destroy,
+      :embeddable_host_create,
+      :embeddable_host_update,
+      :embeddable_host_destroy
     ]
   end
 

app/services/staff_action_logger.rb

@@ -589,15 +589,7 @@ class StaffActionLogger
       "payload_url: #{web_hook.payload_url}"
     ]
 
-    if changes = opts[:changes]
-      changes.reject! { |k, v| k == "updated_at" }
-      old_values = []
-      new_values = []
-      changes.each do |k, v|
-        old_values << "#{k}: #{v[0]}"
-        new_values << "#{k}: #{v[1]}"
-      end
-    end
+    old_values, new_values = get_changes(opts[:changes])
 
     UserHistory.create!(params(opts).merge(
       action: action,
@@ -607,8 +599,33 @@ class StaffActionLogger
     ))
   end
 
+  def log_embeddable_host(embeddable_host, action, opts = {})
+    old_values, new_values = get_changes(opts[:changes])
+
+    UserHistory.create!(params(opts).merge(
+      action: action,
+      context: "host: #{embeddable_host.host}",
+      previous_value: old_values&.join(", "),
+      new_value: new_values&.join(", ")
+    ))
+  end
+
   private
 
+  def get_changes(changes)
+    return unless changes
+
+    changes.delete("updated_at")
+    old_values = []
+    new_values = []
+    changes.each do |k, v|
+      old_values << "#{k}: #{v[0]}"
+      new_values << "#{k}: #{v[1]}"
+    end
+
+    [old_values, new_values]
+  end
+
   def params(opts = nil)
     opts ||= {}
     { acting_user_id: @admin.id, context: opts[:context] }

config/locales/client.en.yml

@@ -3689,6 +3689,9 @@ en:
             web_hook_create: "webhook create"
             web_hook_update: "webhook update"
             web_hook_destroy: "webhook destroy"
+            embeddable_host_create: "embeddable host create"
+            embeddable_host_update: "embeddable host update"
+            embeddable_host_destroy: "embeddable host destroy"
         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/locales/server.en.yml

@@ -4359,10 +4359,6 @@ en:
     unknown: "unknown"
     user_merged: "%{username} was merged into this account"
     user_delete_self: "Deleted by self from %{url}"
-    update: "Updated"
-    create: "Created"
-    destroy: "Destroyed"
-
 
   reviewables:
     missing_version: "You must supply a version parameter"

spec/models/embeddable_host_spec.rb

@@ -121,4 +121,26 @@ describe EmbeddableHost do
     end
   end
 
+  describe "reset_embedding_settings" do
+    it "resets all embedding related settings when last embeddable host is removed" do
+      host = Fabricate(:embeddable_host)
+      host2 = Fabricate(:embeddable_host)
+
+      SiteSetting.embed_post_limit = 300
+      SiteSetting.feed_polling_url = "http://test.com"
+      SiteSetting.feed_polling_enabled = true
+
+      host2.destroy
+
+      expect(SiteSetting.embed_post_limit).to eq(300)
+      expect(SiteSetting.feed_polling_url).to eq("http://test.com")
+      expect(SiteSetting.feed_polling_enabled).to eq(true)
+
+      host.destroy
+
+      expect(SiteSetting.embed_post_limit).to eq(SiteSetting.defaults[:embed_post_limit])
+      expect(SiteSetting.feed_polling_url).to eq(SiteSetting.defaults[:feed_polling_url])
+      expect(SiteSetting.feed_polling_enabled).to eq(SiteSetting.defaults[:feed_polling_enabled])
+    end
+  end
 end

spec/requests/admin/embeddable_hosts_controller_spec.rb

@@ -4,4 +4,47 @@ describe Admin::EmbeddableHostsController do
   it "is a subclass of AdminController" do
     expect(Admin::EmbeddableHostsController < Admin::AdminController).to eq(true)
   end
+
+  context 'while logged in as an admin' do
+    let(:admin) { Fabricate(:admin) }
+    let(:embeddable_host) { Fabricate(:embeddable_host) }
+
+    before do
+      sign_in(admin)
+    end
+
+    describe '#create' do
+      it "logs embeddable host create" do
+        post "/admin/embeddable_hosts.json", params: {
+          embeddable_host: { host: "test.com" }
+        }
+
+        expect(response.status).to eq(200)
+        expect(UserHistory.where(acting_user_id: admin.id,
+                                 action: UserHistory.actions[:embeddable_host_create]).exists?).to eq(true)
+      end
+    end
+
+    describe '#update' do
+      it "logs embeddable host update" do
+        put "/admin/embeddable_hosts/#{embeddable_host.id}.json", params: {
+          embeddable_host: { host: "test.com", class_name: "test-class", category_id: "3" }
+        }
+
+        expect(response.status).to eq(200)
+        expect(UserHistory.where(acting_user_id: admin.id,
+                                 action: UserHistory.actions[:embeddable_host_update],
+                                 new_value: "host: test.com, class_name: test-class, category_id: 3").exists?).to eq(true)
+      end
+    end
+
+    describe '#destroy' do
+      it "logs embeddable host destroy" do
+        delete "/admin/embeddable_hosts/#{embeddable_host.id}.json", params: {}
+
+        expect(response.status).to eq(200)
+        expect(UserHistory.where(acting_user_id: admin.id, action: UserHistory.actions[:embeddable_host_destroy]).exists?).to eq(true)
+      end
+    end
+  end
 end