Merge pull request #2989 from alphagov/content-modelling/679-get-the-…

…number-of-instances-on-a-page

(679) Get the number of instances on a page

app/controllers/v2/content_items_controller.rb

@@ -22,7 +22,7 @@ def linkables
     end
 
     def embedded
-      results = GetHostContentService.new(
+      results = GetEmbeddedContentService.new(
         path_params[:content_id],
         query_params[:order],
         query_params[:page],

app/presenters/embedded_content_presenter.rb

@@ -32,6 +32,7 @@ def results
           last_edited_by_editor_id: edition.last_edited_by_editor_id,
           last_edited_at: edition.last_edited_at,
           unique_pageviews: edition.unique_pageviews,
+          instances: edition.instances,
           primary_publishing_organisation: {
             content_id: edition.primary_publishing_organisation_content_id,
             title: edition.primary_publishing_organisation_title,

app/queries/get_embedded_content.rb

@@ -12,6 +12,7 @@ class GetEmbeddedContent
       :primary_publishing_organisation_title,
       :primary_publishing_organisation_base_path,
       :unique_pageviews,
+      :instances,
     )
 
     DEFAULT_PER_PAGE = 10
@@ -27,17 +28,18 @@ class GetEmbeddedContent
     }.freeze
 
     FIELDS = [
-      TABLES[:editions][:id],
-      TABLES[:editions][:title],
-      TABLES[:editions][:base_path],
-      TABLES[:editions][:document_type],
-      TABLES[:editions][:publishing_app],
-      TABLES[:editions][:last_edited_by_editor_id],
-      TABLES[:editions][:last_edited_at],
-      TABLES[:primary_links][:target_content_id].as("primary_publishing_organisation_content_id"),
-      TABLES[:org_editions][:title].as("primary_publishing_organisation_title"),
-      TABLES[:org_editions][:base_path].as("primary_publishing_organisation_base_path"),
-      TABLES[:statistics_caches][:unique_pageviews],
+      { field: TABLES[:editions][:id], included_in_group?: true },
+      { field: TABLES[:editions][:title], included_in_group?: true },
+      { field: TABLES[:editions][:base_path], included_in_group?: true },
+      { field: TABLES[:editions][:document_type], included_in_group?: true },
+      { field: TABLES[:editions][:publishing_app], included_in_group?: true },
+      { field: TABLES[:editions][:last_edited_by_editor_id], included_in_group?: true },
+      { field: TABLES[:editions][:last_edited_at], included_in_group?: true },
+      { field: TABLES[:primary_links][:target_content_id], alias: "primary_publishing_organisation_content_id", included_in_group?: true },
+      { field: TABLES[:org_editions][:title], alias: "primary_publishing_organisation_title", included_in_group?: true },
+      { field: TABLES[:org_editions][:base_path], alias: "primary_publishing_organisation_base_path", included_in_group?: true },
+      { field: TABLES[:statistics_caches][:unique_pageviews], included_in_group?: true },
+      { field: TABLES[:editions][:id].count, alias: "instances", included_in_group?: false },
     ].freeze
 
     ORDER_FIELDS = {
@@ -46,6 +48,7 @@ class GetEmbeddedContent
       unique_pageviews: TABLES[:statistics_caches][:unique_pageviews],
       primary_publishing_organisation_title: TABLES[:org_editions][:title],
       last_edited_at: TABLES[:editions][:last_edited_at],
+      instances: TABLES[:editions][:id].count,
     }.freeze
 
     ORDER_DIRECTIONS = %i[asc desc].freeze
@@ -94,9 +97,17 @@ def count_query
       "SELECT COUNT(*) FROM (#{arel_query.to_sql}) AS full_query"
     end
 
+    def select_fields
+      FIELDS.map { |f| f[:alias].present? ? f[:field].as(f[:alias]) : f[:field] }
+    end
+
+    def group_fields
+      FIELDS.select { |f| f[:included_in_group?] }.pluck(:field)
+    end
+
     def arel_joins
       TABLES[:editions]
-        .project(FIELDS)
+        .project(select_fields)
         .join(TABLES[:links]).on(
           TABLES[:links][:edition_id].eq(TABLES[:editions][:id]),
         )
@@ -117,6 +128,7 @@ def arel_joins
         .join(TABLES[:statistics_caches], Arel::Nodes::OuterJoin).on(
           TABLES[:statistics_caches][:document_id].eq(TABLES[:documents][:id]),
         )
+        .group(group_fields)
     end
 
     def embedded_link_type

app/services/embedded_content_finder_service.rb

@@ -8,20 +8,20 @@ class EmbeddedContentFinderService
   def fetch_linked_content_ids(details, locale)
     content_references = details.values.map { |value|
       find_content_references(value)
-    }.flatten.compact.uniq
+    }.flatten.compact
 
