Merge pull request #9985 from alphagov/content-modelling/941-empty-de…

…tails-of-changes-in-change-history (941) Hide details if there is no change history
alphagov · Feb 26, 2025 · b196ac2 · b196ac2
2 parents 6c63459 + d84dec8
commit b196ac2
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 79 deletions.
diff --git a/...ck_manager/content_block/document/show/document_timeline/timeline_item_component.html.erb b/...ck_manager/content_block/document/show/document_timeline/timeline_item_component.html.erb
@@ -12,25 +12,13 @@
     <%= date %>
   </p>
 
-  <% if version.field_diffs.present? %>
+  <% if details_of_changes.present? %>
     <div class="timeline__diff-table">
       <%= render "govuk_publishing_components/components/details", {
         title: "Details of changes",
         open: is_latest,
       } do %>
-        <% capture do %>
-          <%= render ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::FieldChangesTableComponent.new(
-            version:,
-            schema:,
-            ) %>
-
-          <% embedded_object_diffs.each do |item| %>
-            <%= render ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::EmbeddedObject::FieldChangesTableComponent.new(
-              **item,
-              content_block_edition: version.item,
-              ) %>
-          <% end %>
-        <% end %>
+        <% details_of_changes %>
       <% end %>
     </div>
   <% end %>

diff --git a/...nt_block_manager/content_block/document/show/document_timeline/timeline_item_component.rb b/...nt_block_manager/content_block/document/show/document_timeline/timeline_item_component.rb
@@ -51,4 +51,31 @@ def embedded_object_diffs
       end
     }.flatten
   end
+
+  def details_of_changes
+    @details_of_changes ||= begin
+      return "" if version.field_diffs.blank?
+
+      [
+        main_object_field_changes,
+        embedded_object_field_changes,
+      ].join.html_safe
+    end
+  end
+
+  def main_object_field_changes
+    render ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::FieldChangesTableComponent.new(
+      version:,
+      schema:,
+    )
+  end
+
+  def embedded_object_field_changes
+    embedded_object_diffs.map do |item|
+      render ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::EmbeddedObject::FieldChangesTableComponent.new(
+        **item,
+        content_block_edition: version.item,
+      )
+    end
+  end
 end
diff --git a/.../components/content_block/document/show/document_timeline/timeline_item_component_test.rb b/.../components/content_block/document/show/document_timeline/timeline_item_component_test.rb
@@ -110,7 +110,7 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::Timel
   describe "when field diffs are present" do
     let(:field_diffs) { { "foo" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "previous value", new_value: "new value") } }
     let(:version) do
-      build(
+      build_stubbed(
         :content_block_version,
         event: "created",
         whodunnit: user.id,
@@ -121,27 +121,32 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::Timel
       )
     end
 
-    it "renders the table component" do
-      table_component = ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::FieldChangesTableComponent.new(
-        version: build(:content_block_version, field_diffs: []),
+    let!(:table_component) do
+      ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::FieldChangesTableComponent.new(
+        version:,
         schema:,
       )
+    end
 
+    before do
       ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::FieldChangesTableComponent
         .expects(:new)
         .with(version:, schema:)
         .returns(table_component)
 
       component
         .expects(:render)
-        .with("govuk_publishing_components/components/details", { title: "Details of changes", open: false })
-        .with_block_given
-        .yields
+        .with(table_component)
+        .once
+        .returns("TABLE COMPONENT")
+    end
 
+    it "renders the table component unopened" do
       component
         .expects(:render)
-        .with(table_component)
-        .once
+        .with("govuk_publishing_components/components/details", { title: "Details of changes", open: false })
+        .with_block_given
+        .yields
 
       render_inline component
     end
@@ -157,70 +162,72 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::Timel
         render_inline component
       end
     end
+  end
 
-    describe "when a field diff is for an embedded object" do
-      let(:subschema) { stub(:subschema, id: "embedded_schema") }
-      let(:schema) { stub(:schema, subschemas: [subschema]) }
+  describe "when a field diff is for an embedded object" do
+    let(:subschema) { stub(:subschema, id: "embedded_schema") }
+    let(:schema) { stub(:schema, subschemas: [subschema]) }
 
-      let(:field_diffs) do
-        {
-          "details" => {
-            "embedded_schema" => {
-              "something" => {
-                "field1" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "before", new_value: "after"),
-                "field2" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "before", new_value: "after"),
-              },
+    let(:field_diffs) do
+      {
+        "details" => {
+          "embedded_schema" => {
+            "something" => {
+              "field1" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "before", new_value: "after"),
+              "field2" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "before", new_value: "after"),
             },
           },
-        }
-      end
+        },
+      }
+    end
 
-      let(:version) do
-        build(
-          :content_block_version,
-          event: "created",
-          whodunnit: user.id,
-          state: "published",
-          created_at: 4.days.ago,
-          item: content_block_edition,
-          field_diffs:,
-        )
-      end
+    let(:version) do
+      build_stubbed(
+        :content_block_version,
+        event: "created",
+        whodunnit: user.id,
+        state: "published",
+        created_at: 4.days.ago,
+        item: content_block_edition,
+        field_diffs:,
+      )
+    end
 
-      it "renders the embedded table component" do
-        table_component = stub("table_component")
+    it "renders the embedded table component" do
+      table_component = stub("table_component")
 
-        ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::EmbeddedObject::FieldChangesTableComponent
-          .expects(:new)
-          .with(
-            object_id: "something",
-            field_diff: {
-              "field1" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "before", new_value: "after"),
-              "field2" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "before", new_value: "after"),
-            },
-            subschema_id: "embedded_schema",
-            content_block_edition:,
-          )
-          .returns(table_component)
+      ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::EmbeddedObject::FieldChangesTableComponent
+        .expects(:new)
+        .with(
+          object_id: "something",
+          field_diff: {
+            "field1" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "before", new_value: "after"),
+            "field2" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: "before", new_value: "after"),
+          },
+          subschema_id: "embedded_schema",
+          content_block_edition:,
+        )
+        .returns(table_component)
 
-        component
-          .expects(:render)
-          .with("govuk_publishing_components/components/details", { title: "Details of changes", open: false })
-          .with_block_given
-          .yields
+      component
+        .expects(:render)
+        .with("govuk_publishing_components/components/details", { title: "Details of changes", open: false })
+        .with_block_given
+        .yields
 
-        component
-          .expects(:render)
-          .with(table_component)
-          .once
+      component
+        .expects(:render)
+        .with(table_component)
+        .once
+        .returns("TABLE COMPONENT 1")
 
-        component
-          .expects(:render)
-          .with(anything)
-          .once
+      component
+        .expects(:render)
+        .with(anything)
+        .once
+        .returns("TABLE COMPONENT 2")
 
-        render_inline component
-      end
+      render_inline component
     end
   end
 end
diff --git a/lib/engines/content_block_manager/test/factories/content_block_version.rb b/lib/engines/content_block_manager/test/factories/content_block_version.rb
@@ -2,15 +2,15 @@
   factory :content_block_version, class: "ContentBlockManager::ContentBlock::Version" do
     event { "created" }
     item do
-      create(
+      build(
         :content_block_edition,
-        document: create(
+        document: build(
           :content_block_document,
           block_type: "email_address",
         ),
       )
     end
-    whodunnit { create(:user).id }
+    whodunnit { build(:user).id }
     state {}
   end
 end