Add ability to show PII related information on the timeline

A show_pii flag allows optional exclusion of personally-identifiable
information from the event details and audited changes relating to
a patient. This has been set to false by default in the controller,
but can be toggled on or off with a check box in the UI.

Author: murugapl

app/components/app_timeline_filter_component.html.erb

@@ -158,6 +158,15 @@
                                          "data-turbo-permanent": "true" %>
             <% end %>
             <% end %>
+
+            <div class="nhsuk-u-margin-bottom-4">
+            <%= f.govuk_check_box :show_pii, true,
+                                  label: { text: "Show PII" },
+                                  checked: @show_pii,
+                                  "data-autosubmit-target": "field",
+                                  "data-action": "autosubmit#submit",
+                                  "data-turbo-permanent": "true" %>
+            </div>
 
             <%= helpers.govuk_button_link_to "Reset filters",
                                              @reset_url,

app/components/app_timeline_filter_component.rb

@@ -11,7 +11,8 @@ def initialize(
     class_imports:,
     cohort_imports:,
     sessions:,
-    reset_url:
+    reset_url:,
+    show_pii:
   )
     @url = url
     @patient = patient
@@ -23,6 +24,7 @@ def initialize(
     @cohort_imports = cohort_imports
     @sessions = sessions
     @reset_url = reset_url
+    @show_pii = show_pii
   end
 
   attr_reader :url,
@@ -33,5 +35,6 @@ def initialize(
               :class_imports,
               :cohort_imports,
               :sessions,
-              :reset_url
+              :reset_url,
+              :show_pii
 end

app/controllers/inspect/timeline/patients_controller.rb

@@ -7,6 +7,8 @@ class Inspect::Timeline::PatientsController < ApplicationController
 
   layout "full"
 
+  SHOW_PII = false
+
   DEFAULT_EVENT_NAMES = %w[
     consents
     school_moves
@@ -20,6 +22,8 @@ class Inspect::Timeline::PatientsController < ApplicationController
   ].freeze
 
   def show
+    @show_pii = params[:show_pii] || SHOW_PII
+
     params.reverse_merge!(event_names: DEFAULT_EVENT_NAMES)
     params[:audit_config] ||= {}
 
@@ -43,7 +47,8 @@ def show
       TimelineRecords.new(
         @patient,
         detail_config: build_details_config,
-        audit_config: audit_config
+        audit_config: audit_config,
+        show_pii: @show_pii
       ).load_timeline_events(event_names)
 
     @no_events_message = true if @patient_timeline.empty?
@@ -95,8 +100,10 @@ def sample_patient(compare_option)
         :invalid_patient
       end
     else
-      raise ArgumentError,
-            "Invalid patient comparison option: #{compare_option}"
+      raise ArgumentError, <<~MESSAGE
+      Invalid patient comparison option: #{compare_option}.
+      Supported options are: class_import, cohort_import, session, manual_entry
+    MESSAGE
     end
   end
 

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

@@ -6,7 +6,7 @@
   <%= "Rails console: Patient.find(#{@patient.id})" %>
 </code>
 
-<% if params[:event_names].include?("audits") && @patient_timeline %>
+<% if params[:event_names].include?("audits") && @patient_timeline && !@show_pii %>
   <%= govuk_inset_text do %>
     <p class="nhsuk-body">
       Only audited changes that do not involve PII are included
@@ -16,13 +16,13 @@
 
 <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,
             team: @patient.sessions.includes(:team).first.team,
             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],
@@ -31,11 +31,13 @@
               event_names: Inspect::Timeline::PatientsController::DEFAULT_EVENT_NAMES,
               detail_config: TimelineRecords::DEFAULT_DETAILS_CONFIG,
               compare_option: nil,
+              show_pii: false,
             ),
+            show_pii: @show_pii,
           ) %>
   </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,102 @@
 
 class TimelineRecords
   DEFAULT_DETAILS_CONFIG = {
-    cohort_imports: [],
-    class_imports: [],
-    sessions: %i[],
-    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],
+    cohort_imports: %i[],
+    class_imports: %i[],
+    parents: %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 = {
-    cohort_imports: %i[rows_count status uploaded_by_user_id],
-    class_imports: %i[rows_count year_groups uploaded_by_user_id location_id],
-    sessions: %i[],
-    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
+    ],
+    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[],
+    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
-    ]
+    ],
+    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],
+    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 = {
@@ -69,14 +133,59 @@ 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
+    location_id
+    programme_id
+    vaccine_id
+    organisation_id
+    team_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)
@@ -240,12 +349,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(:team) { create(:team, programmes: [programme]) }
   let(:session) { create(:session, team:, programmes: [programme]) }
   let(:class_import) { create(:class_import, session:) }
@@ -398,6 +403,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(