-    check_all_references_exist(content_references, locale)
+    check_all_references_exist(content_references.uniq, locale)
     content_references.map(&:content_id)
   end
 
   def find_content_references(value)
     case value
     when Array
-      value.map { |item| find_content_references(item) }.flatten
+      value.map { |item| find_content_references(item) }.flatten.uniq
     when Hash
-      value.map { |_, v| find_content_references(v) }.flatten
+      value.map { |_, v| find_content_references(v) }.flatten.uniq
     when String
-      value.scan(EMBED_REGEX).map { |match| ContentReference.new(document_type: match[1], content_id: match[2], embed_code: match[0]) }.uniq
+      value.scan(EMBED_REGEX).map { |match| ContentReference.new(document_type: match[1], content_id: match[2], embed_code: match[0]) }
     else
       []
     end

app/services/get_host_content_service.rb → app/services/get_embedded_content_service.rb

@@ -1,4 +1,4 @@
-class GetHostContentService
+class GetEmbeddedContentService
   def initialize(target_content_id, order, page, per_page)
     @target_content_id = target_content_id
     @order = order

spec/integration/embedded_content_spec.rb

@@ -1,4 +1,6 @@
 RSpec.describe "Embedded documents" do
+  include_context "PutContent call"
+
   let!(:publishing_organisation) do
     create(:live_edition,
            title: "bar",
@@ -43,16 +45,12 @@
   context "when an edition embeds a reference to the content block" do
     it "returns details of the edition and its publishing organisation in the results" do
       last_edited_at = "2023-01-01T08:00:00.000Z"
-      host_edition = create(:live_edition,
-                            publishing_app: "whitehall",
-                            last_edited_at: Time.zone.parse(last_edited_at),
-                            details: {
-                              "body" => "<p>{{embed:email_address:#{content_block.content_id}}}</p>\n",
-                            },
-                            links_hash: {
-                              primary_publishing_organisation: [publishing_organisation.content_id],
-                              embed: [content_block.content_id],
-                            })
+      host_edition = Timecop.freeze Time.zone.parse(last_edited_at) do
+        create_live_edition(
+          body: "<p>{{embed:content_block_email_address:#{content_block.content_id}}}</p>\n",
+          primary_publishing_organisation_uuid: publishing_organisation.content_id,
+        )
+      end
 
       statistics_cache = create(:statistics_cache, document: host_edition.document, unique_pageviews: 333)
 
@@ -73,6 +71,7 @@
           "last_edited_by_editor_id" => host_edition.last_edited_by_editor_id,
           "last_edited_at" => last_edited_at,
           "unique_pageviews" => statistics_cache.unique_pageviews,
+          "instances" => 1,
           "primary_publishing_organisation" => {
             "content_id" => publishing_organisation.content_id,
             "title" => publishing_organisation.title,
@@ -81,6 +80,43 @@
         },
       )
     end
+
+    context "when embedded content appears more than once in a field" do
+      let!(:host_edition) do
+        create_live_edition(
+          body: "<p>{{embed:content_block_email_address:#{content_block.content_id}}} {{embed:content_block_email_address:#{content_block.content_id}}}</p>\n",
+        )
+      end
+
+      it "should return multiple instances" do
+        get "/v2/content/#{content_block.content_id}/embedded"
+        response_body = parsed_response
+
+        expect(response_body["content_id"]).to eq(content_block.content_id)
+        expect(response_body["total"]).to eq(1)
+
+        expect(response_body["results"][0]["instances"]).to eq(2)
+      end
+
+      context "when the host edition is changed to reference the content once" do
+        before do
+          create_live_edition(
+            content_id: host_edition.content_id,
+            body: "<p>{{embed:content_block_email_address:#{content_block.content_id}}}</p>\n",
+          )
+        end
+
+        it "should return only one instance" do
+          get "/v2/content/#{content_block.content_id}/embedded"
+          response_body = parsed_response
+
+          expect(response_body["content_id"]).to eq(content_block.content_id)
+          expect(response_body["total"]).to eq(1)
+
+          expect(response_body["results"][0]["instances"]).to eq(1)
+        end
+      end
+    end
   end
 
   context "pagination" do
@@ -275,4 +311,27 @@ def expect_request_to_order_by(order_argument:, expected_results:)
       end
     end
   end
+
+private
+
+  def create_live_edition(content_id: SecureRandom.uuid, body: "Some body goes here", primary_publishing_organisation_uuid: nil)
+    payload.merge!(
+      document_type: "press_release",
+      schema_name: "news_article",
+      details: { body: },
+    )
+
+    if primary_publishing_organisation_uuid.present?
+      payload.merge!(
+        links: {
+          primary_publishing_organisation: [primary_publishing_organisation_uuid],
+        },
+      )
+    end
+
+    put "/v2/content/#{content_id}", params: payload.to_json
+    post "/v2/content/#{content_id}/publish", params: {}.to_json
+
+    Document.find_by(content_id:).live
+  end
 end

