From eb8dbf881691479e48c083b346bb33a7044a750d Mon Sep 17 00:00:00 2001
From: John Ferlito <johnf@inodes.org>
Date: Wed, 17 Jan 2024 19:06:16 +1100
Subject: [PATCH] Fix some N+1 queries

---
 app/controllers/collections_controller.rb |  2 +-
 app/controllers/items_controller.rb       | 37 ++++++++++++++++-------
 app/views/collections/_items.html.haml    |  2 +-
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/app/controllers/collections_controller.rb b/app/controllers/collections_controller.rb
index e75d9c99..611c2764 100644
--- a/app/controllers/collections_controller.rb
+++ b/app/controllers/collections_controller.rb
@@ -44,7 +44,7 @@ def show
     @num_items_ready = @collection.items.where.not(digitised_on: nil).count
     @num_essences = Essence.where(item_id: @collection.items).count
 
-    @items = @collection.items.includes(:access_condition, :collection).page(params[:items_page]).per(params[:items_per_page])
+    @items = @collection.items.includes(:access_condition, :collection, :essences).page(params[:items_page]).per(params[:items_per_page])
 
     @items = @items.order(params[:sort] ? "#{params[:sort]} #{params[:direction]}" : :identifier)
 
diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb
index 492e8717..ef156a00 100644
--- a/app/controllers/items_controller.rb
+++ b/app/controllers/items_controller.rb
@@ -2,8 +2,11 @@ class ItemsController < ApplicationController
   include HasReturnToLastSearch
   include ItemQueryBuilder
 
-  load_and_authorize_resource :collection, find_by: :identifier, except: %i[search advanced_search bulk_update bulk_edit new_report send_report report_sent]
-  load_and_authorize_resource :item, find_by: :identifier, through: :collection, except: %i[search advanced_search bulk_update bulk_edit new_report send_report report_sent]
+  before_action :find_item, only: :show
+  load_and_authorize_resource :collection, find_by: :identifier,
+                                           except: %i[search advanced_search bulk_update bulk_edit new_report send_report report_sent]
+  load_and_authorize_resource :item, find_by: :identifier, through: :collection,
+                                     except: %i[search advanced_search bulk_update bulk_edit new_report send_report report_sent]
   authorize_resource only: %i[advanced_search bulk_update bulk_edit new_report send_report report_sent]
 
   def show
@@ -347,6 +350,19 @@ def find_by_full_identifier
     end
   end
 
+  # So we can include things and solve N + 1 queries
+  def find_item
+    @item = Item.includes([
+                            { item_agents: %i[agent_role user] },
+                            { item_admins: %i[agent_role user] },
+                            :collection,
+                            :essences,
+                            :item_countries,
+                            :item_subject_languages,
+                            :item_content_languages
+                          ]).find_by(identifier: params[:id])
+  end
+
   def save_item_catalog_file(item)
     return if item.nil?
 
@@ -398,9 +414,12 @@ def build_advanced_search
 
   def build_deletable_params(item, items)
     item.bulk_deleteable[:countries] = bulk_deletable_relation(ItemCountry, Country, :country_id, items)
-    item.bulk_deleteable[:subject_languages] = bulk_deletable_relation(ItemSubjectLanguage, Language, :language_id, items)
-    item.bulk_deleteable[:content_languages] = bulk_deletable_relation(ItemContentLanguage, Language, :language_id, items)
-    item.bulk_deleteable[:data_categories] = bulk_deletable_relation(ItemDataCategory, DataCategory, :data_category_id, items)
+    item.bulk_deleteable[:subject_languages] =
+      bulk_deletable_relation(ItemSubjectLanguage, Language, :language_id, items)
+    item.bulk_deleteable[:content_languages] =
+      bulk_deletable_relation(ItemContentLanguage, Language, :language_id, items)
+    item.bulk_deleteable[:data_categories] =
+      bulk_deletable_relation(ItemDataCategory, DataCategory, :data_category_id, items)
     item.bulk_deleteable[:data_types] = bulk_deletable_relation(ItemDataType, DataType, :data_type_id, items)
   end
 
@@ -431,11 +450,9 @@ def advanced_search_params
   end
 
   def item_params
-    if %w[create update bulk_update].include?(params[:action])
-      tidy_params!
-    end
+    tidy_params! if %w[create update bulk_update].include?(params[:action])
 
-    permitted = params
+    params
       .require(:item)
       .permit(
         :identifier, :title, :external, :url, :description, :region, :collection_id,
@@ -464,7 +481,5 @@ def item_params
         item_agents_attributes: {},
         country_ids: [], subject_language_ids: [], content_language_ids: [], data_category_ids: [], data_type_ids: [], admin_ids: [], user_ids: []
       )
-
-    permitted
   end
 end
diff --git a/app/views/collections/_items.html.haml b/app/views/collections/_items.html.haml
index 6d626b4d..87e23d2d 100644
--- a/app/views/collections/_items.html.haml
+++ b/app/views/collections/_items.html.haml
@@ -48,7 +48,7 @@
                 %td.nowrap{style: 'min-width: 75px;'}
                   - if item.digitised_on
                     = item.digitised_on.strftime('%d/%m/%Y')
-                %td= item.essences.count
+                %td= item.essences.size
               %td{style: 'min-width: 60px;'}
                 = link_to 'View', [item.collection, item], {class: 'minwhitespace'}
                 = link_to 'Edit', edit_collection_item_path(item.collection, item), {class: 'minwhitespace'} if can? :update, item