From c1ddce6dc7742007c7606aee5a7129eb350664d3 Mon Sep 17 00:00:00 2001
From: Thomas von Deyen <vondeyen@blish.cloud>
Date: Mon, 3 Aug 2026 17:38:19 +0200
Subject: [PATCH] refactor: route element visibility through ElementsRepository
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The JSON:API page decorator filtered visible elements itself before
handing them to the repository, which left the `public?` iteration
duplicated outside the abstraction that is meant to own it. Routing the
filter through `ElementsRepository#visible` is behavior identical today
— the repository calls `to_a` on the already loaded association and runs
the same Ruby side select, so the controller's eager load survives — but
it makes the repository the single choke point, so it can later own the
preloading needed for visibility as well.

Because that eager load is easy to break by accident (applying the
`published` scope to the loaded association makes Rails re-query per
page and drop the nested element and ingredient includes), the index
request spec now asserts that the number of element load queries does
not grow with the number of pages.
---
 app/models/alchemy/json_api/page.rb          |  3 +-
 spec/requests/alchemy/json_api/pages_spec.rb | 42 ++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/app/models/alchemy/json_api/page.rb b/app/models/alchemy/json_api/page.rb
index 3e3eb777..ae3bd232 100644
--- a/app/models/alchemy/json_api/page.rb
+++ b/app/models/alchemy/json_api/page.rb
@@ -45,8 +45,7 @@ def ancestor_ids
       def element_repository
         return Alchemy::ElementsRepository.none unless page_version
 
-        # Need to use select here, otherwise rails would not eager load the elements correctly
-        Alchemy::ElementsRepository.new(page_version.elements.select(&:public))
+        Alchemy::ElementsRepository.new(page_version.elements).visible
       end
     end
   end
diff --git a/spec/requests/alchemy/json_api/pages_spec.rb b/spec/requests/alchemy/json_api/pages_spec.rb
index 85339288..624024cd 100644
--- a/spec/requests/alchemy/json_api/pages_spec.rb
+++ b/spec/requests/alchemy/json_api/pages_spec.rb
@@ -401,6 +401,48 @@
       end
     end
 
+    context "with elements" do
+      def create_page_with_element
+        page = FactoryBot.create(:alchemy_page, :public)
+        FactoryBot.create(
+          :alchemy_element,
+          page_version: page.public_version,
+          name: "article",
+          autogenerate_ingredients: true
+        )
+        page
+      end
+
+      # Only counts full record loads, so the id only queries the etag generation
+      # runs for every page do not hide a per page element load.
+      def count_element_load_queries
+        queries = []
+        subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |_name, _start, _finish, _id, payload|
+          next if payload[:name] == "SCHEMA" || payload[:cached]
+          queries << payload[:sql] if payload[:sql].include?(%("alchemy_elements"."name"))
+        end
+        yield
+        queries.size
+      ensure
+        ActiveSupport::Notifications.unsubscribe(subscriber)
+      end
+
+      it "loads elements eager, not once per page" do
+        create_page_with_element
+        queries_for_one_page = count_element_load_queries do
+          get alchemy_json_api.pages_path(include: "all_elements.ingredients")
+        end
+        expect(queries_for_one_page).to be > 0
+
+        2.times { create_page_with_element }
+        queries_for_three_pages = count_element_load_queries do
+          get alchemy_json_api.pages_path(include: "all_elements.ingredients")
+        end
+
+        expect(queries_for_three_pages).to eq(queries_for_one_page)
+      end
+    end
+
     context "with pagination params" do
       before do
         FactoryBot.create_list(:alchemy_page, 3, :public)