Skip to content

Commit

Permalink
Handle incorrect content IDs found in embeds
Browse files Browse the repository at this point in the history
Previously, an uncaught error was raised if a
content block's content ID couldn't be found,
which prevented any editions being published if
they had an incorrect embed code within them.
There was no feedback for the user on Mainstream
when this happened and it looks like the edition
*has* been published, on Whitehall a nondescript
server error is shown to users when attempting to
publish.

When a Host Edition is updated at the content PUT
endpoint and embedded content links are looked
for, this change sends an exception to Sentry to
alert developers the issue has occurred, and does
not add the incorrect ID to the Links on an
Edition.

When the content is then rendered by the
`ContentEmbedPresenter`, we again check whether
the corresponding Edition exists and alert Sentry
again.

The user should be able to see their changes
published, and the embed code will appear as plain
text on the frontend, so it should be easy to spot
and change.
  • Loading branch information
Harriethw committed Mar 4, 2025
1 parent 0fb19ae commit c5cf3e7
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 c5cf3e7

Please sign in to comment.