spec/presenters/embedded_content_presenter_spec.rb

@@ -19,7 +19,8 @@
               unique_pageviews: 123,
               primary_publishing_organisation_content_id: organisation_edition_id,
               primary_publishing_organisation_title: "bar",
-              primary_publishing_organisation_base_path: "/bar")]
+              primary_publishing_organisation_base_path: "/bar",
+              instances: 1)]
     end
 
     let(:result) { described_class.present(target_edition_id, host_editions, total, total_pages) }
@@ -38,6 +39,7 @@
             last_edited_by_editor_id:,
             last_edited_at:,
             unique_pageviews: 123,
+            instances: 1,
             primary_publishing_organisation: {
               content_id: organisation_edition_id,
               title: "bar",

spec/queries/get_embedded_content_spec.rb

@@ -37,6 +37,8 @@
     end
 
     context "when there are live and draft editions that embed the target content" do
+      let(:target_content_id) { content_block.content_id }
+
       it "returns the live editions" do
         target_content_id = content_block.content_id
         published_host_editions = create_list(:live_edition, 2,
@@ -84,8 +86,25 @@
           expect(results[i].primary_publishing_organisation_title).to eq(organisation.title)
           expect(results[i].primary_publishing_organisation_base_path).to eq(organisation.base_path)
           expect(results[i].unique_pageviews).to eq(expected_pageviews[host_edition.document.id])
+          expect(results[i].instances).to eq(1)
         end
       end
+
+      it "returns instance counts when the content is embedded more than once" do
+        create(:live_edition,
+               details: {
+                 body: "<p>{{embed:email_address:#{target_content_id}}}</p>\n",
+               },
+               links_hash: {
+                 primary_publishing_organisation: [organisation.content_id],
+                 embed: [target_content_id, target_content_id],
+               },
+               publishing_app: "example-app")
+
+        results = described_class.new(target_content_id).call
+
+        expect(results[0].instances).to eq(2)
+      end
     end
 
     context "when there are superseded editions that embed the target content" do
@@ -147,8 +166,7 @@ def expect_sort_call_for(order_field:, order_direction:)
         expect(ActiveRecord::Base.connection).to receive(:select_all) { |arel_query|
           expect(arel_query.orders.length).to eq(1)
           expect(arel_query.orders[0]).to be_a(order_direction == :asc ? Arel::Nodes::Ascending : Arel::Nodes::Descending)
-          expect(arel_query.orders[0].expr.relation.name).to eq(order_field.relation.name)
-          expect(arel_query.orders[0].expr.name).to eq(order_field.name)
+          expect(arel_query.orders[0].expr).to eq(order_field)
         }.and_return([])
       end
     end

spec/services/embedded_content_finder_service_spec.rb

@@ -32,6 +32,14 @@
         expect(links).to eq([editions[0].content_id, editions[1].content_id])
       end
 
+      it "returns duplicates when there is more than one content reference in the field" do
+        details = { field_name => "{{embed:#{document_type}:#{editions[0].content_id}}} {{embed:#{document_type}:#{editions[0].content_id}}} {{embed:#{document_type}:#{editions[1].content_id}}}" }
+
+        links = EmbeddedContentFinderService.new.fetch_linked_content_ids(details, Edition::DEFAULT_LOCALE)
+
+        expect(links).to eq([editions[0].content_id, editions[0].content_id, editions[1].content_id])
+      end
+
       it "finds content references when #{field_name} is an array of hashes" do
         details = { field_name => [{ "content" => "{{embed:#{document_type}:#{editions[0].content_id}}} {{embed:#{document_type}:#{editions[1].content_id}}}" }] }
 
@@ -45,6 +53,10 @@
           field_name => [
             {
               body: [
+                {
+                  content: "some string with a reference: {{embed:#{document_type}:#{editions[0].content_id}}}",
+                  content_type: "text/html",
+                },
                 {
                   content: "some string with a reference: {{embed:#{document_type}:#{editions[0].content_id}}}",
                   content_type: "text/govspeak",
@@ -55,6 +67,10 @@
             },
             {
               body: [
+                {
+                  content: "some string with another reference: {{embed:#{document_type}:#{editions[1].content_id}}}",
+                  content_type: "text/html",
+                },
                 {
                   content: "some string with another reference: {{embed:#{document_type}:#{editions[1].content_id}}}",
                   content_type: "text/govspeak",

spec/services/get_host_content_service_spec.rb → spec/services/get_embedded_content_service_spec.rb

@@ -1,4 +1,4 @@
-RSpec.describe GetHostContentService do
+RSpec.describe GetEmbeddedContentService do
   describe "#call" do
     let(:organisation) do
       edition_params = {