PERF: Remove N+1 queries on site serializer.

approved

#1

PERF: Remove N+1 queries on site serializer.

diff --git a/plugin.rb b/plugin.rb
index e7b9df4..24bb46c 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -329,11 +329,23 @@ SQL
     attributes :custom_fields
 
     def custom_fields
-      object.custom_fields.slice("enable_accepted_answers")
+      object.custom_fields
     end
 
     def include_custom_fields?
-      custom_fields.present?
+      SiteSetting.show_filter_by_solved_status && custom_fields.present?
+    end
+  end
+
+  require_dependency 'site'
+  class ::Site
+    alias_method :discourse_categories, :categories
+
+    def categories
+      @categories ||= begin
+        Category.preload_custom_fields(discourse_categories, ["enable_accepted_answers"]) if SiteSetting.show_filter_by_solved_status
+        discourse_categories
+      end
     end
   end
 
diff --git a/spec/models/site_spec.rb b/spec/models/site_spec.rb
index 09cfac3..2e4e27b 100644
--- a/spec/models/site_spec.rb
+++ b/spec/models/site_spec.rb
@@ -5,6 +5,10 @@ describe Site do
   let(:category) { Fabricate(:category) }
   let(:guardian) { Guardian.new }
 
+  before do
+    SiteSetting.show_filter_by_solved_status = true
+  end
+
   it "includes `enable_accepted_answers` custom field for categories" do
     category.custom_fields["enable_accepted_answers"] = true
     category.save_custom_fields

GitHub sha: 0fa41a8b


#2

Should we exclude the custom fields as well if the plugin is not enabled?


#3

We should extend the core code base instead of monkey patching here. Something like what CategoryList does.


Follow Up #4

#6

I think so. Custom fields are not added by the core. If other plugins wanted to add it then we should create a clean way in core. Else they also will trigger the N+1.


#7

I agree. CategoryList looks cleaner. I will update something similar in the core.


Approved #8

Follow Up #9

#12

It’s done as per the commits


Approved #13