Skip to content

Commit

Permalink
Fix for csv preview with legacy_url_path
Browse files Browse the repository at this point in the history
Reference issue:https://govuk.sentry.io/issues/4265154142/?alert_rule_id=3654813&alert_type=issue&notification_uuid=8ab09d48-0416-4305-9e3c-0de49561a46a&project=202225&referrer=slack

Currently content item when it returns attachments, it returns modern url
hence when it tries to find  for legacy url, it fails and @attachment_metadata is empty
this cause @attachment_metadata.first to be nil and hence the error.

With the fix we try to not rely on legacy_url_path, instead we look for the filename.
  • Loading branch information
syed-ali-tw committed Sep 29, 2023
1 parent 29f35e0 commit 35c07f7
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
8 changes: 6 additions & 2 deletions app/controllers/csv_preview_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ def show

parent_document_path = URI(@asset["parent_document_url"]).request_uri
@content_item = GdsApi.content_store.content_item(parent_document_path).to_hash

@attachment_metadata = @content_item.dig("details", "attachments").select do |attachment|
attachment["url"] =~ /#{Regexp.escape(asset_path)}$/
attachment["filename"] == asset_filename
end

original_error = nil
row_sep = :auto
download_file = params[:legacy] ? whitehall_media_download : media_download
Expand Down Expand Up @@ -58,6 +58,10 @@ def asset_path
request.path.sub("/preview", "").sub(/^\//, "")
end

def asset_filename
asset_path.split("/").last
end

def whitehall_media_download
GdsApi.asset_manager.whitehall_media(asset_path).body
end
Expand Down
10 changes: 8 additions & 2 deletions test/integration/csv_preview_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ def setup_asset_manager(legacy_url_path, parent_document_url, asset_manager_id =
end

def setup_content_item_legacy(legacy_url_path, parent_document_base_path)
filename = legacy_url_path.split("/").last
content_item = {
base_path: parent_document_base_path,
document_type: "guidance",
Expand All @@ -251,11 +252,13 @@ def setup_content_item_legacy(legacy_url_path, parent_document_base_path)
attachments: [
{
title: "Attachment 1",
url: "https://www.gov.uk/government/uploads/system/uploads/attachment_data/file/5678/filename.csv",
filename: "file.csv",
url: "https://www.gov.uk/government/uploads/system/uploads/attachment_data/file/5678/file.csv",
file_size: "1024",
},
{
title: "Attachment 2",
filename:,
url: "https://www.gov.uk/#{legacy_url_path}",
file_size: "2048",
},
Expand All @@ -280,6 +283,7 @@ def setup_content_item_legacy(legacy_url_path, parent_document_base_path)
end

def setup_content_item_non_legacy(non_legacy_url_path, parent_document_base_path)
filename = non_legacy_url_path.split("/").last
content_item = {
base_path: parent_document_base_path,
document_type: "guidance",
Expand All @@ -288,11 +292,13 @@ def setup_content_item_non_legacy(non_legacy_url_path, parent_document_base_path
attachments: [
{
title: "Attachment 1",
url: "https://www.gov.uk/media/5678/filename.csv",
filename: "file.csv",
url: "https://www.gov.uk/media/5678/file.csv",
file_size: "1024",
},
{
title: "Attachment 2",
filename:,
url: "https://www.gov.uk/#{non_legacy_url_path}",
file_size: "2048",
},
Expand Down

0 comments on commit 35c07f7

Please sign in to comment.