From 14b8cabbfcf9215983310cb90ced5650d2d9ab07 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:37:48 +0100 Subject: [PATCH] CLDC-2673 Display UPRN in duplicate logs fields when it's given (#1990) * Display UPRN in duplicate logs fields when it's given * Display fields relevant to specific logs * Display postcode data for uprn for duplicates --- app/controllers/duplicate_logs_controller.rb | 23 -------- app/helpers/duplicate_logs_helper.rb | 44 +++++++++++++++ app/models/lettings_log.rb | 16 ++++++ app/models/log.rb | 7 +++ app/models/sales_log.rb | 11 ++++ .../_duplicate_log_check_answers.erb | 14 ++--- .../duplicate_logs_controller_spec.rb | 55 +++++++++++++++++++ 7 files changed, 139 insertions(+), 31 deletions(-) diff --git a/app/controllers/duplicate_logs_controller.rb b/app/controllers/duplicate_logs_controller.rb index bc0d628ed..d5ba94130 100644 --- a/app/controllers/duplicate_logs_controller.rb +++ b/app/controllers/duplicate_logs_controller.rb @@ -11,10 +11,6 @@ class DuplicateLogsController < ApplicationController def show if @log @all_duplicates = [@log, *@duplicate_logs] - @duplicate_check_questions = duplicate_check_question_ids.map { |question_id| - question = @log.form.get_question(question_id, @log) - question if question.page.routed_to?(@log, current_user) - }.compact else render_not_found end @@ -59,25 +55,6 @@ private @duplicates = duplicates_for_organisation(@organisation) end - def duplicate_check_question_ids - if @log.lettings? - ["owning_organisation_id", - "startdate", - "tenancycode", - "postcode_full", - "scheme_id", - "location_id", - "age1", - "sex1", - "ecstat1", - @log.household_charge == 1 ? "household_charge" : nil, - "tcharge", - @log.is_carehome? ? "chcharge" : nil].compact - else - %w[owning_organisation_id saledate purchid age1 sex1 ecstat1 postcode_full] - end - end - def find_original_log query_params = URI.parse(request.url).query original_log_id = CGI.parse(query_params)["original_log_id"][0]&.to_i if query_params.present? diff --git a/app/helpers/duplicate_logs_helper.rb b/app/helpers/duplicate_logs_helper.rb index 29c95e003..ec8ac9185 100644 --- a/app/helpers/duplicate_logs_helper.rb +++ b/app/helpers/duplicate_logs_helper.rb @@ -53,4 +53,48 @@ module DuplicateLogsHelper def duplicate_list_header(duplicate_sets_count) duplicate_sets_count > 1 ? "Review these #{duplicate_sets_count} sets of logs" : "Review this #{duplicate_sets_count} set of logs" end + + def duplicate_log_question_label(question) + if question.id == "uprn" + "Postcode (from UPRN)" + else + get_question_label(question) + end + end + + def duplicate_log_answer_label(question, log) + if question.id == "uprn" + postcode_question = log.form.get_question("postcode_full", log) + get_answer_label(postcode_question, log) + else + get_answer_label(question, log) + end + end + + def duplicate_log_extra_value(question, log) + if question.id == "uprn" + postcode_question = log.form.get_question("postcode_full", log) + postcode_question.get_extra_check_answer_value(log) + else + question.get_extra_check_answer_value(log) + end + end + + def duplicate_log_answer_label_present(question, log, current_user) + if question.id == "uprn" + postcode_question = log.form.get_question("postcode_full", log) + postcode_question.answer_label(log, current_user).present? + else + question.answer_label(log, current_user).present? + end + end + + def duplicate_log_inferred_answers(question, log) + if question.id == "uprn" + postcode_question = log.form.get_question("postcode_full", log) + postcode_question.get_inferred_answers(log) + else + question.get_inferred_answers(log) + end + end end diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 1bcca5496..51dc6ff4d 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -601,6 +601,22 @@ class LettingsLog < Log public_send("details_known_#{person_index}") == 1 end + def duplicate_check_question_ids + ["owning_organisation_id", + "startdate", + "tenancycode", + form.start_date.year < 2023 || uprn.blank? ? "postcode_full" : nil, + form.start_date.year >= 2023 && uprn.present? ? "uprn" : nil, + "scheme_id", + "location_id", + "age1", + "sex1", + "ecstat1", + household_charge == 1 ? "household_charge" : nil, + "tcharge", + is_carehome? ? "chcharge" : nil].compact + end + private def reset_invalid_unresolved_log_fields! diff --git a/app/models/log.rb b/app/models/log.rb index 5ba887c7a..7870076da 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -193,6 +193,13 @@ class Log < ApplicationRecord form.edit_end_date < Time.zone.now || older_than_previous_collection_year? end + def duplicate_check_questions(current_user) + duplicate_check_question_ids.map { |question_id| + question = form.get_question(question_id, self) + question if question.page.routed_to?(self, current_user) + }.compact + end + private # Handle logs that are older than previous collection start date diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index f144336bf..905b7d5a1 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -436,4 +436,15 @@ class SalesLog < Log self.pcodenk = nil if errors.attribute_names.include? :postcode_full end + + def duplicate_check_question_ids + ["owning_organisation_id", + "saledate", + "purchid", + "age1", + "sex1", + "ecstat1", + form.start_date.year < 2023 || uprn.blank? ? "postcode_full" : nil, + form.start_date.year >= 2023 && uprn.present? ? "uprn" : nil].compact + end end diff --git a/app/views/duplicate_logs/_duplicate_log_check_answers.erb b/app/views/duplicate_logs/_duplicate_log_check_answers.erb index ee350545b..44b788959 100644 --- a/app/views/duplicate_logs/_duplicate_log_check_answers.erb +++ b/app/views/duplicate_logs/_duplicate_log_check_answers.erb @@ -1,28 +1,26 @@
<%= govuk_summary_list do |summary_list| %> - <% @duplicate_check_questions.each do |question| %> + <% log.duplicate_check_questions(current_user).each do |question| %> <% summary_list.row do |row| %> - <% row.key { get_question_label(question) } %> + <% row.key { duplicate_log_question_label(question) } %> <% row.value do %> <%= simple_format( - get_answer_label(question, log), + duplicate_log_answer_label(question, log), wrapper_tag: "span", class: "govuk-!-margin-right-4", ) %> - <% extra_value = question.get_extra_check_answer_value(log) %> - - <% if extra_value && question.answer_label(log, current_user).present? %> + <% if duplicate_log_extra_value(question, log) && duplicate_log_answer_label_present(question, log, current_user) %> <%= simple_format( - extra_value, + duplicate_log_extra_value(question, log), wrapper_tag: "span", class: "govuk-!-font-weight-regular app-!-colour-muted", ) %> <% end %> - <% question.get_inferred_answers(log).each do |inferred_answer| %> + <% duplicate_log_inferred_answers(question, log).each do |inferred_answer| %> <%= inferred_answer %> <% end %> <% end %> diff --git a/spec/requests/duplicate_logs_controller_spec.rb b/spec/requests/duplicate_logs_controller_spec.rb index a3ed916a5..933a0f54d 100644 --- a/spec/requests/duplicate_logs_controller_spec.rb +++ b/spec/requests/duplicate_logs_controller_spec.rb @@ -29,6 +29,24 @@ RSpec.describe DuplicateLogsController, type: :request do end context "when user is signed in" do + before do + body = { + results: [ + { + DPA: { + "POSTCODE": "LS16 6FT", + "POST_TOWN": "Westminster", + "PO_BOX_NUMBER": "321", + "DOUBLE_DEPENDENT_LOCALITY": "Double Dependent Locality", + }, + }, + ], + }.to_json + + stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=123") + .to_return(status: 200, body:, headers: {}) + end + context "when user is support" do let(:support_user_org) { create(:organisation) } let(:user) { create(:user, :support, organisation: support_user_org) } @@ -67,6 +85,25 @@ RSpec.describe DuplicateLogsController, type: :request do expect(page).to have_link("Change", href: "/lettings-logs/#{duplicate_logs[1].id}/tenant-code?first_remaining_duplicate_id=#{lettings_log.id}&organisation_id=#{lettings_log.owning_organisation_id}&original_log_id=#{lettings_log.id}&referrer=duplicate_logs") end + it "displays check your answers for each log with correct questions where UPRN is given" do + lettings_log.update!(uprn: "123", uprn_known: 1, uprn_confirmed: 1) + duplicate_logs[0].update!(uprn: "123", uprn_known: 1, uprn_confirmed: 1) + get "/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}" + + expect(page).to have_content("Q5 - Tenancy start date", count: 3) + expect(page).to have_content("Q7 - Tenant code", count: 3) + expect(page).to have_content("Q12 - Postcode", count: 1) + expect(page).to have_content("Postcode (from UPRN)", count: 2) + expect(page).to have_content("Q32 - Lead tenant’s age", count: 3) + expect(page).to have_content("Q33 - Lead tenant’s gender identity", count: 3) + expect(page).to have_content("Q37 - Lead tenant’s working situation", count: 3) + expect(page).to have_content("Household rent and charges", count: 3) + expect(page).to have_link("Change", count: 24) + expect(page).to have_link("Change", href: "/lettings-logs/#{lettings_log.id}/tenant-code?first_remaining_duplicate_id=#{duplicate_logs[0].id}&original_log_id=#{lettings_log.id}&referrer=duplicate_logs") + expect(page).to have_link("Change", href: "/lettings-logs/#{duplicate_logs[0].id}/tenant-code?first_remaining_duplicate_id=#{lettings_log.id}&original_log_id=#{lettings_log.id}&referrer=duplicate_logs") + expect(page).to have_link("Change", href: "/lettings-logs/#{duplicate_logs[1].id}/tenant-code?first_remaining_duplicate_id=#{lettings_log.id}&original_log_id=#{lettings_log.id}&referrer=duplicate_logs") + end + it "displays buttons to delete" do expect(page).to have_link("Keep this log and delete duplicates", count: 3) expect(page).to have_link("Keep this log and delete duplicates", href: "/lettings-logs/#{lettings_log.id}/delete-duplicates?organisation_id=#{lettings_log.owning_organisation_id}&original_log_id=#{lettings_log.id}") @@ -160,6 +197,24 @@ RSpec.describe DuplicateLogsController, type: :request do expect(page).to have_link("Change", href: "/sales-logs/#{duplicate_logs[1].id}/purchaser-code?first_remaining_duplicate_id=#{sales_log.id}&organisation_id=#{sales_log.owning_organisation_id}&original_log_id=#{sales_log.id}&referrer=duplicate_logs") end + it "displays check your answers for each log with correct questions when UPRN is given" do + sales_log.update!(uprn: "123", uprn_known: 1) + duplicate_logs[0].update!(uprn: "123", uprn_known: 1) + duplicate_logs[1].update!(uprn: "123", uprn_known: 1) + get "/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}" + + expect(page).to have_content("Q1 - Sale completion date", count: 3) + expect(page).to have_content("Q2 - Purchaser code", count: 3) + expect(page).to have_content("Q20 - Lead buyer’s age", count: 3) + expect(page).to have_content("Q21 - Buyer 1’s gender identity", count: 3) + expect(page).to have_content("Q25 - Buyer 1's working situation", count: 3) + expect(page).to have_content("Postcode (from UPRN)", count: 3) + expect(page).to have_link("Change", count: 21) + expect(page).to have_link("Change", href: "/sales-logs/#{sales_log.id}/purchaser-code?first_remaining_duplicate_id=#{duplicate_logs[0].id}&original_log_id=#{sales_log.id}&referrer=duplicate_logs") + expect(page).to have_link("Change", href: "/sales-logs/#{duplicate_logs[0].id}/purchaser-code?first_remaining_duplicate_id=#{sales_log.id}&original_log_id=#{sales_log.id}&referrer=duplicate_logs") + expect(page).to have_link("Change", href: "/sales-logs/#{duplicate_logs[1].id}/purchaser-code?first_remaining_duplicate_id=#{sales_log.id}&original_log_id=#{sales_log.id}&referrer=duplicate_logs") + end + it "displays buttons to delete" do expect(page).to have_link("Keep this log and delete duplicates", count: 3) expect(page).to have_link("Keep this log and delete duplicates", href: "/sales-logs/#{sales_log.id}/delete-duplicates?organisation_id=#{sales_log.owning_organisation_id}&original_log_id=#{sales_log.id}")