From d84dec82cb924d80f5ca082acef49d331884cbcf Mon Sep 17 00:00:00 2001 From: pezholio Date: Wed, 26 Feb 2025 10:12:05 +0000 Subject: [PATCH] Hide details if there is no change history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sometimes change history can be present, but doesn’t have items within it. To get around this, we move the logic to the component and only render the details if there are any details rendered. Additionally, I’ve changed the version factory to build, rather than create so we don’t have to create versions in the database unneccessarily --- .../timeline_item_component.html.erb | 16 +-- .../timeline_item_component.rb | 27 ++++ .../timeline_item_component_test.rb | 131 +++++++++--------- .../test/factories/content_block_version.rb | 6 +- 4 files changed, 101 insertions(+), 79 deletions(-) diff --git a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline/timeline_item_component.html.erb b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline/timeline_item_component.html.erb index ec322c0c160..e767301a4bb 100644 --- a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline/timeline_item_component.html.erb +++ b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline/timeline_item_component.html.erb @@ -12,25 +12,13 @@ <%= date %>

- <% if version.field_diffs.present? %> + <% if details_of_changes.present? %>
<%= 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 %>
<% end %> diff --git a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline/timeline_item_component.rb b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline/timeline_item_component.rb index 14b3d6967d5..0dd0b0af749 100644 --- a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline/timeline_item_component.rb +++ b/lib/engines/content_block_manager/app/components/content_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/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline/timeline_item_component_test.rb b/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline/timeline_item_component_test.rb index c2217dc354d..223541c076f 100644 --- a/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline/timeline_item_component_test.rb +++ b/lib/engines/content_block_manager/test/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,12 +121,14 @@ 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:) @@ -134,14 +136,17 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::Timel 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 index e680b3b9b64..971124ac854 100644 --- 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