Add ability to show PII related information on the timeline

A show_pii flag allows optional exclusion of personall-identifiable
information from the event details and audited changes relating to
a patient. This has been set to true by default in the controller,
but can be changed to false trivially in case we decide not to
display PII in the future.

Author: murugapl

app/controllers/inspect/timeline/patients_controller.rb

@@ -9,6 +9,8 @@ class PatientsController < ApplicationController
 
       layout "full"
 
+      SHOW_PII = true
+
       DEFAULT_EVENT_NAMES = %w[
         consents
         school_moves
@@ -22,6 +24,8 @@ class PatientsController < ApplicationController
       ].freeze
 
       def show
+        @show_pii = SHOW_PII
+
         params.reverse_merge!(event_names: DEFAULT_EVENT_NAMES)
         params[:audit_config] ||= {}
 
@@ -46,7 +50,8 @@ def show
           TimelineRecords.new(
             @patient,
             detail_config: build_details_config,
-            audit_config: audit_config
+            audit_config: audit_config,
+            show_pii: @show_pii
           ).load_grouped_events(event_names)
 
         @no_events_message = true if @patient_timeline.empty?

app/views/inspect/timeline/patients/show.html.erb

@@ -16,12 +16,12 @@
 
 <div class="nhsuk-grid-row nhsuk-u-margin-top-4">
 
-  <div class="nhsuk-grid-column-one-third app-grid-column--sticky">
+  <div class="nhsuk-grid-column-one-half app-grid-column--sticky">
       <%= render AppTimelineFilterComponent.new(
             url: inspect_timeline_patient_path,
             patient: @patient,
             event_options: TimelineRecords::DEFAULT_DETAILS_CONFIG,
-            timeline_fields: TimelineRecords::AVAILABLE_DETAILS_CONFIG,
+            timeline_fields: @show_pii ? TimelineRecords::AVAILABLE_DETAILS_CONFIG_WITH_PII : TimelineRecords::AVAILABLE_DETAILS_CONFIG,
             class_imports: @patient_events[:class_imports],
             cohort_imports: @patient_events[:cohort_imports],
             sessions: @patient_events[:sessions],
@@ -34,7 +34,7 @@
           ) %>
   </div>
 
