From 79f0077bdf9e754103031e9e9d967b3251498d0f Mon Sep 17 00:00:00 2001 From: Lakshmi Murugappan Date: Fri, 29 Aug 2025 23:12:22 +0100 Subject: [PATCH 1/3] Add AppTimelineComponent Component to view events in a timelien format. This will be used by the PDS lookup history feature, and eventually the activity log too. --- app/assets/stylesheets/components/_index.scss | 1 + .../stylesheets/components/_timeline.scss | 126 ++++++++++++++++++ app/components/app_timeline_component.rb | 41 ++++++ .../components/app_timeline_component_spec.rb | 66 +++++++++ 4 files changed, 234 insertions(+) create mode 100644 app/assets/stylesheets/components/_timeline.scss create mode 100644 app/components/app_timeline_component.rb create mode 100644 spec/components/app_timeline_component_spec.rb diff --git a/app/assets/stylesheets/components/_index.scss b/app/assets/stylesheets/components/_index.scss index 551882b7b2..e8115a8957 100644 --- a/app/assets/stylesheets/components/_index.scss +++ b/app/assets/stylesheets/components/_index.scss @@ -17,4 +17,5 @@ @forward "summary-list"; @forward "tables"; @forward "tag"; +@forward "timeline"; @forward "vaccine-method"; diff --git a/app/assets/stylesheets/components/_timeline.scss b/app/assets/stylesheets/components/_timeline.scss new file mode 100644 index 0000000000..6f346289fb --- /dev/null +++ b/app/assets/stylesheets/components/_timeline.scss @@ -0,0 +1,126 @@ +@use "../vendor/nhsuk-frontend" as *; + +$_badge-size-mobile: 16px; +$_badge-small-size-mobile: 12px; +$_timeline-border-width: 2px; + +@function _dot-size($size) { + @if $size == "default" { + @return $_badge-size-mobile; + } @else if $size == "small" { + @return $_badge-small-size-mobile; + } +} + +@function _dot-ml($size) { + @return - calc(($size / 2) + ($_timeline-border-width / 2)); +} + +@function _dot-mt-tablet($margin) { + @return $margin - 1px; +} + +@mixin _badge($size) { + $mt: 4px; + $mt-small: 6px; + + height: _dot-size($size); + margin-left: _dot-ml(_dot-size($size)); + margin-right: if( + $size == "default", + nhsuk-spacing(4), + nhsuk-spacing(4) + 2px + ); + margin-top: if($size == "default", $mt, $mt-small); + width: _dot-size($size); + + @include nhsuk-media-query($from: tablet) { + $tablet: _dot-size($size) + 4px; + + height: $tablet; + margin-left: _dot-ml($tablet); + margin-top: if( + $size == "default", + _dot-mt-tablet($mt), + _dot-mt-tablet($mt-small) + ); + width: $tablet; + } +} + +.app-timeline { + list-style: none; + padding: 0; + + @include nhsuk-responsive-margin(5, "bottom"); + @include nhsuk-responsive-padding(2, "top"); + + &__item { + display: flex; + margin-bottom: 0; + margin-left: 12px; + margin-top: -6px; + position: relative; + + @include nhsuk-responsive-padding(5, "bottom"); + + &:last-child { + padding: 0; + + &::before { + border: none; + } + } + + &::before { + border-left: $_timeline-border-width solid $color_nhsuk-grey-3; + bottom: 0; + content: ""; + display: block; + left: -$_timeline-border-width; + position: absolute; + top: nhsuk-spacing(2); + width: $_timeline-border-width; + } + + &--past::before { + border-color: $color_nhsuk-blue; + } + } + + &__badge { + flex-shrink: 0; + z-index: 1; + + @include _badge("default"); + + &--small { + @include _badge("small"); + } + } + + &__header { + font-weight: normal; + margin-bottom: 0; + + @include nhsuk-font-size(19); + + &.nhsuk-u-font-weight-bold .app-u-monospace { + font-weight: inherit; + } + } + + &__content { + :last-child { + margin-bottom: 0; + } + } + + &__description { + color: $nhsuk-secondary-text-color; + padding-top: 0; + + @include nhsuk-responsive-margin(2, "bottom"); + @include nhsuk-font-size(16); + } +} diff --git a/app/components/app_timeline_component.rb b/app/components/app_timeline_component.rb new file mode 100644 index 0000000000..1fe80a1eac --- /dev/null +++ b/app/components/app_timeline_component.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class AppTimelineComponent < ViewComponent::Base + erb_template <<-ERB + + ERB + + def initialize(items) + @items = items + end + + def render? + @items.present? + end +end diff --git a/spec/components/app_timeline_component_spec.rb b/spec/components/app_timeline_component_spec.rb new file mode 100644 index 0000000000..0f1361533e --- /dev/null +++ b/spec/components/app_timeline_component_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +describe AppTimelineComponent do + subject(:rendered) { render_inline(described_class.new(items)) } + + context "when there are no items" do + let(:items) { [] } + + it "does not render" do + expect(rendered.to_html).to be_blank + end + end + + context "when items are present" do + let(:items) do + [ + { heading_text: "Step 1", description: "First step", active: true }, + { + heading_text: "Step 2", + description: "Past step", + is_past_item: true + }, + { heading_text: "Step 3", description: "Future step" } + ] + end + + it "renders a list of timeline items" do + expect(rendered.css("ul.app-timeline li.app-timeline__item").count).to eq( + 3 + ) + end + + it "renders the heading text" do + expect(rendered).to have_text("Step 1") + expect(rendered).to have_text("Step 2") + expect(rendered).to have_text("Step 3") + end + + it "renders the description text" do + expect(rendered).to have_text("First step") + expect(rendered).to have_text("Past step") + expect(rendered).to have_text("Future step") + end + + it "renders a bold header for active items" do + expect(rendered.css("h3.nhsuk-u-font-weight-bold")).to have_text("Step 1") + end + + it "renders a large badge for active and past items" do + expect(rendered.css("svg.app-timeline__badge")).to be_present + end + + it "renders a small badge for future items" do + expect(rendered.css("svg.app-timeline__badge--small")).to be_present + end + end + + context "when an item is blank" do + let(:items) { [nil, { heading_text: "Step X", description: "Valid" }] } + + it "skips rendering blank items" do + expect(rendered.css("li.app-timeline__item").count).to eq(1) + expect(rendered).to have_text("Step X") + end + end +end From a88f395b1f3be0c2e65b43ab6c3980c7d2caaedd Mon Sep 17 00:00:00 2001 From: Lakshmi Murugappan Date: Fri, 29 Aug 2025 23:14:09 +0100 Subject: [PATCH 2/3] Add PDS lookup history timeline Adds a view that can be reached from the global patient record, to view the patient's latest PDS lookup history. This link only appears if there are any PDSSearchResults for the patient and their current NHS number matches the number from the last search result set. --- app/components/app_child_summary_component.rb | 12 +++++- app/controllers/patients_controller.rb | 23 ++++++++++ app/models/patient.rb | 8 ++++ app/models/pds_search_result.rb | 43 +++++++++++++++++++ .../patients/pds_search_history.html.erb | 29 +++++++++++++ config/locales/en.yml | 13 ++++++ config/routes.rb | 3 +- 7 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 app/views/patients/pds_search_history.html.erb diff --git a/app/components/app_child_summary_component.rb b/app/components/app_child_summary_component.rb index 0a202f0da8..11f31c40f9 100644 --- a/app/components/app_child_summary_component.rb +++ b/app/components/app_child_summary_component.rb @@ -19,7 +19,9 @@ def initialize( def call govuk_summary_list( - actions: @change_links.present? || @remove_links.present? + actions: + @change_links.present? || @remove_links.present? || + pds_search_history_link.present? ) do |summary_list| summary_list.with_row do |row| row.with_key { "NHS number" } @@ -30,6 +32,8 @@ def call href:, visually_hidden_text: "NHS number" ) + elsif (href = pds_search_history_link) + row.with_action(text: "PDS history", href:) end end @@ -209,4 +213,10 @@ def format_year_group def highlight_if(value, condition) condition ? tag.span(value, class: "app-highlight") : value end + + def pds_search_history_link + return unless @child.is_a?(Patient) && @child.pds_lookup_match? + + pds_search_history_patient_path(@child) + end end diff --git a/app/controllers/patients_controller.rb b/app/controllers/patients_controller.rb index 41ace8abdd..2cbde29d8e 100644 --- a/app/controllers/patients_controller.rb +++ b/app/controllers/patients_controller.rb @@ -25,6 +25,29 @@ def log def edit end + def pds_search_history + latest_results = @patient.pds_search_results.includes(:import).latest_set + + @timeline_items = + if latest_results.present? + latest_results + .map(&:timeline_item) + .sort_by { |item| item["created_at"] } + else + [] + end + + time = latest_results&.last&.import&.processed_at + + if @patient.nhs_number.present? + @timeline_items << { + active: true, + heading_text: "NHS number is #{@patient.nhs_number}", + description: time&.to_date&.to_fs(:long) + } + end + end + def invite_to_clinic PatientLocation.find_or_create_by!( patient: @patient, diff --git a/app/models/patient.rb b/app/models/patient.rb index 12c4173698..1cf023f4a1 100644 --- a/app/models/patient.rb +++ b/app/models/patient.rb @@ -656,6 +656,14 @@ class NHSNumberMismatch < StandardError class UnknownGPPractice < StandardError end + def latest_pds_search_result + pds_search_results.latest_set&.first&.changeset&.pds_nhs_number + end + + def pds_lookup_match? + nhs_number.present? && nhs_number == latest_pds_search_result + end + private def patient_status(association, programme:, academic_year:) diff --git a/app/models/pds_search_result.rb b/app/models/pds_search_result.rb index 3c9ae9acb1..bf3498dda9 100644 --- a/app/models/pds_search_result.rb +++ b/app/models/pds_search_result.rb @@ -49,4 +49,47 @@ class PDSSearchResult < ApplicationRecord no_postcode: 5 }, validate: true + + def self.grouped_sets + records = all.to_a + grouped_records = + records.group_by do |record| + if record.import_id.present? + [:import, record.import_type, record.import_id] + else + [:date, record.created_at.to_date] + end + end + grouped_records.values + end + + def self.latest_set + grouped_sets.max_by { |set| set.map(&:created_at).max } + end + + def pds_nhs_number + changeset&.pds_nhs_number + end + + def changeset + return unless import_id + + PatientChangeset.find_by( + import_type: import_type, + import_id: import_id, + patient_id: patient_id + ) + end + + def timeline_item + { + is_past_item: true, + heading_text: human_enum_name(:step), + description: + I18n.t( + "activerecord.attributes.#{self.class.model_name.i18n_key}.results.#{result}", + nhs_number: pds_nhs_number || nhs_number + ) + } + end end diff --git a/app/views/patients/pds_search_history.html.erb b/app/views/patients/pds_search_history.html.erb new file mode 100644 index 0000000000..01cc61a706 --- /dev/null +++ b/app/views/patients/pds_search_history.html.erb @@ -0,0 +1,29 @@ +<% content_for :before_main do %> + <%= render AppBreadcrumbComponent.new(items: [ + { text: t("dashboard.index.title"), href: dashboard_path }, + { text: t("patients.index.title"), href: patients_path }, + { text: @patient.full_name, href: patient_path(@patient) }, + ]) %> +<% end %> + +<% page_title = "NHS number lookup history" %> +<%= h1 page_title: do %> + <%= @patient.full_name %> + <%= page_title %> +<% end %> + +

