Skip to content

Commit

Permalink
Ensure duplicate links are retained in array/hash
Browse files Browse the repository at this point in the history
When a document is part of an array/hash (such as Mainstream documents),
we were still calling `.uniq` after going through each item. This meant
that documents such as Answers, Guides etc weren’t getting the correct
metadata, as any duplicate links were getting thrown away.

To get around this, we call `.uniq` BEFORE calling `.flatten`. This
ensures that we retain the number of instances without double counting
for multipart content.
  • Loading branch information
pezholio committed Dec 10, 2024
1 parent 7c5f675 commit ba0d525
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 2 deletions.
4 changes: 2 additions & 2 deletions app/services/embedded_content_finder_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ def fetch_linked_content_ids(details, locale)
def find_content_references(value)
case value
when Array
value.map { |item| find_content_references(item) }.flatten.uniq
value.map { |item| find_content_references(item) }.uniq.flatten
when Hash
value.map { |_, v| find_content_references(v) }.flatten.uniq
value.map { |_, v| find_content_references(v) }.uniq.flatten
when String
ContentBlockTools::ContentBlockReference.find_all_in_document(value)
else
Expand Down
46 changes: 46 additions & 0 deletions spec/services/embedded_content_finder_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,44 @@
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 and #{field_name} is a multipart document" do
details = {
field_name => [
{
body: [
{
content: "some string with a reference: {{embed:#{document_type}:#{editions[0].content_id}}} {{embed:#{document_type}:#{editions[0].content_id}}}",
content_type: "text/html",
},
{
content: "some string with a reference: {{embed:#{document_type}:#{editions[0].content_id}}} {{embed:#{document_type}:#{editions[0].content_id}}}",
content_type: "text/govspeak",
},
],
slug: "some-slug",
title: "Some title",
},
{
body: [
{
content: "some string with another reference: {{embed:#{document_type}:#{editions[1].content_id}}} {{embed:#{document_type}:#{editions[1].content_id}}}",
content_type: "text/html",
},
{
content: "some string with another reference: {{embed:#{document_type}:#{editions[1].content_id}}} {{embed:#{document_type}:#{editions[1].content_id}}}",
content_type: "text/govspeak",
},
],
slug: "some-other-slug",
title: "Some other title",
},
],
}
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, editions[1].content_id])
end

it "finds content references when the field is a hash" do
details = { field_name => { title: "{{embed:#{document_type}:#{editions[0].content_id}}}", slug: "{{embed:#{document_type}:#{editions[1].content_id}}}", current: true } }

Expand All @@ -94,6 +132,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 and the field is a hash" do
details = { field_name => { title: "{{embed:#{document_type}:#{editions[0].content_id}}} {{embed:#{document_type}:#{editions[0].content_id}}}", slug: "{{embed:#{document_type}:#{editions[1].content_id}}}", current: true } }

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 "errors when given a content ID that is still draft" do
details = { field_name => "{{embed:#{document_type}:#{draft_edition.content_id}}}" }

Expand Down

0 comments on commit ba0d525

Please sign in to comment.