From f0235728602f9322539f03ef36701953e7207388 Mon Sep 17 00:00:00 2001 From: Kevin Dew Date: Mon, 8 Mar 2021 11:26:14 +0000 Subject: [PATCH 1/2] Actually update document_type in test_helper By running `merge` rather than `merge!` this wasn't actually mutating the underlying hash and thus not having an effect. This helps to resolve a problem where a new publication document type has been added and causes some tests to fail. For example: ``` test_random_but_valid_items_do_not_error ERROR (0.39s) Minitest::UnexpectedError: ActionView::Template::Error: translation missing: zh.content_item.schema_name.standard app/views/content_items/publication.html.erb:10 app/controllers/content_items_controller.rb:149:in `block in render_template' app/controllers/content_items_controller.rb:183:in `block in with_locale' app/controllers/content_items_controller.rb:183:in `with_locale' app/controllers/content_items_controller.rb:148:in `render_template' app/controllers/content_items_controller.rb:25:in `show' ``` --- test/test_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index 4753e3b94..c224d90af 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -173,7 +173,7 @@ def setup_and_visit_content_item_with_taxons(name, taxons) def setup_and_visit_random_content_item(document_type: nil) content_item = GovukSchemas::RandomExample.for_schema(frontend_schema: schema_type) do |payload| - payload.merge("document_type" => document_type) unless document_type.nil? + payload.merge!("document_type" => document_type) unless document_type.nil? payload end From 37b8b4bf079ab48ba3c7d1b525bb5170c08bd769 Mon Sep 17 00:00:00 2001 From: Kevin Dew Date: Mon, 8 Mar 2021 11:59:30 +0000 Subject: [PATCH 2/2] Remove assumptions of attachment existence This resolves a couple of scenarios where assumptions are made that documents will have attachments and attachments with particular ids exist. This is data that developers assume is the case (and most likely is) but is beyond the guarantees that schemas make. This mismatch can cause tests to fail when valid schemas are generated which cannot be rendered. --- app/presenters/content_item/attachments.rb | 2 +- app/views/content_items/_attachments.html.erb | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/presenters/content_item/attachments.rb b/app/presenters/content_item/attachments.rb index 628e8d625..ca26c41cc 100644 --- a/app/presenters/content_item/attachments.rb +++ b/app/presenters/content_item/attachments.rb @@ -1,7 +1,7 @@ module ContentItem module Attachments def attachment_details(attachment_id) - content_item["details"]["attachments"].find do |attachment| + content_item.dig("details", "attachments")&.find do |attachment| attachment["id"] == attachment_id end end diff --git a/app/views/content_items/_attachments.html.erb b/app/views/content_items/_attachments.html.erb index f16792a73..7ffcd8e65 100644 --- a/app/views/content_items/_attachments.html.erb +++ b/app/views/content_items/_attachments.html.erb @@ -1,4 +1,6 @@ -<% if legacy_pre_rendered_documents.present? || attachments.any? %> +<% attachments ||= [] %> +<% attachment_details = attachments.filter_map { |id| @content_item&.attachment_details(id) } %> +<% if legacy_pre_rendered_documents.present? || attachment_details.any? %>
<%= render 'govuk_publishing_components/components/heading', text: title, @@ -11,15 +13,14 @@ <%= raw(legacy_pre_rendered_documents) %> <% end %> <% else %> - <% attachments.each do |attachment_id| %> + <% attachment_details.each do |details| %>
<%= render 'govuk_publishing_components/components/attachment', { heading_level: 3, - attachment: @content_item.attachment_details(attachment_id) + attachment: details } %>
<% end %> <% end %> -
<% end %>