From 9885eb88eae6bac0c1f786a1cfa5a343414f8a02 Mon Sep 17 00:00:00 2001 From: pezholio Date: Fri, 21 Feb 2025 14:32:08 +0000 Subject: [PATCH 1/6] Ensure change notes are not copied when cloning --- .../content_block_manager/content_block/edition.rb | 11 +++++++---- .../unit/app/models/content_block_edition_test.rb | 6 +++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition.rb b/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition.rb index d492a6c82eb..566edf5dd54 100644 --- a/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition.rb +++ b/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition.rb @@ -38,10 +38,13 @@ def first_class_details def clone_edition(creator:) new_edition = dup - new_edition.state = "draft" - new_edition.organisation = lead_organisation - new_edition.creator = creator - + new_edition.assign_attributes( + state: "draft", + organisation: lead_organisation, + creator: creator, + change_note: nil, + internal_change_note: nil, + ) new_edition end diff --git a/lib/engines/content_block_manager/test/unit/app/models/content_block_edition_test.rb b/lib/engines/content_block_manager/test/unit/app/models/content_block_edition_test.rb index 3333b36739d..bcc1fef66bf 100644 --- a/lib/engines/content_block_manager/test/unit/app/models/content_block_edition_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/models/content_block_edition_test.rb @@ -242,7 +242,9 @@ class ContentBlockManager::ContentBlockEditionTest < ActiveSupport::TestCase :content_block_edition, :email_address, title: "Some title", details: { "my" => "details" }, - state: "published" + state: "published", + change_note: "Something", + internal_change_note: "Something else" ) creator = create(:user) @@ -254,6 +256,8 @@ class ContentBlockManager::ContentBlockEditionTest < ActiveSupport::TestCase assert_equal new_edition.creator, creator assert_equal new_edition.title, content_block_edition.title assert_equal new_edition.details, content_block_edition.details + assert_equal new_edition.change_note, nil + assert_equal new_edition.internal_change_note, nil end end From 154bf79ef91264ef6d0656b9f8dc801501617db9 Mon Sep 17 00:00:00 2001 From: pezholio Date: Thu, 27 Feb 2025 16:33:57 +0000 Subject: [PATCH 2/6] Add embeddd object columns to versions This will allow us to capture additional information about an when an embedded object has been created. --- ...0250226161012_add_event_details_to_versions.rb | 15 +++++++++++++++ db/schema.rb | 4 +++- 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20250226161012_add_event_details_to_versions.rb diff --git a/db/migrate/20250226161012_add_event_details_to_versions.rb b/db/migrate/20250226161012_add_event_details_to_versions.rb new file mode 100644 index 00000000000..9c04012c55f --- /dev/null +++ b/db/migrate/20250226161012_add_event_details_to_versions.rb @@ -0,0 +1,15 @@ +class AddEventDetailsToVersions < ActiveRecord::Migration[8.0] + def up + change_table :content_block_versions, bulk: true do |t| + t.string :updated_embedded_object_type + t.string :updated_embedded_object_name + end + end + + def down + change_table :content_block_versions, bulk: true do |t| + t.remove :updated_embedded_object_type + t.remove :updated_embedded_object_name + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 39e6c4780d1..9a0290494f7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_02_20_131003) do +ActiveRecord::Schema[8.0].define(version: 2025_02_26_161012) do create_table "assets", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "asset_manager_id", null: false t.string "variant", null: false @@ -250,6 +250,8 @@ t.datetime "created_at", precision: nil, null: false t.text "state", size: :medium t.json "field_diffs" + t.string "updated_embedded_object_type" + t.string "updated_embedded_object_name" t.index ["item_id"], name: "index_content_block_versions_on_item_id" t.index ["item_type"], name: "index_content_block_versions_on_item_type" end From 56f61b351bd4f055b339066e9924630f6ddec802 Mon Sep 17 00:00:00 2001 From: pezholio Date: Thu, 27 Feb 2025 15:12:43 +0000 Subject: [PATCH 3/6] Allow event details to be passed to a version This adds a virutal field to allow us to pass information about an event to a version. --- .../content_block/edition/has_audit_trail.rb | 10 ++++++++- .../has_audit_trail_test.rb | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition/has_audit_trail.rb b/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition/has_audit_trail.rb index e90b6105ede..05885f4b2a4 100644 --- a/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition/has_audit_trail.rb +++ b/lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition/has_audit_trail.rb @@ -19,6 +19,8 @@ def self.acting_as(actor) after_update :record_update end + attr_accessor :updated_embedded_object_type, :updated_embedded_object_name + private def record_create @@ -30,7 +32,13 @@ def record_update unless draft? user = Current.user state = try(:state) - versions.create!(event: "updated", user:, state:, field_diffs: generate_diff) + versions.create!( + event: "updated", + user:, state:, + field_diffs: generate_diff, + updated_embedded_object_type:, + updated_embedded_object_name: + ) end end end diff --git a/lib/engines/content_block_manager/test/unit/app/models/content_block_edition/has_audit_trail_test.rb b/lib/engines/content_block_manager/test/unit/app/models/content_block_edition/has_audit_trail_test.rb index 45338f578c8..6ee9505ff13 100644 --- a/lib/engines/content_block_manager/test/unit/app/models/content_block_edition/has_audit_trail_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/models/content_block_edition/has_audit_trail_test.rb @@ -49,6 +49,27 @@ class ContentBlockManager::HasAuditTrailTest < ActiveSupport::TestCase assert_equal "scheduled", version.state end + it "adds event details if provided" do + Current.user = user + edition = create( + :content_block_edition, + creator: user, + document: create(:content_block_document, :email_address), + ) + edition.expects(:generate_diff).returns({}) + edition.updated_embedded_object_type = "something" + edition.updated_embedded_object_name = "here" + + assert_changes -> { edition.versions.count }, from: 1, to: 2 do + edition.publish! + end + + version = edition.versions.first + + assert_equal edition.updated_embedded_object_type, version.updated_embedded_object_type + assert_equal edition.updated_embedded_object_name, version.updated_embedded_object_name + end + it "does not record a version when updating an existing draft" do edition = create( :content_block_edition, From 2cad96229cc33e0f068c5267a8f6de7ec9c3cad0 Mon Sep 17 00:00:00 2001 From: pezholio Date: Thu, 27 Feb 2025 15:15:02 +0000 Subject: [PATCH 4/6] Add event details when updating an embedded object --- .../content_block/editions/embedded_objects_controller.rb | 2 ++ .../features/step_definitions/embedded_object_steps.rb | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/embedded_objects_controller.rb b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/embedded_objects_controller.rb index 96281722f56..e344855b984 100644 --- a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/embedded_objects_controller.rb +++ b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/embedded_objects_controller.rb @@ -59,6 +59,8 @@ def publish object_name: params[:object_name], ) else + @content_block_edition.updated_embedded_object_type = @subschema.block_type + @content_block_edition.updated_embedded_object_name = params[:object_name] ContentBlockManager::PublishEditionService.new.call(@content_block_edition) flash[:notice] = "#{@subschema.name.singularize} created" redirect_path = content_block_manager.content_block_manager_content_block_document_path(@content_block_edition.document) diff --git a/lib/engines/content_block_manager/features/step_definitions/embedded_object_steps.rb b/lib/engines/content_block_manager/features/step_definitions/embedded_object_steps.rb index 29814da2b77..963a32f01a2 100644 --- a/lib/engines/content_block_manager/features/step_definitions/embedded_object_steps.rb +++ b/lib/engines/content_block_manager/features/step_definitions/embedded_object_steps.rb @@ -43,6 +43,10 @@ @details.keys.each do |k| assert_equal edition.details[object_type.parameterize.pluralize][key][k], @details[k] end + + version = edition.versions.order("created_at asc").first + assert_equal version.updated_embedded_object_type, object_type.pluralize + assert_equal version.updated_embedded_object_name, @object_name end Then("I should see errors for the required {string} fields") do |object_type| From 68c7a6002461376ab0c806c609359508a429ffd5 Mon Sep 17 00:00:00 2001 From: pezholio Date: Thu, 27 Feb 2025 15:16:31 +0000 Subject: [PATCH 5/6] Add helper to check if embedded object was updated This checks for the presence of the relevant event details, which we can then use to generate the history in the next commit --- .../content_block_manager/content_block/version.rb | 4 ++++ .../unit/app/models/content_block_version_test.rb | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/lib/engines/content_block_manager/app/models/content_block_manager/content_block/version.rb b/lib/engines/content_block_manager/app/models/content_block_manager/content_block/version.rb index d5b6a6695cd..551facdfe42 100644 --- a/lib/engines/content_block_manager/app/models/content_block_manager/content_block/version.rb +++ b/lib/engines/content_block_manager/app/models/content_block_manager/content_block/version.rb @@ -10,6 +10,10 @@ class Version < ApplicationRecord def field_diffs self[:field_diffs] ? ContentBlock::DiffItem.from_hash(self[:field_diffs]) : {} end + + def is_embedded_update? + updated_embedded_object_type && updated_embedded_object_name + end end end end diff --git a/lib/engines/content_block_manager/test/unit/app/models/content_block_version_test.rb b/lib/engines/content_block_manager/test/unit/app/models/content_block_version_test.rb index 3de7d71f34e..4c425a977bf 100644 --- a/lib/engines/content_block_manager/test/unit/app/models/content_block_version_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/models/content_block_version_test.rb @@ -71,4 +71,17 @@ class ContentBlockManager::ContentBlockVersionTest < ActiveSupport::TestCase assert_equal ({}), content_block_version.field_diffs end end + + describe "#is_embedded_update?" do + it "returns false by default" do + assert_not content_block_version.is_embedded_update? + end + + it "returns true when updated_embedded_object_type and updated_embedded_object_name are set" do + content_block_version.updated_embedded_object_type = "something" + content_block_version.updated_embedded_object_name = "something" + + assert content_block_version.is_embedded_update? + end + end end From c2539253491748df7b39191d3cff8288756630d5 Mon Sep 17 00:00:00 2001 From: pezholio Date: Thu, 27 Feb 2025 15:19:31 +0000 Subject: [PATCH 6/6] Support embedded items in component --- .../timeline_item_component.html.erb | 11 +++- .../timeline_item_component.rb | 17 ++++-- .../timeline_item_component_test.rb | 52 +++++++++++++++++++ 3 files changed, 76 insertions(+), 4 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 e767301a4bb..771c72d2375 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,7 +12,16 @@ <%= date %>

