Merge pull request #9995 from alphagov/content-modelling/936-weird-en…

…tries-in-the-change-history

(936) Fix weird entries in the change history

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

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

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 %>
   </p>
 
-  <% if details_of_changes.present? %>
+  <% if version.is_embedded_update? %>
+    <ul class="govuk-list timeline__embedded-item-list">
+      <% new_subschema_item_details.each do |field_name, value| %>
+        <li class="timeline__embedded-item-list__item">
+          <strong class="timeline__embedded-item-list__key"><%= field_name %>:</strong>
+          <span class="timeline__embedded-item-list__value"><%= value %></span>
+        </li>
+      <% end %>
+    </ul>
+  <% elsif details_of_changes.present? %>
     <div class="timeline__diff-table">
       <%= render "govuk_publishing_components/components/details", {
         title: "Details of changes",

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),

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)

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
 

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

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

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|

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

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,

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
 

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