From 1bdb1e912053b7681648a5b5fff9a7ac5e70658a Mon Sep 17 00:00:00 2001 From: Mooktakim Ahmed Date: Mon, 30 Sep 2024 11:13:01 +0100 Subject: [PATCH 1/3] [CPDLP-3586] NPQ Internal API - Implement DQT API calls in NPQ --- .env.template | 3 + .env.test | 2 + app/services/dqt/v1/teacher.rb | 25 ++ app/services/participant_validator.rb | 37 +-- .../services/participant_validator_spec.rb | 221 +++++++++++------- spec/services/dqt/v1/teacher_spec.rb | 151 ++++++++++++ 6 files changed, 342 insertions(+), 97 deletions(-) create mode 100644 app/services/dqt/v1/teacher.rb create mode 100644 spec/services/dqt/v1/teacher_spec.rb diff --git a/.env.template b/.env.template index 3bdd1ef4e4..49aaba8316 100644 --- a/.env.template +++ b/.env.template @@ -16,3 +16,6 @@ TRA_OIDC_REDIRECT_URI=http://localhost:3000/users/auth/tra_openid_connect/callba QUALIFIED_TEACHERS_API_URL="https://qualified-teachers-api.example.com" QUALIFIED_TEACHERS_API_KEY="some-apikey-guid" + +DQT_API_URL="https://preprod.teacher-qualifications-api.education.gov.uk" +DQT_API_KEY="preprod-apikey" diff --git a/.env.test b/.env.test index 6b8c611ec6..25203a6b3a 100644 --- a/.env.test +++ b/.env.test @@ -3,3 +3,5 @@ ECF_APP_BASE_URL="https://ecf-app.gov.uk" ECF_APP_BEARER_TOKEN="ECFAPPBEARERTOKEN" QUALIFIED_TEACHERS_API_URL="https://qualified-teachers-api.example.com" QUALIFIED_TEACHERS_API_KEY="some-apikey-guid" +DQT_API_URL="https://dqt-api.example.com" +DQT_API_KEY="test-apikey" diff --git a/app/services/dqt/v1/teacher.rb b/app/services/dqt/v1/teacher.rb new file mode 100644 index 0000000000..627ddacc23 --- /dev/null +++ b/app/services/dqt/v1/teacher.rb @@ -0,0 +1,25 @@ +module Dqt + module V1 + class Teacher + include HTTParty + + format :json + base_uri ENV["DQT_API_URL"] + headers "Authorization" => "Bearer #{ENV["DQT_API_KEY"]}" + + def self.validate_trn(trn:, birthdate:, nino: nil) + path = "/v1/teachers/#{trn}" + query = { + birthdate:, + } + query[:nino] = nino if nino + + response = get(path, query:) + + if response.success? + response.slice("trn", "active_alert") + end + end + end + end +end diff --git a/app/services/participant_validator.rb b/app/services/participant_validator.rb index 3a4bd96977..b14e1d2e02 100644 --- a/app/services/participant_validator.rb +++ b/app/services/participant_validator.rb @@ -9,20 +9,10 @@ def initialize(trn:, full_name:, date_of_birth:, national_insurance_number: nil) end def call - return if Feature.ecf_api_disabled? - - request = Net::HTTP::Post.new(uri) - request["Authorization"] = "Bearer #{config.bearer_token}" - request.set_form_data(payload) - - response = Net::HTTP.start(uri.hostname, uri.port, use_ssl: use_ssl?, read_timeout: 20) do |http| - http.request(request) - end - - if response.code == "404" - nil + if Feature.ecf_api_disabled? + call_with_dqt else - OpenStruct.new(JSON.parse(response.body)["data"]["attributes"]) + call_with_ecf end end @@ -50,4 +40,25 @@ def use_ssl? def dob_as_string date_of_birth.iso8601 end + + def call_with_dqt + record = Dqt::V1::Teacher.validate_trn(trn:, birthdate: dob_as_string, nino: national_insurance_number) + OpenStruct.new(record) if record + end + + def call_with_ecf + request = Net::HTTP::Post.new(uri) + request["Authorization"] = "Bearer #{config.bearer_token}" + request.set_form_data(payload) + + response = Net::HTTP.start(uri.hostname, uri.port, use_ssl: use_ssl?, read_timeout: 20) do |http| + http.request(request) + end + + if response.code == "404" + nil + else + OpenStruct.new(JSON.parse(response.body)["data"]["attributes"]) + end + end end diff --git a/spec/lib/services/participant_validator_spec.rb b/spec/lib/services/participant_validator_spec.rb index 48e926848e..ce1bdf3939 100644 --- a/spec/lib/services/participant_validator_spec.rb +++ b/spec/lib/services/participant_validator_spec.rb @@ -7,7 +7,7 @@ full_name:, date_of_birth:, national_insurance_number:, - ) + ).call end let(:trn) { rand(1_000_000..9_999_999).to_s } @@ -16,107 +16,160 @@ let(:national_insurance_number) { "AB123456C" } describe "#call" do - context "when matching trn found" do - let(:body) do - { - data: { - attributes: { + context "when ecf_api_disabled is false" do + before { allow(Feature).to receive(:ecf_api_disabled?).and_return(false) } + + context "when matching trn found" do + let(:body) do + { + data: { + attributes: { + trn:, + qts: true, + active_alert: false, + }, + }, + } + end + + before do + stub_request(:post, + "https://ecf-app.gov.uk/api/v1/participant-validation") + .with( + headers: { + "Authorization" => "Bearer ECFAPPBEARERTOKEN", + }, + body: { trn:, - qts: true, - active_alert: false, + date_of_birth: date_of_birth.iso8601, + full_name:, + nino: national_insurance_number, }, - }, - } - end + ) + .to_return(status: 200, body: body.to_json, headers: {}) + end - before do - stub_request(:post, - "https://ecf-app.gov.uk/api/v1/participant-validation") - .with( - headers: { - "Authorization" => "Bearer ECFAPPBEARERTOKEN", - }, - body: { - trn:, - date_of_birth: date_of_birth.iso8601, - full_name:, - nino: national_insurance_number, - }, - ) - .to_return(status: 200, body: body.to_json, headers: {}) + it "returns record with matching trn" do + expect(subject.trn).to eql(trn) + expect(subject.active_alert).to be_falsey + end end - it "returns record with matching trn" do - expect(subject.call.trn).to eql(trn) - expect(subject.call.active_alert).to be_falsey - end - end + context "when different trn found by fuzzy matching" do + let(:body) do + { + data: { + attributes: { + trn: (trn.to_i + 1).to_s, + qts: true, + active_alert: false, + }, + }, + } + end - context "when different trn found by fuzzy matching" do - let(:body) do - { - data: { - attributes: { - trn: (trn.to_i + 1).to_s, - qts: true, - active_alert: false, + before do + stub_request(:post, + "https://ecf-app.gov.uk/api/v1/participant-validation") + .with( + headers: { + "Authorization" => "Bearer ECFAPPBEARERTOKEN", }, - }, - } - end + body: { + trn:, + date_of_birth: date_of_birth.iso8601, + full_name:, + nino: national_insurance_number, + }, + ) + .to_return(status: 200, body: body.to_json, headers: {}) + end - before do - stub_request(:post, - "https://ecf-app.gov.uk/api/v1/participant-validation") - .with( - headers: { - "Authorization" => "Bearer ECFAPPBEARERTOKEN", - }, - body: { - trn:, - date_of_birth: date_of_birth.iso8601, - full_name:, - nino: national_insurance_number, - }, - ) - .to_return(status: 200, body: body.to_json, headers: {}) + it "returns record with diffrent trn" do + expect(subject.trn).to be_present + expect(subject.trn).not_to eql(trn) + expect(subject.active_alert).to be_falsey + end end - it "returns record with diffrent trn" do - expect(subject.call.trn).to be_present - expect(subject.call.trn).not_to eql(trn) - expect(subject.call.active_alert).to be_falsey + context "when no record could be found" do + before do + stub_request(:post, + "https://ecf-app.gov.uk/api/v1/participant-validation") + .with( + headers: { + "Authorization" => "Bearer ECFAPPBEARERTOKEN", + }, + body: { + trn:, + date_of_birth: date_of_birth.iso8601, + full_name:, + nino: national_insurance_number, + }, + ) + .to_return(status: 404, body: "", headers: {}) + end + + it "returns nil" do + expect(subject).to be_nil + end end end - context "when no record could be found" do - before do - stub_request(:post, - "https://ecf-app.gov.uk/api/v1/participant-validation") - .with( - headers: { - "Authorization" => "Bearer ECFAPPBEARERTOKEN", - }, - body: { - trn:, - date_of_birth: date_of_birth.iso8601, - full_name:, - nino: national_insurance_number, - }, - ) - .to_return(status: 404, body: "", headers: {}) + context "when ecf_api_disabled is true" do + before { allow(Feature).to receive(:ecf_api_disabled?).and_return(true) } + + context "when matching trn found" do + let(:body) do + { + "trn" => trn, + "active_alert" => false, + } + end + + before do + allow(Dqt::V1::Teacher).to receive(:validate_trn) + .with(trn:, birthdate: date_of_birth.iso8601, nino: national_insurance_number) + .and_return(body) + end + + it "returns record with matching trn" do + expect(subject.trn).to eq(trn) + expect(subject.active_alert).to be(false) + end end - it "returns nil" do - expect(subject.call).to be_nil + context "when different trn found by fuzzy matching" do + let(:body) do + { + "trn" => (trn.to_i + 1).to_s, + "active_alert" => false, + } + end + + before do + allow(Dqt::V1::Teacher).to receive(:validate_trn) + .with(trn:, birthdate: date_of_birth.iso8601, nino: national_insurance_number) + .and_return(body) + end + + it "returns record with diffrent trn" do + expect(subject.trn).to be_present + expect(subject.trn).not_to eql(trn) + expect(subject.active_alert).to be_falsey + end end - end - context "when ecf_api_disabled flag is toggled on" do - before { Flipper.enable(Feature::ECF_API_DISABLED) } + context "when no record could be found" do + before do + allow(Dqt::V1::Teacher).to receive(:validate_trn) + .with(trn:, birthdate: date_of_birth.iso8601, nino: national_insurance_number) + .and_return(nil) + end - it "returns nil" do - expect(subject.call).to be_nil + it "returns nil" do + expect(subject).to be_nil + end end end end diff --git a/spec/services/dqt/v1/teacher_spec.rb b/spec/services/dqt/v1/teacher_spec.rb new file mode 100644 index 0000000000..72032f1ee6 --- /dev/null +++ b/spec/services/dqt/v1/teacher_spec.rb @@ -0,0 +1,151 @@ +require "rails_helper" + +RSpec.describe Dqt::V1::Teacher do + subject { described_class } + + let(:trn) { "1001000" } + let(:incorrect_trn) { "1001009" } + let(:birthdate) { Date.new(1987, 12, 13) } + let(:nino) { "AB123456D" } + let(:active_alert) { true } + + let(:response_hash) do + { + "trn": trn, + "ni_number": "AB123456D", + "name": "Mostly Populated", + "dob": "1987-12-13", + "active_alert": active_alert, + "state": 0, + "state_name": "Active", + "qualified_teacher_status": { + "name": "Qualified teacher (trained)", + "qts_date": "2021-07-05T00:00:00Z", + "state": 0, + "state_name": "Active", + }, + "induction": { + "start_date": "2021-07-01T00:00:00Z", + "completion_date": "2021-07-05T00:00:00Z", + "status": "Pass", + "state": 0, + "state_name": "Active", + }, + "initial_teacher_training": { + "programme_start_date": "2021-06-27T00:00:00Z", + "programme_end_date": "2021-07-04T00:00:00Z", + "programme_type": "Overseas Trained Teacher Programme", + "result": "Pass", + "subject1": "applied biology", + "subject2": "applied chemistry", + "subject3": "applied computing", + "qualification": "BA (Hons)", + "state": 0, + "state_name": "Active", + }, + "qualifications": [ + { + "name": "Higher Education", + "date_awarded": nil, + }, + { + "name": "NPQH", + "date_awarded": "2021-07-05T00:00:00Z", + }, + { + "name": "Mandatory Qualification", + "date_awarded": nil, + }, + { + "name": "HLTA", + "date_awarded": nil, + }, + { + "name": "NPQML", + "date_awarded": "2021-07-05T00:00:00Z", + }, + { + "name": "NPQSL", + "date_awarded": "2021-07-04T00:00:00Z", + }, + { + "name": "NPQEL", + "date_awarded": "2021-07-04T00:00:00Z", + }, + ], + } + end + + let(:stub_api_request) do + stub_request(:get, "https://dqt-api.example.com/v1/teachers/#{trn}?birthdate=#{birthdate}") + .with( + headers: { + "Accept" => "*/*", + "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", + "Authorization" => "Bearer test-apikey", + "User-Agent" => "Ruby", + }, + ) + .to_return(status: 200, body: response_hash.to_json, headers: {}) + end + + let(:stub_api_404_request) do + stub_request(:get, "https://dqt-api.example.com/v1/teachers/#{trn}?birthdate=#{birthdate}") + .with( + headers: { + "Accept" => "*/*", + "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", + "Authorization" => "Bearer test-apikey", + "User-Agent" => "Ruby", + }, + ) + .to_return(status: 404, body: nil, headers: {}) + end + + let(:stub_api_different_record_request) do + stub_request(:get, "https://dqt-api.example.com/v1/teachers/#{incorrect_trn}?birthdate=#{birthdate}&nino=#{nino}") + .with( + headers: { + "Accept" => "*/*", + "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", + "Authorization" => "Bearer test-apikey", + "User-Agent" => "Ruby", + }, + ) + .to_return(status: 200, body: response_hash.to_json, headers: {}) + end + + describe ".validate_trn" do + it "returns teacher record" do + stub_api_request + + record = subject.validate_trn(trn:, birthdate:) + + expect(record["trn"]).to eq(trn) + expect(record["active_alert"]).to be(true) + end + + context "when record does not exist" do + it "returns nil" do + stub_api_404_request + + record = subject.validate_trn(trn:, birthdate:) + + expect(record).to be_nil + end + end + + context "with incorrect trn but correct nino" do + let(:active_alert) { false } + + it "returns correct record" do + stub_api_different_record_request + + record = subject.validate_trn(trn: incorrect_trn, birthdate:, nino:) + + expect(record["trn"]).to eql(trn) + expect(record["active_alert"]).to be(false) + end + end + end +end From b28ef44305ae61e479ee0b4a22f0992bf2c33151 Mon Sep 17 00:00:00 2001 From: Mooktakim Ahmed Date: Tue, 1 Oct 2024 10:14:52 +0100 Subject: [PATCH 2/3] [CPDLP-3586] Fixed up the rspec tests --- .../services/participant_validator_spec.rb | 26 +++----- spec/services/dqt/v1/teacher_spec.rb | 60 ------------------- 2 files changed, 8 insertions(+), 78 deletions(-) diff --git a/spec/lib/services/participant_validator_spec.rb b/spec/lib/services/participant_validator_spec.rb index ce1bdf3939..a2febe8222 100644 --- a/spec/lib/services/participant_validator_spec.rb +++ b/spec/lib/services/participant_validator_spec.rb @@ -117,7 +117,13 @@ end context "when ecf_api_disabled is true" do - before { allow(Feature).to receive(:ecf_api_disabled?).and_return(true) } + before do + allow(Feature).to receive(:ecf_api_disabled?).and_return(true) + + allow(Dqt::V1::Teacher).to receive(:validate_trn) + .with(trn:, birthdate: date_of_birth.iso8601, nino: national_insurance_number) + .and_return(body) + end context "when matching trn found" do let(:body) do @@ -127,12 +133,6 @@ } end - before do - allow(Dqt::V1::Teacher).to receive(:validate_trn) - .with(trn:, birthdate: date_of_birth.iso8601, nino: national_insurance_number) - .and_return(body) - end - it "returns record with matching trn" do expect(subject.trn).to eq(trn) expect(subject.active_alert).to be(false) @@ -147,12 +147,6 @@ } end - before do - allow(Dqt::V1::Teacher).to receive(:validate_trn) - .with(trn:, birthdate: date_of_birth.iso8601, nino: national_insurance_number) - .and_return(body) - end - it "returns record with diffrent trn" do expect(subject.trn).to be_present expect(subject.trn).not_to eql(trn) @@ -161,11 +155,7 @@ end context "when no record could be found" do - before do - allow(Dqt::V1::Teacher).to receive(:validate_trn) - .with(trn:, birthdate: date_of_birth.iso8601, nino: national_insurance_number) - .and_return(nil) - end + let(:body) { nil } it "returns nil" do expect(subject).to be_nil diff --git a/spec/services/dqt/v1/teacher_spec.rb b/spec/services/dqt/v1/teacher_spec.rb index 72032f1ee6..8ba1caf550 100644 --- a/spec/services/dqt/v1/teacher_spec.rb +++ b/spec/services/dqt/v1/teacher_spec.rb @@ -12,67 +12,7 @@ let(:response_hash) do { "trn": trn, - "ni_number": "AB123456D", - "name": "Mostly Populated", - "dob": "1987-12-13", "active_alert": active_alert, - "state": 0, - "state_name": "Active", - "qualified_teacher_status": { - "name": "Qualified teacher (trained)", - "qts_date": "2021-07-05T00:00:00Z", - "state": 0, - "state_name": "Active", - }, - "induction": { - "start_date": "2021-07-01T00:00:00Z", - "completion_date": "2021-07-05T00:00:00Z", - "status": "Pass", - "state": 0, - "state_name": "Active", - }, - "initial_teacher_training": { - "programme_start_date": "2021-06-27T00:00:00Z", - "programme_end_date": "2021-07-04T00:00:00Z", - "programme_type": "Overseas Trained Teacher Programme", - "result": "Pass", - "subject1": "applied biology", - "subject2": "applied chemistry", - "subject3": "applied computing", - "qualification": "BA (Hons)", - "state": 0, - "state_name": "Active", - }, - "qualifications": [ - { - "name": "Higher Education", - "date_awarded": nil, - }, - { - "name": "NPQH", - "date_awarded": "2021-07-05T00:00:00Z", - }, - { - "name": "Mandatory Qualification", - "date_awarded": nil, - }, - { - "name": "HLTA", - "date_awarded": nil, - }, - { - "name": "NPQML", - "date_awarded": "2021-07-05T00:00:00Z", - }, - { - "name": "NPQSL", - "date_awarded": "2021-07-04T00:00:00Z", - }, - { - "name": "NPQEL", - "date_awarded": "2021-07-04T00:00:00Z", - }, - ], } end From 1c08d622074a875c10059ac48df74725e0365aac Mon Sep 17 00:00:00 2001 From: Mooktakim Ahmed Date: Wed, 2 Oct 2024 18:22:43 +0100 Subject: [PATCH 3/3] [CPDLP-3586] Migrated over DQTRecordCheck logic to NPQ --- app/services/dqt/record_check.rb | 84 ++++++ app/services/dqt/teacher_record.rb | 18 ++ app/services/dqt/v1/teacher.rb | 11 +- app/services/name_matcher.rb | 34 +++ app/services/participant_validator.rb | 6 +- app/services/teacher_reference_number.rb | 36 +++ .../services/participant_validator_spec.rb | 60 ++--- spec/services/dqt/record_check_spec.rb | 240 ++++++++++++++++++ spec/services/dqt/teacher_record_spec.rb | 48 ++++ spec/services/dqt/v1/teacher_spec.rb | 17 +- spec/services/name_matcher_spec.rb | 41 +++ .../services/teacher_reference_number_spec.rb | 70 +++++ 12 files changed, 628 insertions(+), 37 deletions(-) create mode 100644 app/services/dqt/record_check.rb create mode 100644 app/services/dqt/teacher_record.rb create mode 100644 app/services/name_matcher.rb create mode 100644 app/services/teacher_reference_number.rb create mode 100644 spec/services/dqt/record_check_spec.rb create mode 100644 spec/services/dqt/teacher_record_spec.rb create mode 100644 spec/services/name_matcher_spec.rb create mode 100644 spec/services/teacher_reference_number_spec.rb diff --git a/app/services/dqt/record_check.rb b/app/services/dqt/record_check.rb new file mode 100644 index 0000000000..2f82ef605f --- /dev/null +++ b/app/services/dqt/record_check.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module Dqt + class RecordCheck + TITLES = %w[mr mrs miss ms dr prof rev].freeze + + CheckResult = Struct.new( + :dqt_record, + :trn_matches, + :name_matches, + :dob_matches, + :nino_matches, + :total_matched, + :failure_reason, + ) + + def call + check_record + end + + private + + attr_reader :trn, :nino, :full_name, :date_of_birth, :check_first_name_only + + def initialize(trn:, full_name:, date_of_birth:, nino: nil, check_first_name_only: true) + @trn = trn + @full_name = full_name&.strip + @date_of_birth = date_of_birth + @nino = nino + @check_first_name_only = check_first_name_only + end + + def dqt_record(padded_trn) + V1::Teacher.find(trn: padded_trn, nino:, birthdate: date_of_birth) + end + + def check_record + return check_failure(:trn_and_nino_blank) if trn.blank? && nino.blank? + + @trn = "0000001" if trn.blank? + + padded_trn = TeacherReferenceNumber.new(trn).formatted_trn + dqt_record = TeacherRecord.new(dqt_record(padded_trn)) + + return check_failure(:no_match_found) if dqt_record.blank? + return check_failure(:found_but_not_active) unless dqt_record.active? + + trn_matches = dqt_record.trn == padded_trn + name_matches = name_matches?(dqt_name: dqt_record.name) + dob_matches = dqt_record.dob == date_of_birth + nino_matches = nino.present? && nino.downcase == dqt_record.ni_number&.downcase + + matches = [trn_matches, name_matches, dob_matches, nino_matches].count(true) + + if matches >= 3 + CheckResult.new(dqt_record, trn_matches, name_matches, dob_matches, nino_matches, matches) + elsif matches < 3 && (trn_matches && trn != "1") + if matches == 2 && !name_matches && check_first_name_only + CheckResult.new(dqt_record, trn_matches, name_matches, dob_matches, nino_matches, matches) + else + # If a participant mistypes their TRN and enters someone else's, we should search by NINO instead + # The API first matches by (mandatory) TRN, then by NINO if it finds no results. This works around that. + @trn = "0000001" + check_record + end + else + # we found a record but not enough matched + check_failure(:no_match_found) + end + end + + def name_matches?(dqt_name:) + return false if full_name.blank? + return false if full_name.in?(TITLES) + return false if dqt_name.blank? + + NameMatcher.new(full_name, dqt_name, check_first_name_only:).matches? + end + + def check_failure(reason) + CheckResult.new(nil, false, false, false, false, 0, reason) + end + end +end diff --git a/app/services/dqt/teacher_record.rb b/app/services/dqt/teacher_record.rb new file mode 100644 index 0000000000..1668da0559 --- /dev/null +++ b/app/services/dqt/teacher_record.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Dqt + class TeacherRecord + include ActiveModel::Model + + attr_accessor :trn, + :state_name, + :name, + :dob, + :ni_number, + :active_alert + + def active? + state_name == "Active" + end + end +end diff --git a/app/services/dqt/v1/teacher.rb b/app/services/dqt/v1/teacher.rb index 627ddacc23..cbf7206015 100644 --- a/app/services/dqt/v1/teacher.rb +++ b/app/services/dqt/v1/teacher.rb @@ -7,7 +7,7 @@ class Teacher base_uri ENV["DQT_API_URL"] headers "Authorization" => "Bearer #{ENV["DQT_API_KEY"]}" - def self.validate_trn(trn:, birthdate:, nino: nil) + def self.find(trn:, birthdate:, nino: nil) path = "/v1/teachers/#{trn}" query = { birthdate:, @@ -17,7 +17,14 @@ def self.validate_trn(trn:, birthdate:, nino: nil) response = get(path, query:) if response.success? - response.slice("trn", "active_alert") + response.slice( + "trn", + "state_name", + "name", + "dob", + "ni_number", + "active_alert", + ) end end end diff --git a/app/services/name_matcher.rb b/app/services/name_matcher.rb new file mode 100644 index 0000000000..9bef946bdc --- /dev/null +++ b/app/services/name_matcher.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class NameMatcher + attr_reader :name1, :name2, :check_first_name_only + + TITLES = /\A((mr|mrs|miss|ms|dr|prof|rev)\.?)/ + + def initialize(name1, name2, check_first_name_only: true) + @name1 = name1 + @name2 = name2 + @check_first_name_only = check_first_name_only + end + + def matches? + if check_first_name_only? + first_name(name1).downcase == first_name(name2).downcase + else + strip_title(name1).downcase == strip_title(name2).downcase + end + end + +private + + alias_method :check_first_name_only?, :check_first_name_only + + def first_name(name) + strip_title(name).split(" ").first + end + + def strip_title(str) + parts = str.split(" ") + parts.first.downcase =~ TITLES ? parts.drop(1).join(" ") : str + end +end diff --git a/app/services/participant_validator.rb b/app/services/participant_validator.rb index b14e1d2e02..7f2feedc48 100644 --- a/app/services/participant_validator.rb +++ b/app/services/participant_validator.rb @@ -42,8 +42,10 @@ def dob_as_string end def call_with_dqt - record = Dqt::V1::Teacher.validate_trn(trn:, birthdate: dob_as_string, nino: national_insurance_number) - OpenStruct.new(record) if record + result = Dqt::RecordCheck.new(**payload.merge(check_first_name_only: true)).call + if result.total_matched >= 3 + result.dqt_record + end end def call_with_ecf diff --git a/app/services/teacher_reference_number.rb b/app/services/teacher_reference_number.rb new file mode 100644 index 0000000000..3f4893048a --- /dev/null +++ b/app/services/teacher_reference_number.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class TeacherReferenceNumber + MIN_UNPADDED_TRN_LENGTH = 5 + PADDED_TRN_LENGTH = 7 + + attr_reader :trn, :format_error + + def initialize(trn) + @trn = trn + @format_error = nil + end + + def formatted_trn + @formatted_trn ||= extract_trn_value + end + + def valid? + formatted_trn.present? + end + +private + + def extract_trn_value + @format_error = :blank and return if trn.blank? + + # remove any characters that are not digits + only_digits = trn.to_s.gsub(/[^\d]/, "") + + @format_error = :invalid and return if only_digits.blank? + @format_error = :too_short and return if only_digits.length < MIN_UNPADDED_TRN_LENGTH + @format_error = :too_long and return if only_digits.length > PADDED_TRN_LENGTH + + only_digits.rjust(PADDED_TRN_LENGTH, "0") + end +end diff --git a/spec/lib/services/participant_validator_spec.rb b/spec/lib/services/participant_validator_spec.rb index a2febe8222..2855e3e97a 100644 --- a/spec/lib/services/participant_validator_spec.rb +++ b/spec/lib/services/participant_validator_spec.rb @@ -117,45 +117,47 @@ end context "when ecf_api_disabled is true" do + let(:total_matched) { 3 } + let(:body) do + { + "trn" => trn, + "active_alert" => true, + } + end + let(:dqt_result) do + OpenStruct.new( + dqt_record: Dqt::TeacherRecord.new(body), + total_matched:, + ) + end + before do allow(Feature).to receive(:ecf_api_disabled?).and_return(true) - allow(Dqt::V1::Teacher).to receive(:validate_trn) - .with(trn:, birthdate: date_of_birth.iso8601, nino: national_insurance_number) - .and_return(body) - end + service = instance_double(Dqt::RecordCheck) + allow(service).to receive(:call).and_return(dqt_result) - context "when matching trn found" do - let(:body) do - { - "trn" => trn, - "active_alert" => false, - } - end - - it "returns record with matching trn" do - expect(subject.trn).to eq(trn) - expect(subject.active_alert).to be(false) - end + allow(Dqt::RecordCheck).to receive(:new) + .with( + trn:, + full_name:, + date_of_birth: date_of_birth.iso8601, + nino: national_insurance_number, + check_first_name_only: true, + ).and_return(service) end - context "when different trn found by fuzzy matching" do - let(:body) do - { - "trn" => (trn.to_i + 1).to_s, - "active_alert" => false, - } - end + context "when total_matched is 3" do + let(:total_matched) { 3 } - it "returns record with diffrent trn" do - expect(subject.trn).to be_present - expect(subject.trn).not_to eql(trn) - expect(subject.active_alert).to be_falsey + it "returns teacher record" do + expect(subject.trn).to eq(trn) + expect(subject.active_alert).to be(true) end end - context "when no record could be found" do - let(:body) { nil } + context "when total_matched is 2" do + let(:total_matched) { 2 } it "returns nil" do expect(subject).to be_nil diff --git a/spec/services/dqt/record_check_spec.rb b/spec/services/dqt/record_check_spec.rb new file mode 100644 index 0000000000..13578e9e0b --- /dev/null +++ b/spec/services/dqt/record_check_spec.rb @@ -0,0 +1,240 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Dqt::RecordCheck do + shared_context "with fake DQT response" do + before do + allow(Dqt::V1::Teacher).to(receive(:find).with(trn:, birthdate: date_of_birth, nino:).and_return(fake_api_response || default_api_response)) + end + end + + let(:trn) { "1234567" } + let(:nino) { "QQ123456A" } + let(:date_of_birth) { 25.years.ago.to_date } + let(:full_name) { "Mr Nelson Muntz" } + let(:kwargs) { { full_name:, trn:, date_of_birth:, nino: } } + let(:default_api_response) do + { + "state_name" => "Active", + "trn" => trn, + "name" => full_name, + "ni_number" => nino, + "dob" => 25.years.ago.to_date, + "active_alert": true, + } + end + let(:fake_api_response) { nil } + + subject { described_class.new(**kwargs) } + + context "when trn and national insurance number are blank" do + let(:trn) { "" } + let(:nino) { "" } + + include_context "with fake DQT response" + + it { expect(subject.call.failure_reason).to be(:trn_and_nino_blank) } + end + + context "when inactive" do + include_context "with fake DQT response" do + let(:fake_api_response) { { "state_name" => "Inactive" } } + end + + it { expect(subject.call.failure_reason).to be(:found_but_not_active) } + end + + context "when active" do + describe "matching on TRN" do + context "when exact" do + include_context "with fake DQT response" + + it("#trn_matches is true") { expect(subject.call.trn_matches).to be(true) } + end + + context "when different" do + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("trn" => "9988776") } + end + + it("#trn_matches is false") { expect(subject.call.trn_matches).to be(false) } + end + end + + describe "matching on name" do + context "when check_first_name_only: true" do + context "when exact" do + include_context "with fake DQT response" + + it("#name_matches is true") { expect(subject.call.name_matches).to be(true) } + end + + context "when there is whitespace around the supplied name" do + let(:full_name) { " Mr Nelson Muntz " } + + include_context "with fake DQT response" + + it("#name_matches is true") { expect(subject.call.name_matches).to be(true) } + end + + context "when there is whitespace around the name in the API response" do + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("name" => " #{full_name} ") } + end + + it("#name_matches is true") { expect(subject.call.name_matches).to be(true) } + end + + context "when first names are different but surnames are the same" do + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("name" => "Mr Eddie Muntz") } + end + + it("#name_matches is false") { expect(subject.call.name_matches).to be(false) } + end + + context "when full_name is blank" do + let(:full_name) { "" } + + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("name" => "Nelson Muntz") } + end + + it("#name_matches is false") { expect(subject.call.name_matches).to be(false) } + end + + context "when full_name is title" do + let(:full_name) { "mr" } + + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("name" => "Nelson Muntz") } + end + + it("#name_matches is false") { expect(subject.call.name_matches).to be(false) } + end + end + + context "when check_first_name_only: false" do + let(:kwargs) { { full_name:, trn:, date_of_birth:, nino:, check_first_name_only: false } } + + context "when exact" do + include_context "with fake DQT response" + + it("#name_matches is true") { expect(subject.call.name_matches).to be(true) } + end + + context "when first names match but surnames are different" do + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("name" => "Mr Nelson Piquet") } + end + + it("#name_matches is false") { expect(subject.call.name_matches).to be(false) } + end + + context "when full_name is blank" do + let(:full_name) { nil } + + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("name" => "Nelson Muntz") } + end + + it("#name_matches is false") { expect(subject.call.name_matches).to be(false) } + end + + context "when full_name is title" do + let(:full_name) { "mr" } + + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("name" => "Nelson Muntz") } + end + + it("#name_matches is false") { expect(subject.call.name_matches).to be(false) } + end + end + end + + describe "matching on date of birth" do + context "when exact" do + include_context "with fake DQT response" + + it("#dob_matches is true") { expect(subject.call.dob_matches).to be(true) } + end + + context "when different" do + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("dob" => 27.years.ago.to_date) } + end + + it("#dob_matches is false") { expect(subject.call.dob_matches).to be(false) } + end + end + + describe "matching on national insurance number" do + context "when exact" do + include_context "with fake DQT response" + + it("#nino_matches is true") { expect(subject.call.nino_matches).to be(true) } + end + + context "when blank" do + include_context "with fake DQT response" do + let(:nino) { nil } + end + + it("#nino_matches is false") { expect(subject.call.nino_matches).to be(false) } + end + + context "when different" do + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.merge("ni_number" => "ZZ123456X") } + end + + it("#nino_matches is false") { expect(subject.call.nino_matches).to be(false) } + end + end + + describe "overall match status" do + include_context "with fake DQT response" + + context "when everything matches" do + it("#total_matches is 4") { expect(subject.call.total_matched).to eq(4) } + it("#failure_reason is nil") { expect(subject.call.failure_reason).to be_nil } + end + end + + context "when there are less than three matches excluding TRN" do + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.except("dob").merge("ni_number" => "QQ121212Q") } + end + + before do + allow_any_instance_of(Dqt::RecordCheck).to receive(:check_record).and_call_original + end + + it "sets trn to 0000001 and calls check_record again" do + allow(Dqt::V1::Teacher).to(receive(:find).with(trn: "0000001", birthdate: date_of_birth, nino:).and_return(fake_api_response || default_api_response)) + + expect(subject.send(:trn)).to eq(trn) + + subject.call + + expect(subject.send(:trn)).to eq("0000001") + end + + context "when the TRN matches and DoB or Nino but the name doesn't match (2 matches)" do + include_context "with fake DQT response" do + let(:fake_api_response) { default_api_response.except("dob").merge("name" => "Jimbo Jones") } + end + + it "returns the record and match results" do + result = subject.call + expect(result.trn_matches).to be(true) + expect(result.name_matches).to be(false) + expect(result.dqt_record).to be_present + expect(result.total_matched).to eq(2) + end + end + end + end +end diff --git a/spec/services/dqt/teacher_record_spec.rb b/spec/services/dqt/teacher_record_spec.rb new file mode 100644 index 0000000000..e23d1294a0 --- /dev/null +++ b/spec/services/dqt/teacher_record_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Dqt::TeacherRecord, type: :model do + let(:state_name) { "Active" } + let(:attributes) do + { + trn: "123456", + state_name:, + name: "John Doe", + dob: Date.new(1990, 1, 1).iso8601, + ni_number: "AB123456C", + active_alert: true, + } + end + + subject { described_class.new(attributes) } + + describe "attributes" do + it "returns correct attributes" do + expect(subject.trn).to eq("123456") + expect(subject.state_name).to eq("Active") + expect(subject.name).to eq("John Doe") + expect(subject.dob).to eq("1990-01-01") + expect(subject.ni_number).to eq("AB123456C") + expect(subject.active_alert).to be(true) + end + end + + describe "#active?" do + context "when the state_name is 'Active'" do + let(:state_name) { "Active" } + + it "returns true" do + expect(subject.active?).to be(true) + end + end + + context "when the state_name is not 'Active'" do + let(:state_name) { "Inactive" } + + it "returns false" do + expect(subject.active?).to be(false) + end + end + end +end diff --git a/spec/services/dqt/v1/teacher_spec.rb b/spec/services/dqt/v1/teacher_spec.rb index 8ba1caf550..a99061e24a 100644 --- a/spec/services/dqt/v1/teacher_spec.rb +++ b/spec/services/dqt/v1/teacher_spec.rb @@ -12,7 +12,11 @@ let(:response_hash) do { "trn": trn, + "ni_number": "AB123456D", + "name": "Mostly Populated", + "dob": "1987-12-13", "active_alert": active_alert, + "state_name": "Active", } end @@ -55,21 +59,26 @@ .to_return(status: 200, body: response_hash.to_json, headers: {}) end - describe ".validate_trn" do + describe ".find" do it "returns teacher record" do stub_api_request - record = subject.validate_trn(trn:, birthdate:) + record = subject.find(trn:, birthdate:) expect(record["trn"]).to eq(trn) expect(record["active_alert"]).to be(true) + + expect(record["ni_number"]).to eq("AB123456D") + expect(record["name"]).to eq("Mostly Populated") + expect(record["dob"]).to eq("1987-12-13") + expect(record["state_name"]).to eq("Active") end context "when record does not exist" do it "returns nil" do stub_api_404_request - record = subject.validate_trn(trn:, birthdate:) + record = subject.find(trn:, birthdate:) expect(record).to be_nil end @@ -81,7 +90,7 @@ it "returns correct record" do stub_api_different_record_request - record = subject.validate_trn(trn: incorrect_trn, birthdate:, nino:) + record = subject.find(trn: incorrect_trn, birthdate:, nino:) expect(record["trn"]).to eql(trn) expect(record["active_alert"]).to be(false) diff --git a/spec/services/name_matcher_spec.rb b/spec/services/name_matcher_spec.rb new file mode 100644 index 0000000000..271de02219 --- /dev/null +++ b/spec/services/name_matcher_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe NameMatcher do + names = { + ["Miss Allison Taylor", "Ms Allison Page"] => [true, false], + ["Miss Allison Taylor", "Allison Taylor"] => [true, true], + ["Mr Allison Taylor", "Mr Allison Page"] => [true, false], + ["Mr Allison Taylor", "Allison Taylor"] => [true, true], + ["rev Allison Taylor", "Prof. Allison Page"] => [true, false], + ["Dr. Allison Taylor", "Allison Taylor"] => [true, true], + ["Allison Taylor", "Prof. Allison Taylor"] => [true, true], + ["Dr. Nicola Taylor", "Allison Taylor"] => [false, false], + } + + names.each do |input, output| + context "when #{input.first}, #{input.last}" do + let(:name_1) { input.first } + let(:name_2) { input.last } + + subject { described_class.new(name_1, name_2, check_first_name_only:).matches? } + + context "when first name check" do + let(:check_first_name_only) { true } + + it "returns #{output.first}" do + expect(subject).to be(output.first) + end + end + + context "when full name check" do + let(:check_first_name_only) { false } + + it "returns #{output.last}" do + expect(subject).to be(output.last) + end + end + end + end +end diff --git a/spec/services/teacher_reference_number_spec.rb b/spec/services/teacher_reference_number_spec.rb new file mode 100644 index 0000000000..7f8c197332 --- /dev/null +++ b/spec/services/teacher_reference_number_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe TeacherReferenceNumber do + describe "valid?" do + context "when the original value contains the correct number of digits" do + it "returns true" do + expect(described_class.new("RP99/12345")).to be_valid + expect(described_class.new("12345")).to be_valid + expect(described_class.new("1234567")).to be_valid + end + end + + context "when the original value is blank" do + subject(:trn) { described_class.new("") } + + it "returns false" do + expect(trn).not_to be_valid + end + + it "sets the format_error to :blank" do + trn.valid? + expect(trn.format_error).to eq(:blank) + end + end + + context "when the original value contains fewer than 5 digits" do + subject(:trn) { described_class.new("R12WWS/ 2") } + + it "returns false" do + expect(trn).not_to be_valid + end + + it "sets the format_error to :too_short" do + trn.valid? + expect(trn.format_error).to eq(:too_short) + end + end + + context "when the original value contains more than 7 digits" do + subject(:trn) { described_class.new("RP11/12345678") } + + it "returns false" do + expect(trn).not_to be_valid + end + + it "sets the format_error to :too_long" do + trn.valid? + expect(trn.format_error).to eq(:too_long) + end + end + end + + describe "#formatted_trn" do + it "removes non-numeric characters" do + expect(described_class.new("RP22/ 21 1 33").formatted_trn).to eq("2221133") + end + + it "zero pads the value to 7-digits" do + expect(described_class.new("RP99/123").formatted_trn).to eq("0099123") + end + + context "when the original value is not valid" do + it "returns nil" do + expect(described_class.new("QWERTY123").formatted_trn).to be_nil + end + end + end +end