Show changes to child records (manual or uploaded) in the activity log

murugapl · murugapl · commit e3d4e48a7021 · 2026-04-23T11:02:40.000+01:00
This displays the collected info on changes to patient records in the child activity log
diff --git a/app/components/app_activity_log_component.rb b/app/components/app_activity_log_component.rb
@@ -34,6 +34,13 @@ class AppActivityLogComponent < ViewComponent::Base
               <%= event[:invalidated] ? tag.s(body) : body %>
             </p></blockquote>
           <% end %>
+
+          <% if (changes = event[:changes]).present? %>
+            <%= render AppDetailsComponent.new(open: true) do |details| %>
+              <% details.with_summary { pluralize(changes.size, "field") + " updated" } %>
+              <%= changes_summary_list(changes) %>
+            <% end %>
+          <% end %>
         <% end %>
       <% end %>
     </div>
@@ -57,6 +64,7 @@ def all_events
       gillick_assessment_events,
       note_events,
       notify_events,
+      patient_change_events,
       patient_merge_events,
       patient_specific_direction_events,
       pre_screening_events,
@@ -270,6 +278,26 @@ def notify_events
     end
   end
 
+  CHANGE_SOURCE_TITLES = {
+    "manual_edit" => "Record updated manually",
+    "cohort_import" =>
+      "Record updated after new details were imported in a cohort upload",
+    "class_import" =>
+      "Record updated after new details were imported in a class upload"
+  }.freeze
+
+  def patient_change_events
+    patient_change_log_entries.map do |entry|
+      title = CHANGE_SOURCE_TITLES[entry.source]
+      {
+        title:,
+        at: entry.created_at,
+        by: entry.user,
+        changes: entry.recorded_changes
+      }
+    end
+  end
+
   def patient_merge_events
     patient_merge_log_entries.map do |patient_merge_log_entry|
       {
@@ -435,6 +463,8 @@ def attendance_events
 
   private
 
+  delegate :format_nhs_number, :govuk_summary_list, to: :helpers
+
   def include_programme_specific_events?
     @programme_type.present? || @session.present?
   end
@@ -446,6 +476,13 @@ def archive_reasons
       @patient.archive_reasons.where(team: @team).includes(:created_by)
   end
 
+  def patient_change_log_entries
+    return [] if include_programme_specific_events?
+
+    @patient_change_log_entries ||=
+      @patient.patient_change_log_entries.includes(:user)
+  end
+
   def patient_merge_log_entries
     return [] if include_programme_specific_events?
 
@@ -615,6 +652,75 @@ def vaccination_records
         end
   end
 
+  def changes_summary_list(changes)
+    rows =
+      PatientChangeLogEntry::TRACKED_ATTRIBUTES.filter_map do |attr|
+        next unless changes.key?(attr)
+
+        old_val, new_val = changes[attr]
+        {
+          key: {
+            text: PatientChangeLogEntry.label_for(attr)
+          },
+          value: {
+            text: change_value_html(attr, old_val, new_val)
+          }
+        }
+      end
+    govuk_summary_list(rows:)
+  end
+
+  def change_value_html(attr, old_val, new_val)
+    arrow =
+      tag.svg(
+        safe_join(
+          [
+            tag.title("changed to"),
+            tag.path(
+              d:
+                "m14.7 6.3 5 5c.2.2.3.4.3.7 0 .3-.1.5-.3.7l-5 5" \
+                  "a1 1 0 0 1-1.4-1.4l3.3-3.3H5a1 1 0 0 1 0-2" \
+                  "h11.6l-3.3-3.3a1 1 0 1 1 1.4-1.4Z"
+            )
+          ]
+        ),
+        class: "nhsuk-icon nhsuk-icon--arrow-right",
+        style: "vertical-align: middle",
+        xmlns: "http://www.w3.org/2000/svg",
+        viewBox: "0 0 24 24",
+        width: "16",
+        height: "16",
+        focusable: "false",
+        role: "img",
+        "aria-label": "changed to"
+      )
+
+    safe_join(
+      [
+        format_change_value(attr, old_val),
+        " ",
+        arrow,
+        " ",
+        tag.mark(format_change_value(attr, new_val), class: "app-highlight")
+      ]
+    )
+  end
+
+  def format_change_value(attr, value)
+    return "Not provided" if value.nil? || value.to_s.empty?
+
+    case attr
+    when "date_of_birth"
+      Date.parse(value.to_s).to_fs(:long)
+    when "gender_code"
+      Patient.human_enum_name(:gender_code, value)
+    when "nhs_number"
+      format_nhs_number(value.to_s)
+    else
+      value.to_s
+    end
+  end
+
   def expired_items_for(academic_year:, programmes:)
     {
       consents:,
diff --git a/app/models/patient_change_log_entry.rb b/app/models/patient_change_log_entry.rb
@@ -23,19 +23,26 @@
 #  fk_rails_...  (user_id => users.id)
 #
 class PatientChangeLogEntry < ApplicationRecord
-  TRACKED_ATTRIBUTES = %w[
-    given_name
-    family_name
-    preferred_given_name
-    preferred_family_name
-    date_of_birth
-    gender_code
-    nhs_number
-    address_line_1
-    address_line_2
-    address_town
-    address_postcode
-  ].freeze
+  ATTRIBUTE_LABELS = {
+    "nhs_number" => "NHS number",
+    "given_name" => "First name",
+    "family_name" => "Last name",
+    "preferred_given_name" => "Preferred first name",
+    "preferred_family_name" => "Preferred last name",
+    "date_of_birth" => "Date of birth",
+    "gender_code" => "Gender",
+    "address_line_1" => "Address line 1",
+    "address_line_2" => "Address line 2",
+    "address_town" => "Town",
+    "address_postcode" => "Postcode"
+  }.freeze
+  private_constant :ATTRIBUTE_LABELS
+
+  TRACKED_ATTRIBUTES = ATTRIBUTE_LABELS.keys.freeze
+
+  def self.label_for(attribute)
+    ATTRIBUTE_LABELS[attribute]
+  end
 
   belongs_to :patient
   belongs_to :user, optional: true
@@ -56,7 +63,13 @@ def self.log_import_changes!(patients:, import:)
     patients.each do |patient|
       next if patient.id.blank?
 
-      recorded_changes = patient.changes.slice(*TRACKED_ATTRIBUTES)
+      recorded_changes =
+        patient
+          .changes
+          .slice(*TRACKED_ATTRIBUTES)
+          .reject do |_attr, (old_val, new_val)|
+            old_val.presence == new_val.presence
+          end
       next if recorded_changes.empty?
 
       create!(patient:, user:, source:, recorded_changes:)
diff --git a/spec/components/app_activity_log_component_spec.rb b/spec/components/app_activity_log_component_spec.rb
@@ -788,6 +788,117 @@
     end
   end
 
+  describe "patient change log events" do
+    let(:component) { described_class.new(patient:, team:) }
+
+    context "with a manual edit" do
+      before do
+        create(
+          :patient_change_log_entry,
+          patient:,
+          user:,
+          source: :manual_edit,
+          recorded_changes: {
+            "nhs_number" => %w[9990000018 9758623168]
+          },
+          created_at: Time.zone.local(2025, 6, 1, 12)
+        )
+      end
+
+      include_examples "card",
+                       title: "Record updated manually",
+                       date: "1 June 2025 at 12:00pm",
+                       by: "JOY, Nurse"
+
+      it "renders a details expandable with the number of changed fields" do
+        expect(rendered).to have_css(
+          "details.nhsuk-details[open]",
+          text: "1 field updated"
+        )
+      end
+
+      it "renders the field label and formatted old and new values" do
+        expect(rendered).to have_css(
+          ".nhsuk-summary-list__key",
+          text: "NHS number"
+        )
+        expect(rendered).to have_css(
+          ".nhsuk-summary-list__value",
+          text: "999 000 0018"
+        )
+        expect(rendered).to have_css(
+          ".nhsuk-summary-list__value mark.app-highlight",
+          text: "975 862 3168"
+        )
+      end
+    end
+
+    context "with a cohort import" do
+      before do
+        create(
+          :patient_change_log_entry,
+          patient:,
+          user:,
+          source: :cohort_import,
+          recorded_changes: {
+            "given_name" => %w[Sarah Sara]
+          },
+          created_at: Time.zone.local(2025, 6, 1, 12)
+        )
+      end
+
+      include_examples "card",
+                       title:
+                         "Record updated after new details were imported in a cohort upload",
+                       date: "1 June 2025 at 12:00pm",
+                       by: "JOY, Nurse"
+    end
+
+    context "with a class import" do
+      before do
+        create(
+          :patient_change_log_entry,
+          patient:,
+          user:,
+          source: :class_import,
+          recorded_changes: {
+            "given_name" => %w[Sarah Sara]
+          },
+          created_at: Time.zone.local(2025, 6, 1, 12)
+        )
+      end
+
+      include_examples "card",
+                       title:
+                         "Record updated after new details were imported in a class upload",
+                       date: "1 June 2025 at 12:00pm",
+                       by: "JOY, Nurse"
+    end
+
+    context "with multiple changed fields" do
+      before do
+        create(
+          :patient_change_log_entry,
+          patient:,
+          user:,
+          source: :manual_edit,
+          recorded_changes: {
+            "given_name" => %w[Sarah Sara],
+            "family_name" => %w[Doe Dow]
+          },
+          created_at: Time.zone.local(2025, 6, 1, 12)
+        )
+      end
+
+      it "renders the count of changed fields in the disclosure" do
+        expect(rendered).to have_css(
+          "details.nhsuk-details[open]",
+          text: "2 fields updated"
+        )
+      end
+    end
+  end
+
   describe "patient merge events" do
     let(:component) { described_class.new(patient:, team:) }
 
diff --git a/spec/features/import_child_records_with_duplicates_spec.rb b/spec/features/import_child_records_with_duplicates_spec.rb
@@ -29,6 +29,7 @@
     then_i_should_see_a_success_message
     and_i_should_return_to_the_import_page
     and_the_first_duplicate_record_should_be_persisted
+    and_a_change_log_entry_is_created_for_the_cohort_import_change
 
     when_i_visit_the_import_issues_page
     and_i_review_the_second_duplicate_record
@@ -79,6 +80,7 @@
       then_i_should_see_a_success_message
       and_i_should_return_to_the_import_page
       and_the_first_duplicate_record_should_be_persisted
+      and_a_change_log_entry_is_created_for_the_cohort_import_change
 
       when_i_visit_the_import_issues_page
       and_i_review_the_second_duplicate_record
@@ -383,6 +385,18 @@ def and_the_first_duplicate_record_should_be_persisted
     expect(@first_patient.pending_changes).to eq({})
   end
 
+  def and_a_change_log_entry_is_created_for_the_cohort_import_change
+    visit patient_path(@first_patient)
+    within(".nhsuk-card", text: "Activity log") do
+      expect(page).to have_content(
+        "Record updated after new details were imported in a cohort upload"
+      )
+      expect(page).to have_css(".nhsuk-summary-list__key", text: "Postcode")
+      expect(page).to have_content("SW11 1AA")
+      expect(page).to have_css("mark.app-highlight", text: "SW1A 1AA")
+    end
+  end
+
   def and_the_second_record_should_not_be_updated
     @second_patient.reload
     expect(@second_patient.given_name).to eq("James")
@@ -448,8 +462,10 @@ def when_i_go_to_the_fourth_uploaded_record
   end
 
   def then_i_should_see_the_address_is_updated
-    expect(page).to have_content("10 Downing Street")
-    expect(page).not_to have_content("11 Downing Street")
+    within(".nhsuk-card", text: "Child record") do
+      expect(page).to have_content("10 Downing Street")
+      expect(page).not_to have_content("11 Downing Street")
+    end
   end
 
   def and_the_required_feature_flags_are_enabled
diff --git a/spec/features/import_class_lists_with_duplicates_spec.rb b/spec/features/import_class_lists_with_duplicates_spec.rb
diff --git a/spec/features/manage_children_spec.rb b/spec/features/manage_children_spec.rb
