Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] [DO NOT MERGE] Add tracking to details elements in attachments on form pages #3048

Closed
wants to merge 1 commit into from
+114 −36
Closed

[WIP] [DO NOT MERGE] Add tracking to details elements in attachments on form pages #3048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/presenters/publication_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ def details
end

def documents
content_item["details"]["documents"].to_a.join("")
docs = content_item["details"]["attachments"].select { |a| a["locale"] == locale }
docs.each do |doc|
doc.delete("alternative_format_contact_email") if doc["accessible"] == true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this about? Maybe this change belongs in its own commit with an explanation in the commit message.

Minor style comment: I find it a bit confusing that the documents method returns a subset of["details"]["attachments"]. I'd expect this code to follow the pattern set on line 21. Is there a reason we need to call this method documents? If not could we have an attachments method, and a filtered_attachments method (that contains the filtering logic), and is called in the templates? Maybe that's not possible if partials are being shared...

end
docs
end

def featured_attachments
Expand Down
35 changes: 23 additions & 12 deletions app/views/content_items/_attachments.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,37 @@
<% attachments ||= [] %>
<% attachment_details = attachments.filter_map { |id| @content_item&.attachment_details(id) } %>
<% if legacy_pre_rendered_documents.present? || attachment_details.any? %>
<%
attachments ||= nil
attachments_html ||= nil
featured_attachments ||= []
attachment_details = featured_attachments.filter_map { |id| @content_item&.attachment_details(id) }
%>
<% if attachments || attachments_html %>
<section id="<%= title.parameterize %>">
<%= render 'govuk_publishing_components/components/heading',
text: title,
mobile_top_margin: true %>

<% if legacy_pre_rendered_documents.present? %>
<% add_gem_component_stylesheet("details") %>
<% if attachments %>
<% attachments.each do |details| %>
<%= render 'govuk_publishing_components/components/attachment', {
heading_level: 3,
attachment: details,
margin_bottom: 6
} %>
<% end %>
<% elsif attachments_html %>
<%= render 'govuk_publishing_components/components/govspeak', {
direction: page_text_direction,
} do %>
<% add_gem_component_stylesheet("details") if legacy_pre_rendered_documents.include? "govuk\-details" %>
<%= raw(legacy_pre_rendered_documents) %>
<%= raw(attachments_html) %>
<% end %>
<% else %>
<% attachment_details.each do |details| %>
<div class="govuk-!-static-margin-bottom-6">
<%= render 'govuk_publishing_components/components/attachment', {
heading_level: 3,
attachment: details
} %>
</div>
<%= render 'govuk_publishing_components/components/attachment', {
heading_level: 3,
attachment: details,
margin_bottom: 6
} %>
<% end %>
<% end %>
</section>
Expand Down
8 changes: 4 additions & 4 deletions app/views/content_items/call_for_evidence.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@

<%= render "attachments",
title: t("call_for_evidence.download_outcome"),
legacy_pre_rendered_documents: @content_item.outcome_documents,
attachments: @content_item.outcome_attachments %>
attachments_html: @content_item.outcome_documents,
featured_attachments: @content_item.outcome_attachments %>

<%= render 'govuk_publishing_components/components/heading', text: t("call_for_evidence.detail_of_outcome"), mobile_top_margin: true %>
<div class="call-for-evidence-outcome-detail">
Expand Down Expand Up @@ -118,8 +118,8 @@

<%= render "attachments",
title: t("call_for_evidence.documents"),
legacy_pre_rendered_documents: @content_item.documents,
attachments: @content_item.featured_attachments %>
attachments_html: @content_item.documents,
featured_attachments: @content_item.featured_attachments %>
</div>

<% if @content_item.ways_to_respond? %>
Expand Down
12 changes: 6 additions & 6 deletions app/views/content_items/consultation.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@

<%= render "attachments",
title: t("consultation.download_outcome"),
legacy_pre_rendered_documents: @content_item.final_outcome_documents,
attachments: @content_item.final_outcome_attachments %>
attachments_html: @content_item.final_outcome_documents,
featured_attachments: @content_item.final_outcome_attachments %>

<%= render 'govuk_publishing_components/components/heading', text: t("consultation.detail_of_outcome"), mobile_top_margin: true %>
<div class="consultation-outcome-detail">
Expand All @@ -70,8 +70,8 @@

<%= render "attachments",
title: t("consultation.feedback_received"),
legacy_pre_rendered_documents: @content_item.public_feedback_documents,
attachments: @content_item.public_feedback_attachments %>
attachments_html: @content_item.public_feedback_documents,
featured_attachments: @content_item.public_feedback_attachments %>

