Skip to content

Commit

Permalink
Merge pull request #3172 from alphagov/content-modelling/878-debug-em…
Browse files Browse the repository at this point in the history
…bedded-content

content-modelling/Handle incorrect content IDs found in embeds
  • Loading branch information
Harriethw authored Mar 4, 2025
2 parents 141ad74 + c37b524 commit 9d4ce61
Show file tree
Hide file tree
Showing 5 changed files with 264 additions and 211 deletions.
15 changes: 11 additions & 4 deletions app/presenters/content_embed_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,17 @@ def render_embedded_editions(content)
embedded_content_references.uniq.each do |content_reference|
embed_code = content_reference.embed_code
embedded_edition = embedded_editions[content_reference.content_id]
content = content.gsub(
embed_code,
get_content_for_edition(embedded_edition, embed_code),
)
if embedded_edition.present?
content = content.gsub(
embed_code,
get_content_for_edition(embedded_edition, embed_code),
)
else
Sentry.capture_exception(CommandError.new(
code: 422,
message: "Could not find a live edition for embedded content ID: #{content_reference.content_id}",
))
end
end

content
Expand Down
16 changes: 8 additions & 8 deletions app/services/embedded_content_finder_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ def fetch_linked_content_ids(details, locale)
find_content_references(value)
}.flatten.compact

check_all_references_exist(content_references.uniq, locale)
content_references.map(&:content_id)
live_content_ids(content_references, locale)
end

def find_content_references(value)
Expand All @@ -23,16 +22,17 @@ def find_content_references(value)

private

def check_all_references_exist(content_references, locale)
found_editions = live_editions(content_references, locale)
def live_content_ids(content_references, locale)
found_editions = live_editions(content_references.uniq, locale)
not_found_content_ids = content_references.map(&:content_id) - found_editions.map(&:content_id)

if not_found_content_ids.any?
raise CommandError.new(
code: 422,
message: "Could not find any live editions in locale #{locale} for: #{not_found_content_ids.join(', ')}, ",
)
Sentry.capture_exception(CommandError.new(
code: 422,
message: "Could not find any live editions for embedded content IDs: #{not_found_content_ids.join(', ')}",
))
end
content_references.map(&:content_id) - not_found_content_ids
end

def live_editions(content_references, locale)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,11 @@
payload.merge!(document_type: "press_release", schema_name: "news_article", details: { body: "{{embed:contact:#{fake_content_id}}}" })
end

it "should return a 422 error" do
it "should not create a new Link" do
put "/v2/content/#{content_id}", params: payload.to_json

expect(response).to be_unprocessable
expect(response.body).to match(/Could not find any live editions in locale en for: #{fake_content_id}/)
expect(response).to be_ok
expect(Link.find_by(target_content_id: fake_content_id)).to be_nil
end
end

Expand All @@ -330,11 +330,13 @@
payload.merge!(document_type: "press_release", schema_name: "news_article", details: { body: "{{embed:contact:#{contact.document.content_id}}} {{embed:contact:#{first_fake_content_id}}} {{embed:contact:#{second_fake_content_id}}}" })
end

it "should return a 422 error" do
it "should return a 200 with only the existing links" do
put "/v2/content/#{content_id}", params: payload.to_json

expect(response).to be_unprocessable
expect(response.body).to match(/Could not find any live editions in locale en for: #{first_fake_content_id}, #{second_fake_content_id}/)
expect(response).to be_ok
expect(Link.find_by(target_content_id: first_fake_content_id)).to be_nil
expect(Link.find_by(target_content_id: second_fake_content_id)).to be_nil
expect(Link.find_by(target_content_id: contact.document.content_id)).not_to be_nil
end
end

Expand Down
Loading

0 comments on commit 9d4ce61

Please sign in to comment.