- <% if details_of_changes.present? %> + <% if version.is_embedded_update? %> +
    + <% new_subschema_item_details.each do |field_name, value| %> +
  • + <%= field_name %>: + <%= value %> +
    • + <% end %> +
+ <% elsif details_of_changes.present? %>
<%= render "govuk_publishing_components/components/details", { title: "Details of changes", 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 0dd0b0af749..230e75b0b3a 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 @@ -13,16 +13,27 @@ def initialize(version:, schema:, is_first_published_version:, is_latest:) attr_reader :version, :schema, :is_first_published_version, :is_latest def title - case version.state - when "published" + if version.is_embedded_update? + "#{updated_subschema_id.humanize.singularize} created" + elsif version.state == "published" is_first_published_version ? "#{version.item.block_type.humanize} created" : version.state.capitalize - when "scheduled" + elsif version.state == "scheduled" "Scheduled for publishing on #{version.item.scheduled_publication.to_fs(:long_ordinal_with_at)}" else "#{version.item.block_type.humanize} #{version.state}" end end + def updated_subschema_id + version.updated_embedded_object_type + end + + def new_subschema_item_details + version.field_diffs.dig("details", updated_subschema_id, version.updated_embedded_object_name).map do |field_name, diff| + [field_name.humanize, diff.new_value] + end + end + def date tag.time( version.created_at.to_fs(:long_ordinal_with_at), 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 223541c076f..b1bc435d8cf 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 @@ -230,4 +230,56 @@ class ContentBlockManager::ContentBlock::Document::Show::DocumentTimeline::Timel render_inline component end end + + describe "when the version is an embedded update" do + let(:subschema) { stub(:subschema, id: "embedded_schema", name: "Embedded schema") } + let(:schema) { stub(:schema, subschemas: [subschema]) } + + let(:field_diffs) do + { + "details" => { + "embedded_schema" => { + "something" => { + "field1" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: nil, new_value: "Field 1 value"), + "field2" => ContentBlockManager::ContentBlock::DiffItem.new(previous_value: nil, new_value: "Field 2 value"), + }, + }, + }, + } + 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:, + updated_embedded_object_type: "embedded_schema", + updated_embedded_object_name: "something", + ) + end + + before do + schema.stubs(:subschema).with("embedded_schema").returns(subschema) + end + + it "renders the correct title" do + render_inline component + + assert_selector ".timeline__title", text: "Embedded schema created" + end + + it "renders the details of the updated object" do + render_inline component + + assert_selector ".timeline__embedded-item-list .timeline__embedded-item-list__item:nth-child(1) .timeline__embedded-item-list__key", text: "Field1:" + assert_selector ".timeline__embedded-item-list .timeline__embedded-item-list__item:nth-child(1) .timeline__embedded-item-list__value", text: "Field 1 value" + + assert_selector ".timeline__embedded-item-list .timeline__embedded-item-list__item:nth-child(2) .timeline__embedded-item-list__key", text: "Field2:" + assert_selector ".timeline__embedded-item-list .timeline__embedded-item-list__item:nth-child(2) .timeline__embedded-item-list__value", text: "Field 2 value" + end + end end