<% if @content_item.public_feedback_detail %>
<%= render 'govuk_publishing_components/components/heading', {
Expand Down Expand Up @@ -148,8 +148,8 @@

<%= render "attachments",
title: t("consultation.documents"),
legacy_pre_rendered_documents: @content_item.documents,
attachments: @content_item.featured_attachments %>
attachments_html: @content_item.documents,
featured_attachments: @content_item.featured_attachments %>
</div>

<% if @content_item.ways_to_respond? %>
Expand Down
4 changes: 2 additions & 2 deletions app/views/content_items/publication.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@
<div class="responsive-bottom-margin">
<%= render "attachments",
title: t("publication.documents", count: 5), # This should always be pluralised.
legacy_pre_rendered_documents: @content_item.documents,
attachments: @content_item.featured_attachments
attachments: @content_item.documents,
featured_attachments: @content_item.featured_attachments
%>

<section id="details">
Expand Down
2 changes: 1 addition & 1 deletion test/presenters/publication_presenter_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def schema_name
assert_equal schema_item["schema_name"], presented_item.schema_name
assert_equal schema_item["title"], presented_item.title
assert_equal schema_item["details"]["body"], presented_item.details
assert_equal schema_item["details"]["documents"].join(""), presented_item.documents
assert_equal schema_item["details"]["documents"], presented_item.documents_attachments
end

test "#published returns a formatted date of the day the content item became public" do
Expand Down
83 changes: 73 additions & 10 deletions test/views/content_items/attachments.html.erb_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,56 @@
require "test_helper"

class ContentItemsAttachmentsTest < ActionView::TestCase
test "it shows pre-rendered attachments by default" do
def test_data
{
accessible: true,
attachment_type: "file",
command_paper_number: "",
content_type: "application/pdf",
file_size: "1570742",
filename: "aa1-interactive-claim-form.pdf",
hoc_paper_number: "",
id: "7556005",
isbn: "",
locale: "en",
number_of_pages: 30,
title: "Attendance Allowance claim form",
unique_reference: "AA1",
unnumbered_command_paper: false,
unnumbered_hoc_paper: false,
url: "https://assets.publishing.service.gov.uk/media/64132b818fa8f55576ac627e/aa1-interactive-claim-form.pdf",
}
end

test "it can display pre-rendered attachments" do
render(
partial: "content_items/attachments",
locals: { title: "Documents",
legacy_pre_rendered_documents: "some html" },
locals: {
title: "Documents",
attachments_html: "some html",
},
)

assert_includes rendered, "gem-c-govspeak"
assert_includes rendered, "some html"
end

test "it can render an array of attachments" do
render(
partial: "content_items/attachments",
locals: {
title: "Documents",
attachments: [
test_data,
],
},
)

assert_includes rendered, "gem-c-attachment"
assert_includes rendered, "govuk-link", text: "Attendance Allowance claim form"
assert_not_includes rendered, "gem-c-details"
end

test "can render attachments using their metadata" do
@content_item = PublicationPresenter.new(
{ "details" => { "attachments" => [{ "id" => "attachment_id",
Expand All @@ -24,8 +63,8 @@ class ContentItemsAttachmentsTest < ActionView::TestCase
render(
partial: "content_items/attachments",
locals: { title: "Documents",
legacy_pre_rendered_documents: "",
attachments: %w[attachment_id] },
attachments: "",
featured_attachments: %w[attachment_id] },
)

assert_includes rendered, "gem-c-attachment"
Expand All @@ -35,12 +74,36 @@ class ContentItemsAttachmentsTest < ActionView::TestCase
test "it prioritises pre-rendered attachments" do
render(
partial: "content_items/attachments",
locals: { title: "Documents",
legacy_pre_rendered_documents: "some html",
attachments: %w[attachment_id] },
locals: {
title: "Documents",
attachments: [
test_data,
],
featured_attachments: %w[attachment_id],
},
)

assert_includes rendered, "gem-c-govspeak"
assert_includes rendered, "some html"
assert_includes rendered, "gem-c-attachment"
assert_includes rendered, "govuk-link", text: "Attendance Allowance claim form"
end

test "it shows extra information for inaccessible attachments" do
data = test_data
data[:accessible] = false
data[:alternative_format_contact_email] = "accessible.formats@dwp.gov.uk"
render(
partial: "content_items/attachments",
locals: {
title: "Documents",
attachments: [
data,
],
},
)

assert_includes rendered, "gem-c-attachment"
assert_includes rendered, "govuk-link", text: "Attendance Allowance claim form"
assert_includes rendered, "gem-c-attachment__metadata", text: "This file may not be suitable for users of assistive technology."
assert_includes rendered, "govuk-details__summary-text", text: "Request an accessible format."
end
end
Loading