From 873934d92f75df1915db73ee710de87df4ace38a Mon Sep 17 00:00:00 2001 From: MazOneTwoOne <76905544+MazOneTwoOne@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:17:54 +0000 Subject: [PATCH] EL-1757: Saving Early Eligibility result to `completed_user_journey` table (#1593) * - add in the values to be populated by `journey_logger_service` - add a method to `check` so we know the type of early result - prevent calling the `JourneyLoggerService` if user skips to the results page, we wna to call this earlier to prevent double counting - call `JourneyLoggerService` when user see banner. Also set "type" of early result, when they see the banner - remove when user see `set_early_result_type` from `ResultsController` as this is now set upon seeing the banner. - tidy up naming and separate out methods - test that `JourneyLoggerService` puts correct things into table based on session_data - test that a `track_completed_journey` method in my ResultsController is called based on session_data - add suffix to assessment_id for an early result - add tests for code coverage * - simplify update journey logger service * - add test coverage for new `JourneyLoggerService` refactor completeduserjourney call add a test to check new record is created --------- Co-authored-by: Will Clarke --- app/controllers/forms_controller.rb | 14 ++- app/controllers/results_controller.rb | 18 ++-- app/models/check.rb | 4 + app/models/completed_user_journey.rb | 49 ++++----- app/services/journey_logger_service.rb | 8 +- ...ssessment_id_in_completed_user_journeys.rb | 6 ++ db/schema.rb | 4 +- spec/controllers/results_controller_spec.rb | 46 ++++++++ spec/features/provider_login_spec.rb | 48 +++++++++ spec/services/journey_logger_service_spec.rb | 100 ++++++++++++++++++ 10 files changed, 256 insertions(+), 41 deletions(-) create mode 100644 db/migrate/20241105100606_change_assessment_id_in_completed_user_journeys.rb create mode 100644 spec/controllers/results_controller_spec.rb diff --git a/app/controllers/forms_controller.rb b/app/controllers/forms_controller.rb index a30b8628c..8873f30f2 100644 --- a/app/controllers/forms_controller.rb +++ b/app/controllers/forms_controller.rb @@ -41,8 +41,18 @@ def calculate_early_result if last_tag_in_group?(:gross_income) cfe_result = CfeService.result(session_data, Steps::Helper.completed_steps_for(session_data, step)) session_data["early_result"] = { "result" => cfe_result.gross_income_result, - "gross_income_excess" => cfe_result.gross_income_excess } - # add analytics tracking here? completed user journeys + "gross_income_excess" => cfe_result.gross_income_excess, + "type" => "gross_income" } + if @check.early_ineligible_result? + track_completed_journey_for_early_result + end end end + + def track_completed_journey_for_early_result + session_data["api_response"] = CfeService.call(session_data, Steps::Helper.completed_steps_for(session_data, step)) + calculation_result = CalculationResult.new(session_data) + office_code = signed_in? && current_provider.present? ? current_provider.first_office_code : nil + JourneyLoggerService.call(assessment_id, calculation_result, @check, office_code, cookies) + end end diff --git a/app/controllers/results_controller.rb b/app/controllers/results_controller.rb index 12f0bb7d0..da443ebc2 100644 --- a/app/controllers/results_controller.rb +++ b/app/controllers/results_controller.rb @@ -9,7 +9,6 @@ def create def early_result_redirect @previous_step = params[:step].to_sym session_data["api_response"] = CfeService.call(session_data, Steps::Helper.completed_steps_for(session_data, @previous_step)) - set_early_result_type redirect_to result_path(assessment_code:) end @@ -18,8 +17,9 @@ def show @early_result_type = session_data.dig("early_result", "type") @early_eligibility_selection = session_data.fetch("early_eligibility_selection", nil) @model = CalculationResult.new(session_data) - # we'll need to move this tracking point or do something with it - track_completed_journey(@model) + + track_completed_journey(@model) unless @check.early_ineligible_result? + track_page_view(page: :view_results) @journey_continues_on_another_page = @check.controlled? && @model.decision == "eligible" @feedback = @journey_continues_on_another_page ? :freetext : :satisfaction @@ -55,15 +55,9 @@ def load_check @check = Check.new(session_data) end - def set_early_result_type - session_data["early_result"]["type"] = "gross_income" - end - def track_completed_journey(calculation_result) - if signed_in? && current_provider.present? - JourneyLoggerService.call(assessment_id, calculation_result, @check, current_provider.first_office_code, cookies) - else - JourneyLoggerService.call(assessment_id, calculation_result, @check, nil, cookies) - end + office_code = signed_in? && current_provider.present? ? current_provider.first_office_code : nil + + JourneyLoggerService.call(assessment_id, calculation_result, @check, office_code, cookies) end end diff --git a/app/models/check.rb b/app/models/check.rb index 111c074a0..46629bd65 100644 --- a/app/models/check.rb +++ b/app/models/check.rb @@ -199,4 +199,8 @@ def early_ineligible_result? def gross_income_excess session_data.dig("early_result", "gross_income_excess") end + + def early_result_type + session_data.dig("early_result", "type") + end end diff --git a/app/models/completed_user_journey.rb b/app/models/completed_user_journey.rb index 790803aac..5644817d4 100644 --- a/app/models/completed_user_journey.rb +++ b/app/models/completed_user_journey.rb @@ -1,30 +1,31 @@ class CompletedUserJourney < ApplicationRecord end -#------------------------------------------------------------------------------ +#---------------------------------------------------------------------------------- # CompletedUserJourney # -# Name SQL Type Null Primary Default -# -------------------- -------------------- ------- ------- ---------- -# id bigint false true -# assessment_id character varying false false -# certificated boolean false false -# partner boolean false false -# person_over_60 boolean false false -# passported boolean false false -# main_dwelling_owned boolean true false -# vehicle_owned boolean true false -# smod_assets boolean true false -# outcome character varying false false -# capital_contribution boolean true false -# income_contribution boolean false false -# completed date true false -# form_downloaded boolean true false false -# matter_type character varying true false -# asylum_support boolean true false -# client_age character varying true false -# session jsonb true false -# office_code character varying true false -# early_result_type character varying true false +# Name SQL Type Null Primary Default +# -------------------- -------------------- ------- ------- ---------- +# id bigint false true +# assessment_id character varying false false +# certificated boolean false false +# partner boolean false false +# person_over_60 boolean false false +# passported boolean false false +# main_dwelling_owned boolean true false +# vehicle_owned boolean true false +# smod_assets boolean true false +# outcome character varying false false +# capital_contribution boolean true false +# income_contribution boolean false false +# completed date true false +# form_downloaded boolean true false false +# matter_type character varying true false +# asylum_support boolean true false +# client_age character varying true false +# session jsonb true false +# office_code character varying true false +# early_result_type character varying true false +# early_eligibility_result boolean false false # -#------------------------------------------------------------------------------ +#---------------------------------------------------------------------------------- diff --git a/app/services/journey_logger_service.rb b/app/services/journey_logger_service.rb index cca79d0c5..7b5c93927 100644 --- a/app/services/journey_logger_service.rb +++ b/app/services/journey_logger_service.rb @@ -4,8 +4,12 @@ def call(assessment_id, calculation_result, check, portal_user_office_code, cook return if cookies[CookiesController::NO_ANALYTICS_MODE] attributes = build_attributes(calculation_result, check, portal_user_office_code) + early_ineligible_result = FeatureFlags.enabled?(:ee_banner, check.session_data) ? check.early_ineligible_result? : nil + CompletedUserJourney.transaction do - if (journey = CompletedUserJourney.find_by(assessment_id:)) + journey = CompletedUserJourney.find_by(assessment_id:, early_eligibility_result: early_ineligible_result) + + if journey journey.update!(attributes) else CompletedUserJourney.create!(attributes.merge(assessment_id:)) @@ -35,6 +39,8 @@ def build_attributes(calculation_result, check, portal_user_office_code) matter_type: matter_type(check), session: check.session_data, office_code: portal_user_office_code, + early_result_type: check.early_result_type, + early_eligibility_result: check.early_ineligible_result?, } end diff --git a/db/migrate/20241105100606_change_assessment_id_in_completed_user_journeys.rb b/db/migrate/20241105100606_change_assessment_id_in_completed_user_journeys.rb new file mode 100644 index 000000000..14e8410f9 --- /dev/null +++ b/db/migrate/20241105100606_change_assessment_id_in_completed_user_journeys.rb @@ -0,0 +1,6 @@ +class ChangeAssessmentIdInCompletedUserJourneys < ActiveRecord::Migration[7.2] + def change + remove_index :completed_user_journeys, column: :assessment_id, unique: true + add_index :completed_user_journeys, :assessment_id, unique: false + end +end diff --git a/db/schema.rb b/db/schema.rb index e5226a5b1..d6e04e646 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_11_01_120059) do +ActiveRecord::Schema[7.2].define(version: 2024_11_05_100606) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -108,7 +108,7 @@ t.string "office_code" t.string "early_result_type" t.boolean "early_eligibility_result" - t.index ["assessment_id"], name: "index_completed_user_journeys_on_assessment_id", unique: true + t.index ["assessment_id"], name: "index_completed_user_journeys_on_assessment_id" end create_table "feature_flag_overrides", force: :cascade do |t| diff --git a/spec/controllers/results_controller_spec.rb b/spec/controllers/results_controller_spec.rb new file mode 100644 index 000000000..46723a2c7 --- /dev/null +++ b/spec/controllers/results_controller_spec.rb @@ -0,0 +1,46 @@ +require "rails_helper" + +RSpec.describe ResultsController, type: :controller do + include Devise::Test::ControllerHelpers + + let(:email_address) { "test@testing.com" } + let(:assessment_code) { "123456" } + let(:session_data) { { "some" => "data", "assessment_code" => assessment_code } } + let(:calculation_result) { instance_double(CalculationResult) } + let(:check) { instance_double(Check, controlled?: false, early_ineligible_result?: false) } + + before do + OmniAuth.config.mock_auth[:saml] = build(:mock_saml_auth) + provider = create(:provider, email: email_address, first_office_code: "1Q630KL") + sign_in provider + + allow(controller).to receive(:session_data).and_return(session_data) + allow(CalculationResult).to receive(:new).and_return(calculation_result) + allow(Check).to receive(:new).and_return(check) + allow(controller).to receive(:track_page_view) + end + + describe "GET #show" do + context "when @check.early_ineligible_result? is true", :ee_banner do + before do + allow(check).to receive(:early_ineligible_result?).and_return(true) + end + + it "does not call JourneyLoggerService" do + expect(JourneyLoggerService).not_to receive(:call) + get :show, params: { assessment_code: } + end + end + + context "when @check.early_ineligible_result? is false", :ee_banner do + before do + allow(check).to receive(:early_ineligible_result?).and_return(false) + end + + it "calls JourneyLoggerService" do + expect(JourneyLoggerService).to receive(:call).with(any_args) + get :show, params: { assessment_code: } + end + end + end +end diff --git a/spec/features/provider_login_spec.rb b/spec/features/provider_login_spec.rb index 52846a1b6..65ba7982d 100644 --- a/spec/features/provider_login_spec.rb +++ b/spec/features/provider_login_spec.rb @@ -42,4 +42,52 @@ expect(page).to have_content("Your client is likely to qualify for civil legal aid") end end + + context "when signed in I can complete an early eligibility check", :ee_banner do + let(:email_address) { Faker::Internet.email } + + before do + sign_in create(:provider, email: email_address, first_office_code: "1Q630KL") + visit "/" + stub_request(:post, %r{v6/assessments\z}).to_return( + body: build(:api_result, eligible: "eligible").to_json, + headers: { "Content-Type" => "application/json" }, + ) + first = instance_double(CfeResult, ineligible_gross_income?: true, + gross_income_excess: 100, + gross_income_result: "ineligible") + second = instance_double(CfeResult, ineligible_gross_income?: false, + gross_income_excess: 0, + gross_income_result: "eligible") + allow(CfeService).to receive(:result).and_return(first, second) + allow(CfeService).to receive(:call).and_return build(:api_result, eligible: "ineligible") + start_assessment + fill_in_forms_until(:applicant) + fill_in_applicant_screen(partner: "No", passporting: "No") + fill_in_dependant_details_screen + fill_in_employment_status_screen(choice: "Employed or self-employed") + fill_in_income_screen(gross: "8000", frequency: "Every month") + fill_in_forms_until(:other_income) + fill_in_other_income_screen_with_friends_and_family + end + + it "when I continue the check" do + confirm_screen("outgoings") + expect(page).to have_content("Gross monthly income limit exceeded") + fill_in_outgoings_screen + expect(page).not_to have_content("Gross monthly income limit exceeded") + fill_in_forms_until(:check_answers) + click_on "Submit" + expect(page).to have_current_path(/\A\/check-result/) + expect(page).to have_content "Your client's key eligibility totals" + end + + it "when I go straight to results the check" do + confirm_screen("outgoings") + expect(page).to have_content("Gross monthly income limit exceeded") + click_on "Go to results page" + expect(page).to have_current_path(/\A\/check-result/) + expect(page).to have_content "Your client's key eligibility totals" + end + end end diff --git a/spec/services/journey_logger_service_spec.rb b/spec/services/journey_logger_service_spec.rb index 12b22479d..e0166b26c 100644 --- a/spec/services/journey_logger_service_spec.rb +++ b/spec/services/journey_logger_service_spec.rb @@ -32,6 +32,8 @@ expect(output.matter_type).to eq "asylum" expect(output.session.with_indifferent_access).to eq session_data expect(output.office_code).to eq "office-code" + expect(output.early_result_type).to be_nil + expect(output.early_eligibility_result).to be false end it "skips saving in no-analytics mode" do @@ -76,6 +78,8 @@ expect(output.outcome).to eq "contribution_required" expect(output.capital_contribution).to be true expect(output.income_contribution).to be true + expect(output.early_result_type).to be_nil + expect(output.early_eligibility_result).to be false end end @@ -179,5 +183,101 @@ expect(output.client_age).to eq "over_60" end end + + context "when it is an early ineligible result for gross income", :ee_banner do + let(:session_data) do + { + immigration_or_asylum: false, + partner: false, + passporting: false, + early_result: { "result" => "ineligible", + "type" => "gross_income" }, + }.with_indifferent_access + end + + it "saves `early_result_type` and `outcome` to completed user journey" do + described_class.call(assessment_id, calculation_result, check, portal_user_office_code, {}) + output = CompletedUserJourney.find_by(assessment_id:) + expect(output.outcome).to eq "ineligible" + expect(output.early_result_type).to eq "gross_income" + expect(output.early_eligibility_result).to be true + end + end + + context "when the journey continues after early ineligible result", :ee_banner do + let(:session_data) do + { + early_result: { "result" => "ineligible", + "type" => "gross_income" }, + }.with_indifferent_access + end + + it "creates a new record at the end of the journey" do + # Initial call with ineligible early eligibility result from session_data + described_class.call(assessment_id, calculation_result, check, portal_user_office_code, {}) + expect(CompletedUserJourney.where(assessment_id:).count).to eq 1 + # Simulate continuing to the end of the journey with an overall_result + updated_api_result = FactoryBot.build(:api_result, overall_result: { result: "ineligible" }) + allow(calculation_result).to receive(:api_response).and_return(updated_api_result) + # Update `check` to alter `early_result_type` as a visit to the results page would + allow(check).to receive(:early_ineligible_result?).and_return(nil) + described_class.call(assessment_id, calculation_result, check, portal_user_office_code, {}) + expect(CompletedUserJourney.where(assessment_id:).count).to eq 2 + end + end + + context "when CompletedUserJourney is called with the same assessment_id and early_ineligible remains true", :ee_banner do + let(:session_data) do + { + immigration_or_asylum: false, + partner: false, + passporting: false, + early_result: { "result" => "ineligible", + "type" => "gross_income" }, + }.with_indifferent_access + end + + it "creates a new record initially, and updates it on subsequent calls" do + described_class.call(assessment_id, calculation_result, check, portal_user_office_code, session_data) + expect(CompletedUserJourney.where(assessment_id:).count).to eq 1 + + modified_session_data = { + immigration_or_asylum: true, + partner: true, + passporting: true, + early_result: { "result" => "ineligible", "type" => "gross_income" }, + }.with_indifferent_access + + described_class.call(assessment_id, calculation_result, check, portal_user_office_code, modified_session_data) + + # Ensure no new record was created; the count should still be 1 + expect(CompletedUserJourney.where(assessment_id:).count).to eq 1 + end + end + + context "when details change in a full journey", :ee_banner do + let(:session_data) { { partner: false } } + + it "updates an existing record" do + existing_record = FactoryBot.create(:completed_user_journey, assessment_id:, early_eligibility_result: false, partner: true) + described_class.call(assessment_id, calculation_result, check, portal_user_office_code, {}) + expect(existing_record.reload.partner).to be false + end + end + + context "when users skips to results page, after they see the early ineligible banner", :ee_banner do + let(:session_data) do + { + early_result: { "result" => "ineligible", + "type" => "gross_income" }, + } + end + + it "updates an existing record" do + existing_record = FactoryBot.create(:completed_user_journey, assessment_id:, early_eligibility_result: true) + described_class.call(assessment_id, calculation_result, check, portal_user_office_code, {}) + expect(existing_record.reload.early_eligibility_result).to be true + end + end end end