From 544fa51b7b6c3f18a3b0a4d7ed3f3d8612357150 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 28 Jan 2025 11:37:32 +0000 Subject: [PATCH 1/2] refactor doc timeline test --- .../show/document_timeline_component_test.rb | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline_component_test.rb b/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline_component_test.rb index 341454637cf..c91289e5144 100644 --- a/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline_component_test.rb +++ b/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline_component_test.rb @@ -4,42 +4,44 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineCompone include Rails.application.routes.url_helpers include ActionView::Helpers::UrlHelper include ApplicationHelper + extend Minitest::Spec::DSL + + let(:user) { create(:user) } test "renders a timeline component with events in correct order" do - @user = create(:user) - @version_1 = create( + version_1 = create( :content_block_version, event: "created", - whodunnit: @user.id, + whodunnit: user.id, ) - @version_2 = create( + version_2 = create( :content_block_version, event: "updated", - whodunnit: @user.id, + whodunnit: user.id, state: "published", ) - @version_3 = create( + version_3 = create( :content_block_version, event: "updated", - whodunnit: @user.id, + whodunnit: user.id, state: "scheduled", ) render_inline(ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineComponent.new( - content_block_versions: [@version_3, @version_2, @version_1], + content_block_versions: [version_3, version_2, version_1], )) assert_selector ".timeline__item", count: 2 assert_equal "Email address scheduled", page.all(".timeline__title")[0].text - assert_equal "by #{linked_author(@user, { class: 'govuk-link' })}", page.all(".timeline__byline")[0].native.inner_html - assert_equal I18n.l(@version_3.created_at, format: :long_ordinal), - page.all("time[datetime='#{@version_3.created_at.iso8601}']")[1].text + assert_equal "by #{linked_author(user, { class: 'govuk-link' })}", page.all(".timeline__byline")[0].native.inner_html + assert_equal I18n.l(version_3.created_at, format: :long_ordinal), + page.all("time[datetime='#{version_3.created_at.iso8601}']")[1].text assert_equal "Email address published", page.all(".timeline__title")[1].text - assert_equal "by #{linked_author(@user, { class: 'govuk-link' })}", page.all(".timeline__byline")[1].native.inner_html - assert_equal I18n.l(@version_2.created_at, format: :long_ordinal), - page.all("time[datetime='#{@version_2.created_at.iso8601}']")[1].text + assert_equal "by #{linked_author(user, { class: 'govuk-link' })}", page.all(".timeline__byline")[1].native.inner_html + assert_equal I18n.l(version_2.created_at, format: :long_ordinal), + page.all("time[datetime='#{version_2.created_at.iso8601}']")[1].text assert_no_selector ".govuk-table" end @@ -62,17 +64,16 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineCompone "previous_value": "old instructions", }, ] - @user = create(:user) - @version = create( + version = create( :content_block_version, event: "updated", - whodunnit: @user.id, + whodunnit: user.id, state: "scheduled", field_diffs: field_diffs, ) render_inline(ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineComponent.new( - content_block_versions: [@version], + content_block_versions: [version], )) assert_equal "old title", page.all("td")[0].text From 52d6e98dab140997e6ea98305db1f0a9119c2606 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 28 Jan 2025 12:34:19 +0000 Subject: [PATCH 2/2] add change notes to document timeline --- .../components/_timeline-component.scss | 8 +++- .../show/document_timeline_component.html.erb | 16 ++++++++ .../show/document_timeline_component.rb | 10 +++++ .../features/edit_object.feature | 1 + .../step_definitions/timeline_steps.rb | 5 +++ .../features/support/helpers.rb | 6 ++- .../show/document_timeline_component_test.rb | 40 ++++++++++++++++++- 7 files changed, 81 insertions(+), 5 deletions(-) diff --git a/lib/engines/content_block_manager/app/assets/stylesheets/content_block_manager/components/_timeline-component.scss b/lib/engines/content_block_manager/app/assets/stylesheets/content_block_manager/components/_timeline-component.scss index ed7daca57f8..b7deb59be42 100644 --- a/lib/engines/content_block_manager/app/assets/stylesheets/content_block_manager/components/_timeline-component.scss +++ b/lib/engines/content_block_manager/app/assets/stylesheets/content_block_manager/components/_timeline-component.scss @@ -51,8 +51,6 @@ .timeline__date { @include govuk-font($size: 16); - margin-top: govuk-spacing(1); - margin-bottom: 0; } .timeline__description { @@ -80,3 +78,9 @@ padding-left: 0; } } + +.timeline__note { + .govuk-body { + margin-bottom: govuk-spacing(2); + } +} diff --git a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline_component.html.erb b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline_component.html.erb index 7607d0b5790..7d2427eb7e3 100644 --- a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline_component.html.erb +++ b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline_component.html.erb @@ -45,6 +45,22 @@ <% end %> <% end %> + +
+ <% if item[:internal_change_note].present? %> +
+

Internal note

+

<%= item[:internal_change_note] %>

+
+ <% end %> + + <% if item[:change_note].present? %> +
+

Public note

+

<%= item[:change_note] %>

+
+ <% end %> +
<% end %> diff --git a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline_component.rb b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline_component.rb index 2051f1ea5b9..5d632ca9e31 100644 --- a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline_component.rb +++ b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/document_timeline_component.rb @@ -15,6 +15,8 @@ def items byline: User.find_by_id(version.whodunnit)&.then { |user| helpers.linked_author(user, { class: "govuk-link" }) } || "unknown user", date: time_html(version.created_at), table_rows: table_rows(version), + internal_change_note: internal_change_note(version), + change_note: change_note(version), } end end @@ -47,4 +49,12 @@ def table_rows(version) end end end + + def internal_change_note(version) + version.item.internal_change_note + end + + def change_note(version) + version.item.change_note + end end diff --git a/lib/engines/content_block_manager/features/edit_object.feature b/lib/engines/content_block_manager/features/edit_object.feature index 26abe9f93da..ee6a24f33a2 100644 --- a/lib/engines/content_block_manager/features/edit_object.feature +++ b/lib/engines/content_block_manager/features/edit_object.feature @@ -35,6 +35,7 @@ Feature: Edit a content object Then the edition should have been updated successfully And I should be taken back to the document page And I should see 2 publish events on the timeline + And I should see the notes on the timeline And I should see the edition diff in a table Scenario: GDS editor cancels the creation of an object when reviewing links diff --git a/lib/engines/content_block_manager/features/step_definitions/timeline_steps.rb b/lib/engines/content_block_manager/features/step_definitions/timeline_steps.rb index 1d72f4d9615..61569f683a5 100644 --- a/lib/engines/content_block_manager/features/step_definitions/timeline_steps.rb +++ b/lib/engines/content_block_manager/features/step_definitions/timeline_steps.rb @@ -7,6 +7,11 @@ expect(page).to have_selector(".timeline__title", text: "Email address published", count:) end +Then("I should see the notes on the timeline") do + expect(page).to have_selector("p", text: @internal_note) + expect(page).to have_selector("p", text: @change_note) +end + Then("I should see the publish event on the timeline") do expect(page).to have_selector(".timeline__title", text: "Email address published") expect(page).to have_selector(".timeline__byline", text: "by Scheduled Publishing Robot") diff --git a/lib/engines/content_block_manager/features/support/helpers.rb b/lib/engines/content_block_manager/features/support/helpers.rb index 0f2548f91b6..a8504462abb 100644 --- a/lib/engines/content_block_manager/features/support/helpers.rb +++ b/lib/engines/content_block_manager/features/support/helpers.rb @@ -71,13 +71,15 @@ def update_content_block end def add_internal_note - fill_in "Describe the change for internal users", with: "Some internal note goes here" + @internal_note = "Some internal note goes here" + fill_in "Describe the change for internal users", with: @internal_note click_save_and_continue end def add_change_note + @change_note = "Some text" choose "Yes - information has been added, updated or removed" - fill_in "Describe the edit for users", with: "Some text" + fill_in "Describe the edit for users", with: @change_note click_save_and_continue end diff --git a/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline_component_test.rb b/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline_component_test.rb index c91289e5144..a0196839fa7 100644 --- a/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline_component_test.rb +++ b/lib/engines/content_block_manager/test/components/content_block/document/show/document_timeline_component_test.rb @@ -5,26 +5,30 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineCompone include ActionView::Helpers::UrlHelper include ApplicationHelper extend Minitest::Spec::DSL - + let(:user) { create(:user) } test "renders a timeline component with events in correct order" do + item = build(:content_block_edition, :email_address, change_note: nil, internal_change_note: nil) version_1 = create( :content_block_version, event: "created", whodunnit: user.id, + item:, ) version_2 = create( :content_block_version, event: "updated", whodunnit: user.id, state: "published", + item:, ) version_3 = create( :content_block_version, event: "updated", whodunnit: user.id, state: "scheduled", + item:, ) render_inline(ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineComponent.new( @@ -44,6 +48,8 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineCompone page.all("time[datetime='#{version_2.created_at.iso8601}']")[1].text assert_no_selector ".govuk-table" + assert_no_selector "h2", text: "Internal note" + assert_no_selector "h2", text: "Public note" end test "renders the edition diff table in correct order" do @@ -83,4 +89,36 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineCompone assert_equal "old instructions", page.all("td")[4].text assert_equal "new instructions", page.all("td")[5].text end + + test "renders an internal change note" do + edition = create(:content_block_edition, :email_address, internal_change_note: "changed x to y") + version = create( + :content_block_version, + item: edition, + event: "updated", + state: "published", + ) + + render_inline(ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineComponent.new( + content_block_versions: [version], + )) + + assert_selector "p", text: "changed x to y" + end + + test "renders a public change note" do + edition = create(:content_block_edition, :email_address, change_note: "changed a to b") + version = create( + :content_block_version, + item: edition, + event: "updated", + state: "published", + ) + + render_inline(ContentBlockManager::ContentBlock::Document::Show::DocumentTimelineComponent.new( + content_block_versions: [version], + )) + + assert_selector "p", text: "changed a to b" + end end