The following timeline shows how this child's NHS number was found by searching the NHS Patient Demographics Service (PDS)

+ +<%= render AppTimelineComponent.new(@timeline_items) %> + +

Terms used in this lookup

+ +

A fuzzy search finds text that matches a term closely as well as exactly. + For example, a fuzzy search can identify Jon Smith even if the term entered was John Smith.

+ +

A wildcard searches for unknown parts of text. For example, if you only + have part of a postcode: CV1, you can use a wildcard to search all records with a postcode + that includes CV1.

+ +

History refers to the child’s history, for example if they have changed + their name or address.

diff --git a/config/locales/en.yml b/config/locales/en.yml index 33ae700522..c3eb0867c7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -214,6 +214,19 @@ en: types: father: dad mother: mum + pds_search_result: + steps: + no_fuzzy_with_history: "Non-fuzzy search (with history)" + no_fuzzy_with_history_daily: "Non-fuzzy search (with history)" + no_fuzzy_without_history: "Non-fuzzy search (without history)" + no_fuzzy_with_wildcard_postcode: "Non-fuzzy search (with history and wildcard postcode)" + no_fuzzy_with_wildcard_given_name: "Non-fuzzy search (with history and wildcard first name)" + no_fuzzy_with_wildcard_family_name: "Non-fuzzy search (with history and wildcard surname)" + fuzzy: "Fuzzy search" + results: + no_matches: "No matches found" + one_match: "Found %{nhs_number}" + too_many_matches: "Too many matches found" programme: types: flu: Flu diff --git a/config/routes.rb b/config/routes.rb index 609c8375cb..8955f7d728 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -175,8 +175,9 @@ end member do - get "log" post "invite-to-clinic" + get "log" + get "pds-search-history" get "edit/nhs-number", controller: "patients/edit", From 5d0a857e20d6a5c1e004da6b3c0d30a275fbaed6 Mon Sep 17 00:00:00 2001 From: Lakshmi Murugappan Date: Thu, 25 Sep 2025 10:27:01 +0100 Subject: [PATCH 3/3] Add tests for PDS lookup history Adds tests for PDS lookup history methods and the new timeline component. --- app/models/pds_search_result.rb | 2 +- .../app_child_summary_component_spec.rb | 32 +++- ...port_child_pds_lookup_extravaganza_spec.rb | 65 ++++++++- spec/models/patient_spec.rb | 82 +++++++++++ spec/models/pds_search_result_spec.rb | 137 ++++++++++++++++++ 5 files changed, 308 insertions(+), 10 deletions(-) diff --git a/app/models/pds_search_result.rb b/app/models/pds_search_result.rb index bf3498dda9..4751ebd4e4 100644 --- a/app/models/pds_search_result.rb +++ b/app/models/pds_search_result.rb @@ -88,7 +88,7 @@ def timeline_item description: I18n.t( "activerecord.attributes.#{self.class.model_name.i18n_key}.results.#{result}", - nhs_number: pds_nhs_number || nhs_number + nhs_number: nhs_number ) } end diff --git a/spec/components/app_child_summary_component_spec.rb b/spec/components/app_child_summary_component_spec.rb index 0714955b8a..d456598ed6 100644 --- a/spec/components/app_child_summary_component_spec.rb +++ b/spec/components/app_child_summary_component_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true describe AppChildSummaryComponent do - subject { render_inline(component) } + subject(:rendered) { render_inline(component) } let(:component) { described_class.new(patient) } @@ -124,4 +124,34 @@ it { should have_text("Archive reason") } it { should have_text("Other: Some details.") } end + + context "with a PDS lookup match" do + let(:patient) { create(:patient) } + + before { allow(patient).to receive(:pds_lookup_match?).and_return(true) } + + it "shows a PDS history action link" do + expect(rendered).to have_link( + "PDS history", + href: + Rails.application.routes.url_helpers.pds_search_history_patient_path( + patient + ) + ) + end + end + + context "without a PDS lookup match" do + let(:patient) { create(:patient) } + + before { allow(patient).to receive(:pds_lookup_match?).and_return(false) } + + it { should_not have_link("PDS history") } + + context "when the record is not a Patient" do + let(:consent_form) { create(:consent_form) } + + it { should_not have_link("PDS history") } + end + end end diff --git a/spec/features/import_child_pds_lookup_extravaganza_spec.rb b/spec/features/import_child_pds_lookup_extravaganza_spec.rb index 14d5fe5fa1..77e26e5006 100644 --- a/spec/features/import_child_pds_lookup_extravaganza_spec.rb +++ b/spec/features/import_child_pds_lookup_extravaganza_spec.rb @@ -24,9 +24,12 @@ # Case 1: Patient with existing NHS number (Albert) - nothing should happen and_i_see_the_patient_uploaded_with_nhs_number and_parents_are_created_for_albert + when_i_click_on_alberts_pds_history + then_i_see_the_pds_lookup_history # Case 2: Existing patient without NHS number (Betty) - should not show duplicate review - and_i_do_not_see_an_import_review_for_the_first_patient_uploaded_without_nhs_number + when_i_go_back_to_the_import_page + then_i_do_not_see_an_import_review_for_the_first_patient_uploaded_without_nhs_number when_i_click_on_the_patient_without_review then_i_see_the_new_patient_has_an_nhs_number and_betty_has_correct_parent_relationships @@ -296,12 +299,14 @@ def and_pds_lookups_dont_return_any_matches def and_pds_lookup_during_import_is_enabled Flipper.enable(:pds_lookup_during_import) - stub_pds_search_to_return_a_patient( - "9999075320", - "family" => "Tweedle", - "given" => "Albert", - "birthdate" => "eq2009-12-29", - "address-postalcode" => "SW11 1EH" + stub_pds_cascading_search( + family_name: "Tweedle", + given_name: "Albert", + birthdate: "eq2009-12-29", + address_postcode: "SW11 1EH", + steps: { + wildcard_family_name: "9999075320" + } ) stub_pds_search_to_return_a_patient( @@ -421,6 +426,30 @@ def stub_pds_cascading_search( end end + def when_i_visit_the_import_page + visit "/" + click_link "Import", match: :first + end + + def when_i_go_back_to_the_import_page + visit "/imports" + click_link "1 September 2025 at 12:00pm" + end + + def when_i_click_on_alberts_pds_history + click_on "TWEEDLE, Albert" + click_link "PDS history" + end + + def when_i_click_review_for(name) + within( + :xpath, + "//div[h3[contains(text(), 'records with import issues')]]" + ) do + within(:xpath, ".//tr[contains(., '#{name}')]") { click_link "Review" } + end + end + def and_i_upload_import_file(filename) travel 1.minute @@ -444,6 +473,26 @@ def then_i_should_see_the_import_failed expect(page).to have_content("12 unmatched records") end + def and_i_start_adding_children_to_the_session + click_on "Import class lists" + end + + def and_i_select_the_year_groups + check "Year 8" + check "Year 9" + check "Year 10" + check "Year 11" + click_on "Continue" + end + + def then_i_should_see_the_import_page + expect(page).to have_content("Import class list") + end + + def then_i_see_the_pds_lookup_history + expect(page).to have_content("NHS number lookup history") + end + def when_i_upload_a_valid_file attach_file( "class_import[csv]", @@ -520,7 +569,7 @@ def and_an_existing_patient_records_exist_in_school ) end - def and_i_do_not_see_an_import_review_for_the_first_patient_uploaded_without_nhs_number + def then_i_do_not_see_an_import_review_for_the_first_patient_uploaded_without_nhs_number expect(page).not_to have_content("Actions Review SAMSON, Betty") end diff --git a/spec/models/patient_spec.rb b/spec/models/patient_spec.rb index 4899e4befb..73b6b5fada 100644 --- a/spec/models/patient_spec.rb +++ b/spec/models/patient_spec.rb @@ -1296,4 +1296,86 @@ end end end + + describe "#latest_pds_search_result" do + subject(:latest_pds_search_result) { patient.latest_pds_search_result } + + let(:patient) { create(:patient) } + + context "with no PDS search results" do + it { should be_nil } + end + + context "with PDS search results but no changeset" do + let(:import) { create(:class_import) } + + before { create(:pds_search_result, patient:, import:) } + + it { should be_nil } + end + + context "with PDS search results and changeset" do + let(:import) { create(:class_import) } + + before do + create( + :patient_changeset, + patient:, + import:, + pds_nhs_number: "9449304130" + ) + create(:pds_search_result, patient:, import:) + end + + it { should eq("9449304130") } + end + end + + describe "#pds_lookup_match?" do + subject(:pds_lookup_match?) { patient.pds_lookup_match? } + + let(:patient) { create(:patient, nhs_number: "9449304130") } + + context "when patient has no NHS number" do + let(:patient) { create(:patient, nhs_number: nil) } + + it { should be(false) } + end + + context "with no PDS search results" do + it { should be(false) } + end + + context "with matching PDS search result" do + let(:import) { create(:class_import) } + + before do + create( + :patient_changeset, + patient:, + import:, + pds_nhs_number: "9449304130" + ) + create(:pds_search_result, patient:, import:) + end + + it { should be(true) } + end + + context "with non-matching PDS search result" do + let(:import) { create(:class_import) } + + before do + create( + :patient_changeset, + patient:, + import:, + pds_nhs_number: "9449310475" + ) + create(:pds_search_result, patient:, import:) + end + + it { should be(false) } + end + end end diff --git a/spec/models/pds_search_result_spec.rb b/spec/models/pds_search_result_spec.rb index 9a6ef10dce..cf2c4d3ff2 100644 --- a/spec/models/pds_search_result_spec.rb +++ b/spec/models/pds_search_result_spec.rb @@ -30,4 +30,141 @@ it { should belong_to(:patient) } it { should belong_to(:import).optional } end + + describe ".grouped_sets" do + subject(:grouped_sets) { described_class.grouped_sets } + + let(:patient) { create(:patient) } + + context "with no records" do + it { should be_empty } + end + + context "with records grouped by import" do + let(:import) { create(:class_import) } + + before do + create(:pds_search_result, patient:, import:) + create(:pds_search_result, patient:, import:) + end + + it "groups records by import" do + expect(grouped_sets.size).to eq(1) + expect(grouped_sets.first.size).to eq(2) + end + end + + context "with records grouped by date" do + before do + travel_to(1.day.ago) do + create(:pds_search_result, patient:, import: nil) + end + create(:pds_search_result, patient:, import: nil) + end + + it "groups records by date when no import" do + expect(grouped_sets.size).to eq(2) + expect(grouped_sets.all? { |set| set.size == 1 }).to be(true) + end + end + end + + describe ".latest_set" do + subject(:latest_set) { described_class.latest_set } + + let(:patient) { create(:patient) } + + context "with no records" do + it { should be_nil } + end + + context "with multiple sets" do + before do + create( + :pds_search_result, + patient:, + import: nil, + created_at: 4.days.ago + ) + create(:pds_search_result, patient:, import: nil, created_at: 1.day.ago) + end + + it "returns the most recent set" do + expect(latest_set.size).to eq(1) + expect(latest_set.first.created_at.to_date).to eq(1.day.ago.to_date) + end + end + end + + describe "#pds_nhs_number" do + subject(:pds_nhs_number) { pds_search_result.pds_nhs_number } + + let(:pds_search_result) { create(:pds_search_result, patient:, import:) } + let(:patient) { create(:patient) } + let(:import) { create(:class_import) } + + context "without a changeset" do + it { should be_nil } + end + + context "with a changeset" do + before do + create( + :patient_changeset, + patient:, + import:, + pds_nhs_number: "9449304130" + ) + end + + it { should eq("9449304130") } + end + end + + describe "#changeset" do + subject(:changeset) { pds_search_result.changeset } + + let(:pds_search_result) { create(:pds_search_result, patient:, import:) } + let(:patient) { create(:patient) } + let(:import) { create(:class_import) } + + context "without import_id" do + let(:pds_search_result) do + create(:pds_search_result, patient:, import: nil) + end + + it { should be_nil } + end + + context "with matching changeset" do + let!(:patient_changeset) { create(:patient_changeset, patient:, import:) } + + it { should eq(patient_changeset) } + end + + context "without matching changeset" do + it { should be_nil } + end + end + + describe "#timeline_item" do + subject(:timeline_item) { pds_search_result.timeline_item } + + let(:pds_search_result) do + create( + :pds_search_result, + step: :fuzzy, + result: :one_match, + nhs_number: "9449304130" + ) + end + + it "returns timeline item hash" do + expect(timeline_item).to include( + is_past_item: true, + heading_text: "Fuzzy search", + description: kind_of(String) + ) + end + end end