-  <div class="nhsuk-grid-column-two-thirds">
+  <div class="nhsuk-grid-column-one-half">
     <% if @no_events_message %>
       <%= render AppWarningCalloutComponent.new(
             heading: "No events found",

lib/timeline_records.rb

@@ -2,38 +2,108 @@
 
 class TimelineRecords
   DEFAULT_DETAILS_CONFIG = {
-    cohort_imports: [],
-    class_imports: [],
-    patient_sessions: %i[session_id],
-    school_moves: %i[school_id source],
-    school_move_log_entries: %i[school_id user_id],
     consents: %i[response route],
+    sessions: %i[],
+    session_attendances: %i[],
     triages: %i[status performed_by_user_id],
-    vaccination_records: %i[outcome session_id]
+    vaccination_records: %i[outcome session_id],
+    organisation: %i[],
+    cohort_imports: %i[],
+    class_imports: %i[],
+    parents: %i[],
+    patient_sessions: %i[session_id],
+    gillick_assessments: %i[],
+    parent_relationships: %i[],
+    school_moves: %i[school_id source],
+    school_move_log_entries: %i[school_id user_id]
   }.freeze
 
   AVAILABLE_DETAILS_CONFIG = {
-    cohort_imports: %i[rows_count status uploaded_by_user_id],
-    class_imports: %i[rows_count year_groups uploaded_by_user_id session_id],
-    patient_sessions: %i[session_id],
-    school_moves: %i[source school_id home_educated],
-    school_move_log_entries: %i[user_id school_id home_educated],
-    consents: %i[
-      programme_id
-      response
-      route
-      parent_id
-      withdrawn_at
-      invalidated_at
+    consents: %i[response route updated_at withdrawn_at invalidated_at],
+    sessions: %i[slug academic_year],
+    session_attendances: %i[attending updated_at],
+    triages: %i[status updated_at invalidated_at performed_by_user_id],
+    vaccination_records: %i[
+      outcome
+      performed_at
+      updated_at
+      discarded_at
+      uuid
+      session_id
+    ],
+    organisation: %i[name ods_code],
+    cohort_imports: %i[
+      csv_filename
+      processed_at
+      status
+      rows_count
+      new_record_count
+      exact_duplicate_record_count
+      changed_record_count
     ],
-    triages: %i[status performed_by_user_id programme_id invalidated_at],
+    class_imports: %i[
+      csv_filename
+      processed_at
+      status
+      rows_count
+      new_record_count
+      exact_duplicate_record_count
+      changed_record_count
+      year_groups
+    ],
+    parents: %i[],
+    patient_sessions: %i[],
+    gillick_assessments: %i[],
+    parent_relationships: %i[],
+    school_moves: %i[school_id source],
+    school_move_log_entries: %i[school_id user_id]
+  }.freeze
+
+  AVAILABLE_DETAILS_CONFIG_WITH_PII = {
+    consents: %i[response route updated_at withdrawn_at invalidated_at],
+    sessions: %i[slug academic_year],
+    session_attendances: %i[attending updated_at],
+    triages: %i[status updated_at invalidated_at performed_by_user_id],
     vaccination_records: %i[
       outcome
-      performed_by_user_id
-      programme_id
+      performed_at
+      updated_at
+      discarded_at
+      uuid
       session_id
-      vaccine_id
-    ]
+    ],
+    organisation: %i[name ods_code],
+    cohort_imports: %i[
+      csv_filename
+      processed_at
+      status
+      rows_count
+      new_record_count
+      exact_duplicate_record_count
+      changed_record_count
+    ],
+    class_imports: %i[
+      csv_filename
+      processed_at
+      status
+      rows_count
+      new_record_count
+      exact_duplicate_record_count
+      changed_record_count
+      year_groups
+    ],
+    parents: %i[full_name email phone],
+    patient_sessions: %i[session_id],
+    gillick_assessments: %i[
+      knows_vaccination
+      knows_disease
+      knows_consequences
+      knows_delivery
+      knows_side_effects
+    ],
+    parent_relationships: %i[type other_name],
+    school_moves: %i[school_id source],
+    school_move_log_entries: %i[school_id user_id]
   }.freeze
 
   DEFAULT_AUDITS_CONFIG = {
@@ -67,14 +137,57 @@ class TimelineRecords
     source
   ].freeze
 
-  def initialize(patient, detail_config: {}, audit_config: {})
+  ALLOWED_AUDITED_CHANGES_WITH_PII = %i[
+    full_name
+    email
+    phone
+    nhs_number
+    given_name
+    family_name
+    date_of_birth
+    address_line_1
+    address_line_2
+    address_town
+    address_postcode
+    home_educated
+    updated_from_pds_at
+    restricted_at
+    date_of_death
+    pending_changes
+    patient_id
+    session_id
+    programme_id
+    vaccine_id
+    organisation_id
+    school_id
+    gp_practice_id
+    uploaded_by_user_id
+    performed_by_user_id
+    user_id
+    parent_id
+    status
+    outcome
+    response
+    route
+    date_of_death_recorded_at
+    restricted_at
+    invalidated_at
+    withdrawn_at
+    rows_count
+    year_groups
+    home_educated
+    source
+  ].freeze
+
+  def initialize(patient, detail_config: {}, audit_config: {}, show_pii: false)
     @patient = patient
     @patient_id = @patient.id
     @patient_events = patient_events(@patient)
     @additional_events = additional_events(@patient)
     @detail_config = extract_detail_config(detail_config)
     @events = []
     @audit_config = audit_config
+    @show_pii = show_pii
   end
 
   def render_timeline(*event_names, truncate_columns: false)
@@ -213,12 +326,15 @@ def audits_events
         end
       ).reject { it.audited_changes.keys == ["updated_from_pds_at"] }
 
+    allowed_changes =
+      @show_pii ? ALLOWED_AUDITED_CHANGES_WITH_PII : ALLOWED_AUDITED_CHANGES
+
     audit_events.map do |audit|
       filtered_changes =
         audit
           .audited_changes
           .each_with_object({}) do |(key, value), hash|
-            if ALLOWED_AUDITED_CHANGES.include?(key.to_sym)
+            if allowed_changes.include?(key.to_sym)
               hash[key] = value
             elsif audits[:include_filtered_audit_changes]
               hash[key] = "[FILTERED]"

spec/lib/timeline_records_spec.rb

@@ -2,11 +2,16 @@
 
 describe TimelineRecords do
   subject(:timeline) do
-    described_class.new(patient, detail_config: detail_config)
+    described_class.new(
+      patient,
+      detail_config: detail_config,
+      show_pii: show_pii
+    )
   end
 
   let(:programme) { create(:programme, :hpv) }
   let(:detail_config) { {} }
+  let(:show_pii) { false }
   let(:organisation) { create(:organisation, programmes: [programme]) }
   let(:session) { create(:session, organisation:, programmes: [programme]) }
   let(:class_import) { create(:class_import, session:) }
@@ -410,6 +415,26 @@
       end
     end
 
+    context "with show_pii: true" do
+      let(:show_pii) { true }
+
+      it "includes PII-based audited changes" do
+        patient.audits.create!(
+          audited_changes: {
+            given_name: %w[Alessia Alice],
+            organisation_id: [nil, 1]
+          }
+        )
+        events = timeline.load_events(["audits"])
+        expect(events.first[:details][:audited_changes]).to have_key(
+          :given_name
+        )
+        expect(events.first[:details][:audited_changes]).to have_key(
+          :organisation_id
+        )
+      end
+    end
+
     context "with include_associated_audits: false" do
       let(:timeline) do
         described_class.new(