From a944649938d671e1e5cfa7bd9ab078e1b1f1d3e0 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 13 Jun 2024 11:23:34 +0100 Subject: [PATCH] DRY up new attachment code - Attachment code in Publications, Consultations, and calls for evidence was recently updated to render locally rather than using the pre-rendered HTML from whitehall - We move some of the duplicated code into the Content Item::Attachment concern, and delete the existing code there (which wasn't being used), and provide a utility method to simplify some of the unavoidably repeated code. - Coverage slightly improved (presumable due to the deleted unused method). --- app/presenters/call_for_evidence_presenter.rb | 30 +++------------- app/presenters/consultation_presenter.rb | 36 ++++--------------- app/presenters/content_item/attachments.rb | 16 +++++++-- app/presenters/publication_presenter.rb | 20 ++--------- 4 files changed, 25 insertions(+), 77 deletions(-) diff --git a/app/presenters/call_for_evidence_presenter.rb b/app/presenters/call_for_evidence_presenter.rb index 29799e6e6..9dc1ba8f0 100644 --- a/app/presenters/call_for_evidence_presenter.rb +++ b/app/presenters/call_for_evidence_presenter.rb @@ -1,12 +1,12 @@ class CallForEvidencePresenter < ContentItemPresenter + include ContentItem::Attachments include ContentItem::Body include ContentItem::Metadata include ContentItem::NationalApplicability include ContentItem::Political include ContentItem::Shareable - include ContentItem::TitleAndContext - include ContentItem::Attachments include ContentItem::SinglePageNotificationButton + include ContentItem::TitleAndContext def opening_date_time content_item["details"]["opening_date"] @@ -50,13 +50,12 @@ def outcome_detail # Read the full outcome, top of page def outcome_documents - # content_item["details"]["outcome_documents"]&.join("") - documents.select { |doc| outcome_attachments.include? doc["id"] } + attachments_from(content_item["details"]["outcome_attachments"]) end # Documents, bottom of page def general_documents - documents.select { |doc| featured_attachments.include? doc["id"] } + attachments_from(content_item["details"]["featured_attachments"]) end def attachments_with_details @@ -65,27 +64,6 @@ def attachments_with_details items.select { |doc| doc["accessible"] == false && doc["alternative_format_contact_email"] }.count end - def outcome_attachments - content_item["details"]["outcome_attachments"] - end - - def featured_attachments - content_item["details"]["featured_attachments"] - end - - def documents - # content_item["details"]["documents"]&.join("") - return [] unless content_item["details"]["attachments"] - - docs = content_item["details"]["attachments"].select { |a| !a.key?("locale") || a["locale"] == locale } - docs.each do |doc| - doc["type"] = "html" unless doc["content_type"] - doc["type"] = "external" if doc["attachment_type"] == "external" - doc["preview_url"] = "#{doc['url']}/preview" if doc["preview_url"] - doc["alternative_format_contact_email"] = nil if doc["accessible"] == true - end - end - def held_on_another_website? held_on_another_website_url.present? end diff --git a/app/presenters/consultation_presenter.rb b/app/presenters/consultation_presenter.rb index 775edf9d4..af31a53f6 100644 --- a/app/presenters/consultation_presenter.rb +++ b/app/presenters/consultation_presenter.rb @@ -1,12 +1,12 @@ class ConsultationPresenter < ContentItemPresenter + include ContentItem::Attachments include ContentItem::Body include ContentItem::Metadata include ContentItem::NationalApplicability include ContentItem::Political include ContentItem::Shareable - include ContentItem::TitleAndContext - include ContentItem::Attachments include ContentItem::SinglePageNotificationButton + include ContentItem::TitleAndContext def opening_date_time content_item["details"]["opening_date"] @@ -54,17 +54,17 @@ def final_outcome_detail # Read the full outcome, top of page def final_outcome_attachments_for_components - documents.select { |doc| final_outcome_attachments.include? doc["id"] } + attachments_from(content_item["details"]["final_outcome_attachments"]) end # Feedback received, middle of page def public_feedback_attachments_for_components - documents.select { |doc| public_feedback_attachments.include? doc["id"] } + attachments_from(content_item["details"]["public_feedback_attachments"]) end # Documents, bottom of page def documents_attachments_for_components - documents.select { |doc| featured_attachments.include? doc["id"] } + attachments_from(content_item["details"]["featured_attachments"]) end def attachments_with_details @@ -74,30 +74,6 @@ def attachments_with_details items.select { |doc| doc["accessible"] == false && doc["alternative_format_contact_email"] }.count end - def documents - return [] unless content_item["details"]["attachments"] - - docs = content_item["details"]["attachments"].select { |a| !a.key?("locale") || a["locale"] == locale } - docs.each do |doc| - doc["type"] = "html" unless doc["content_type"] - doc["type"] = "external" if doc["attachment_type"] == "external" - doc["preview_url"] = "#{doc['url']}/preview" if doc["preview_url"] - doc["alternative_format_contact_email"] = nil if doc["accessible"] == true - end - end - - def final_outcome_attachments - content_item["details"]["final_outcome_attachments"] || [] - end - - def public_feedback_attachments - content_item["details"]["public_feedback_attachments"] || [] - end - - def featured_attachments - content_item["details"]["featured_attachments"] || [] - end - def public_feedback_detail content_item["details"]["public_feedback_detail"] end @@ -135,7 +111,7 @@ def attachment_url end def add_margin? - final_outcome? || public_feedback_detail || public_feedback_attachments.any? + final_outcome? || public_feedback_detail || public_feedback_attachments_for_components.any? end private diff --git a/app/presenters/content_item/attachments.rb b/app/presenters/content_item/attachments.rb index ca26c41cc..68a23e631 100644 --- a/app/presenters/content_item/attachments.rb +++ b/app/presenters/content_item/attachments.rb @@ -1,9 +1,19 @@ module ContentItem module Attachments - def attachment_details(attachment_id) - content_item.dig("details", "attachments")&.find do |attachment| - attachment["id"] == attachment_id + def attachments + return [] unless content_item["details"]["attachments"] + + docs = content_item["details"]["attachments"].select { |a| !a.key?("locale") || a["locale"] == locale } + docs.each do |doc| + doc["type"] = "html" unless doc["content_type"] + doc["type"] = "external" if doc["attachment_type"] == "external" + doc["preview_url"] = "#{doc['url']}/preview" if doc["preview_url"] + doc["alternative_format_contact_email"] = nil if doc["accessible"] == true end end + + def attachments_from(attachment_id_list) + attachments.select { |doc| (attachment_id_list || []).include? doc["id"] } + end end end diff --git a/app/presenters/publication_presenter.rb b/app/presenters/publication_presenter.rb index 3bf58ee0b..a1595f9a8 100644 --- a/app/presenters/publication_presenter.rb +++ b/app/presenters/publication_presenter.rb @@ -1,9 +1,9 @@ class PublicationPresenter < ContentItemPresenter + include ContentItem::Attachments include ContentItem::Metadata include ContentItem::NationalApplicability include ContentItem::NationalStatisticsLogo include ContentItem::Political - include ContentItem::Attachments include ContentItem::SinglePageNotificationButton def details @@ -11,29 +11,13 @@ def details end def attachments_for_components - documents.select { |doc| featured_attachments.include? doc["id"] } + attachments_from(content_item["details"]["featured_attachments"]) end def attachments_with_details attachments_for_components.select { |doc| doc["accessible"] == false && doc["alternative_format_contact_email"] }.count end - def documents - return [] unless content_item["details"]["attachments"] - - docs = content_item["details"]["attachments"].select { |a| !a.key?("locale") || a["locale"] == locale } - docs.each do |doc| - doc["type"] = "html" unless doc["content_type"] - doc["type"] = "external" if doc["attachment_type"] == "external" - doc["preview_url"] = "#{doc['url']}/preview" if doc["preview_url"] - doc["alternative_format_contact_email"] = nil if doc["accessible"] == true - end - end - - def featured_attachments - content_item["details"]["featured_attachments"].to_a - end - def national_statistics? document_type == "national_statistics" end