From 4139f42513181b31a0461980f88a2a7b5b09cf92 Mon Sep 17 00:00:00 2001
From: Michael Walker <mike@barrucadu.co.uk>
Date: Mon, 17 Feb 2020 10:50:19 +0000
Subject: [PATCH] Override old "documents" fields with new "attachments" fields

If both are set and the attachments field is the same size as the
documents field.  If they're not the same size an error is logged.  We
can use this to be confident that, after republishing everything,
we've not somehow lost any data.

When we're happy to remove the old field, we can revert this commit
and just use the new field directly.
---
 app/presenters/consultation_presenter.rb      | 16 ++++++--
 .../featured_attachments_migration.rb         | 23 +++++++++++
 app/presenters/publication_presenter.rb       |  6 ++-
 .../featured_attachments_migration.rb         | 41 +++++++++++++++++++
 4 files changed, 82 insertions(+), 4 deletions(-)
 create mode 100644 app/presenters/content_item/featured_attachments_migration.rb
 create mode 100644 test/presenters/content_item/featured_attachments_migration.rb

diff --git a/app/presenters/consultation_presenter.rb b/app/presenters/consultation_presenter.rb
index 26393817a..5acb45a69 100644
--- a/app/presenters/consultation_presenter.rb
+++ b/app/presenters/consultation_presenter.rb
@@ -5,6 +5,7 @@ class ConsultationPresenter < ContentItemPresenter
   include ContentItem::Political
   include ContentItem::Shareable
   include ContentItem::TitleAndContext
+  include ContentItem::FeaturedAttachmentsMigration
 
   def opening_date_time
     content_item["details"]["opening_date"]
@@ -135,14 +136,23 @@ def ways_to_respond
   end
 
   def final_outcome_documents_list
-    content_item["details"]["final_outcome_documents"] || []
+    @final_outcome_documents_list ||= choose_field(
+      new_field_name: "final_outcome_attachments",
+      old_field_name: "final_outcome_documents",
+    )
   end
 
   def public_feedback_documents_list
-    content_item["details"]["public_feedback_documents"] || []
+    @public_feedback_documents_list ||= choose_field(
+      new_field_name: "public_feedback_attachments",
+      old_field_name: "public_feedback_documents",
+    )
   end
 
   def documents_list
-    content_item["details"]["documents"] || []
+    @documents_list ||= choose_field(
+      new_field_name: "featured_attachments",
+      old_field_name: "documents",
+    )
   end
 end
diff --git a/app/presenters/content_item/featured_attachments_migration.rb b/app/presenters/content_item/featured_attachments_migration.rb
new file mode 100644
index 000000000..4806a1a0f
--- /dev/null
+++ b/app/presenters/content_item/featured_attachments_migration.rb
@@ -0,0 +1,23 @@
+module ContentItem
+  module FeaturedAttachmentsMigration
+    def choose_field(new_field_name:, old_field_name:)
+      new_list = content_item["details"][new_field_name]
+      old_list = content_item["details"][old_field_name] || []
+
+      # don't raise an error just because a document hasn't been
+      # republished to have the new field yet.
+      return old_list if new_list.nil?
+
+      if new_list.length == old_list.length
+        new_list
+      else
+        GovukError.notify(
+          "Mismatch between attachments and documents",
+          extra: { error_message: "Document with #{new_list.length} #{new_field_name} but #{old_list.length} #{old_field_name} at #{base_path}" },
+        )
+
+        old_list
+      end
+    end
+  end
+end
diff --git a/app/presenters/publication_presenter.rb b/app/presenters/publication_presenter.rb
index 97032933c..161635078 100644
--- a/app/presenters/publication_presenter.rb
+++ b/app/presenters/publication_presenter.rb
@@ -3,6 +3,7 @@ class PublicationPresenter < ContentItemPresenter
   include ContentItem::NationalApplicability
   include ContentItem::NationalStatisticsLogo
   include ContentItem::Political
+  include ContentItem::FeaturedAttachmentsMigration
 
   def details
     content_item["details"]["body"]
@@ -27,6 +28,9 @@ def dataset?
 private
 
   def documents_list
-    content_item["details"]["documents"]
+    @documents_list ||= choose_field(
+      new_field_name: "featured_attachments",
+      old_field_name: "documents",
+    )
   end
 end
diff --git a/test/presenters/content_item/featured_attachments_migration.rb b/test/presenters/content_item/featured_attachments_migration.rb
new file mode 100644
index 000000000..a796c81bf
--- /dev/null
+++ b/test/presenters/content_item/featured_attachments_migration.rb
@@ -0,0 +1,41 @@
+require "test_helper"
+
+class FeaturedAttachmentsMigrationTest < ActiveSupport::TestCase
+  class DummyContentItem
+    include ContentItem::FeaturedAttachmentsMigration
+    attr_accessor :content_item
+
+    def initialize(old_list, new_list)
+      @content_item = {
+        "base_path" => "/a/base/path",
+        "details" => { "old_badness" => old_list, "new_hotness" => new_list }.compact,
+        "links" => {},
+      }
+    end
+
+    def choose
+      choose_field(
+        old_field_name: "old_badness",
+        new_field_name: "new_hotness",
+      )
+    end
+  end
+
+  test "presents the old field if the new one is missing" do
+    item = DummyContentItem.new(%w(1 2 3), nil)
+
+    assert_equal %w(1 2 3), item.choose
+  end
+
+  test "presents the old field if the new one is a different size" do
+    item = DummyContentItem.new(%w(1 2 3), %w(foo bar baz bat))
+
+    assert_equal %w(1 2 3), item.choose
+  end
+
+  test "presents the new field if present and the old one is the same size" do
+    item = DummyContentItem.new(%w(1 2 3), %w(foo bar baz))
+
+    assert_equal %w(foo bar baz), item.choose
+  end
+end