Browse Source

CLDC-2496 Correct duplicate data (#1793)

* Update check answers to use the correct logs

* Display different link when correcting duplicates

* allow deduplicating logs by changing the answers

* Route back to the initial log if it still has duplicates

* Update back to log button

* Add banner

* Update success banner

* Fix a flaky test

* Rename param and update conditionals

* Add owning organisation to deduplication fields, only return back to the original log if it still belongs to the org
pull/1830/head^2
kosiakkatrina 1 year ago committed by GitHub
parent
commit
8d085528c4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 11
      app/controllers/duplicate_logs_controller.rb
  2. 48
      app/controllers/form_controller.rb
  3. 13
      app/helpers/duplicate_logs_helper.rb
  4. 2
      app/models/lettings_log.rb
  5. 2
      app/models/sales_log.rb
  6. 21
      app/views/duplicate_logs/_duplicate_log_check_answers.erb
  7. 4
      app/views/duplicate_logs/show.html.erb
  8. 2
      app/views/logs/delete_duplicates.html.erb
  9. 2
      config/locales/en.yml
  10. 23
      spec/features/lettings_log_spec.rb
  11. 23
      spec/features/sales_log_spec.rb
  12. 31
      spec/models/lettings_log_spec.rb
  13. 29
      spec/models/sales_log_spec.rb
  14. 67
      spec/requests/duplicate_logs_controller_spec.rb
  15. 82
      spec/requests/form_controller_spec.rb
  16. 8
      spec/requests/sales_logs_controller_spec.rb

11
app/controllers/duplicate_logs_controller.rb

@ -2,7 +2,7 @@ class DuplicateLogsController < ApplicationController
before_action :authenticate_user!
before_action :find_resource_by_named_id
before_action :find_duplicate_logs
before_action :find_original_log_id
before_action :find_original_log
def show
if @log
@ -61,8 +61,13 @@ private
end
end
def find_original_log_id
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?
original_log_id = CGI.parse(query_params)["original_log_id"][0]&.to_i if query_params.present?
@original_log = if params[:sales_log_id].present?
current_user.sales_logs.find_by(id: original_log_id)
else
current_user.lettings_logs.find_by(id: original_log_id)
end
end
end

48
app/controllers/form_controller.rb

@ -48,7 +48,7 @@ class FormController < ApplicationController
def show_page
if request.params["referrer"] == "interruption_screen" && request.headers["HTTP_REFERER"].present?
@interruption_page_id = URI.parse(request.headers["HTTP_REFERER"]).path.split("/").last.underscore
@interruption_page_referrer_type = referrer_from_query
@interruption_page_referrer_type = from_referrer_query("referrer")
end
if @log
@ -130,10 +130,10 @@ private
end
def is_referrer_type?(referrer_type)
referrer_from_query == referrer_type
from_referrer_query("referrer") == referrer_type
end
def referrer_from_query
def from_referrer_query(query_param)
referrer = request.headers["HTTP_REFERER"]
return unless referrer
@ -141,7 +141,7 @@ private
return unless query_params
parsed_params = CGI.parse(query_params)
parsed_params["referrer"]&.first
parsed_params[query_param]&.first
end
def original_duplicate_log_id_from_query
@ -163,11 +163,15 @@ private
def successful_redirect_path
if FeatureToggle.deduplication_flow_enabled?
if @log.lettings?
if current_user.lettings_logs.duplicate_logs(@log).count.positive?
if is_referrer_type?("duplicate_logs")
return correcting_duplicate_logs_redirect_path
end
if @log.lettings? && current_user.lettings_logs.duplicate_logs(@log).count.positive?
return send("lettings_log_duplicate_logs_path", @log, original_log_id: @log.id)
end
elsif current_user.sales_logs.duplicate_logs(@log).count.positive?
if @log.sales? && current_user.sales_logs.duplicate_logs(@log).count.positive?
return send("sales_log_duplicate_logs_path", @log, original_log_id: @log.id)
end
end
@ -237,4 +241,34 @@ private
end
CONFIRMATION_PAGE_IDS = %w[uprn_confirmation].freeze
def correcting_duplicate_logs_redirect_path
class_name = @log.class.name.underscore
original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id"))
if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).count.positive?
flash[:notice] = deduplication_success_banner unless current_user.send(class_name.pluralize).duplicate_logs(@log).count.positive?
send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id)
else
flash[:notice] = deduplication_success_banner
send("#{class_name}_duplicate_logs_path", "#{class_name}_id".to_sym => from_referrer_query("first_remaining_duplicate_id"), original_log_id: from_referrer_query("original_log_id"))
end
end
def deduplication_success_banner
deduplicated_log_link = "<a class=\"govuk-notification-banner__link govuk-!-font-weight-bold\" href=\"#{send("#{@log.class.name.underscore}_path", @log)}\">Log #{@log.id}</a>"
changed_labels = {
property_postcode: "postcode",
lead_tenant_age: "lead tenant’s age",
rent_4_weekly: "household rent and charges",
rent_bi_weekly: "household rent and charges",
rent_monthly: "household rent and charges",
rent_or_other_charges: "household rent and charges",
address: "postcode",
}
changed_question_label = changed_labels[@page.id.to_sym] || (@page.questions.first.check_answer_label.to_s.presence || @page.questions.first.header.to_s).downcase
I18n.t("notification.duplicate_logs.deduplication_success_banner", log_link: deduplicated_log_link, changed_question_label:).html_safe
end
end

13
app/helpers/duplicate_logs_helper.rb

@ -1,18 +1,18 @@
module DuplicateLogsHelper
include GovukLinkHelper
def duplicate_logs_continue_button(all_duplicates, duplicate_log, original_log_id)
def duplicate_logs_continue_button(all_duplicates, duplicate_log, original_log)
if all_duplicates.count > 1
return govuk_button_link_to "Keep this log and delete duplicates", url_for(
controller: "duplicate_logs",
action: "delete_duplicates",
"#{duplicate_log.class.name.underscore}_id": duplicate_log.id,
original_log_id:,
original_log_id: original_log.id,
)
end
if original_log_id == duplicate_log.id
govuk_button_link_to "Back to Log #{duplicate_log.id}", send("#{duplicate_log.class.name.underscore}_path", duplicate_log)
if !original_log.deleted?
govuk_button_link_to "Back to Log #{original_log.id}", send("#{original_log.class.name.underscore}_path", original_log)
else
type = duplicate_log.lettings? ? "lettings" : "sales"
govuk_button_link_to "Back to #{type} logs", url_for(duplicate_log.class)
@ -22,4 +22,9 @@ module DuplicateLogsHelper
def duplicate_logs_action_href(log, page_id, original_log_id)
send("#{log.model_name.param_key}_#{page_id}_path", log, referrer: "interruption_screen", original_log_id:)
end
def change_duplicate_logs_action_href(log, page_id, all_duplicates, original_log_id)
first_remaining_duplicate_id = all_duplicates.map(&:id).reject { |id| id == log.id }.first
send("#{log.model_name.param_key}_#{page_id}_path", log, referrer: "duplicate_logs", first_remaining_duplicate_id:, original_log_id:)
end
end

2
app/models/lettings_log.rb

@ -79,7 +79,7 @@ class LettingsLog < Log
NUM_OF_WEEKS_FROM_PERIOD = { 2 => 26, 3 => 13, 4 => 12, 5 => 50, 6 => 49, 7 => 48, 8 => 47, 9 => 46, 1 => 52, 10 => 53 }.freeze
SUFFIX_FROM_PERIOD = { 2 => "every 2 weeks", 3 => "every 4 weeks", 4 => "every month" }.freeze
RETIREMENT_AGES = { "M" => 67, "F" => 60, "X" => 67 }.freeze
DUPLICATE_LOG_ATTRIBUTES = %w[tenancycode startdate age1_known age1 sex1 ecstat1 tcharge household_charge chcharge].freeze
DUPLICATE_LOG_ATTRIBUTES = %w[owning_organisation_id tenancycode startdate age1_known age1 sex1 ecstat1 tcharge household_charge chcharge].freeze
def form
FormHandler.instance.get_form(form_name) || FormHandler.instance.current_lettings_form

2
app/models/sales_log.rb

@ -55,7 +55,7 @@ class SalesLog < Log
OPTIONAL_FIELDS = %w[purchid othtype].freeze
RETIREMENT_AGES = { "M" => 65, "F" => 60, "X" => 65 }.freeze
DUPLICATE_LOG_ATTRIBUTES = %w[purchid saledate age1_known age1 sex1 ecstat1 postcode_full].freeze
DUPLICATE_LOG_ATTRIBUTES = %w[owning_organisation_id purchid saledate age1_known age1 sex1 ecstat1 postcode_full].freeze
def lettings?
false

21
app/views/duplicate_logs/_duplicate_log_check_answers.erb

@ -7,14 +7,14 @@
<% row.value do %>
<%= simple_format(
get_answer_label(question, @log),
get_answer_label(question, log),
wrapper_tag: "span",
class: "govuk-!-margin-right-4",
) %>
<% extra_value = question.get_extra_check_answer_value(@log) %>
<% extra_value = question.get_extra_check_answer_value(log) %>
<% if extra_value && question.answer_label(@log, current_user).present? %>
<% if extra_value && question.answer_label(log, current_user).present? %>
<%= simple_format(
extra_value,
wrapper_tag: "span",
@ -22,16 +22,23 @@
) %>
<% end %>
<% question.get_inferred_answers(@log).each do |inferred_answer| %>
<% question.get_inferred_answers(log).each do |inferred_answer| %>
<span class="govuk-!-font-weight-regular app-!-colour-muted"><%= inferred_answer %></span>
<% end %>
<% end %>
<% if @all_duplicates.many? %>
<% row.action(
text: question.action_text(@log),
href: duplicate_logs_action_href(@log, question.page.id, @original_log_id),
text: question.action_text(log),
href: change_duplicate_logs_action_href(log, question.page.id, @all_duplicates, @original_log.id),
visually_hidden_text: question.check_answer_label.to_s.downcase,
) %>
<% else %>
<% row.action(
text: question.action_text(log),
href: duplicate_logs_action_href(log, question.page.id, @original_log.id),
visually_hidden_text: question.check_answer_label.to_s.downcase,
) %>
<% end %>
<% end %>
<% end %>
<% end %>

4
app/views/duplicate_logs/show.html.erb

@ -1,7 +1,7 @@
<% content_for :title, "Check duplicate logs" %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<% if @all_duplicates.count > 1 %>
<% if @all_duplicates.many? %>
<%= govuk_panel(
classes: "app-panel--interruption",
) do %>
@ -18,7 +18,7 @@
<% @all_duplicates.each_with_index do |log, index| %>
<%= render partial: "duplicate_log", locals: { log: } %>
<%= render partial: "duplicate_log_check_answers", locals: { log: } %>
<%= duplicate_logs_continue_button(@all_duplicates, log, @original_log_id) %>
<%= duplicate_logs_continue_button(@all_duplicates, log, @original_log) %>
<% if index < @all_duplicates.count - 1 %>
<hr class="govuk-section-break govuk-section-break--visible govuk-section-break--m">
<% end %>

2
app/views/logs/delete_duplicates.html.erb

@ -29,7 +29,7 @@
<%= govuk_button_to @duplicate_logs.count == 1 ? "Delete this log" : "Delete these logs",
send("delete_logs_#{@log.class.name.underscore}s_path"),
method: "delete",
params: { ids: @duplicate_logs.map(&:id), original_log_id: @original_log_id, remaining_log_id: @log.id } %>
params: { ids: @duplicate_logs.map(&:id), original_log_id: @original_log.id, remaining_log_id: @log.id } %>
<%= govuk_button_link_to(
"Cancel",
send("#{@log.class.name.underscore}_duplicate_logs_path", @log),

2
config/locales/en.yml

@ -187,6 +187,8 @@ en:
duplicate_logs_deleted:
one: "%{log_ids} has been deleted."
other: "%{log_ids} have been deleted."
duplicate_logs:
deduplication_success_banner: "%{log_link} is no longer a duplicate and has been removed from the list.<p class=\"govuk-body govuk-!-margin-top-4\">You changed the %{changed_question_label}.</p>"
validations:
organisation:

23
spec/features/lettings_log_spec.rb

@ -505,6 +505,29 @@ RSpec.describe "Lettings Log Features" do
expect(page).to have_current_path("/lettings-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
expect(page).to have_link("Back to lettings logs", href: "/lettings-logs")
end
it "allows deduplicating logs by changing the answers on the duplicate log" do
expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
click_link("Change", href: "/lettings-logs/#{duplicate_log.id}/tenant-code?first_remaining_duplicate_id=#{lettings_log.id}&original_log_id=#{lettings_log.id}&referrer=duplicate_logs")
fill_in("lettings-log-tenancycode-field", with: "something else")
click_button("Save and continue")
expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
expect(page).to have_link("Back to Log #{lettings_log.id}", href: "/lettings-logs/#{lettings_log.id}")
expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
expect(page).to have_content("Log #{duplicate_log.id} is no longer a duplicate and has been removed from the list")
expect(page).to have_content("You changed the tenant code.")
end
it "allows deduplicating logs by changing the answers on the original log" do
click_link("Change", href: "/lettings-logs/#{lettings_log.id}/tenant-code?first_remaining_duplicate_id=#{duplicate_log.id}&original_log_id=#{lettings_log.id}&referrer=duplicate_logs")
fill_in("lettings-log-tenancycode-field", with: "something else")
click_button("Save and continue")
expect(page).to have_current_path("/lettings-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
expect(page).to have_link("Back to Log #{lettings_log.id}", href: "/lettings-logs/#{lettings_log.id}")
expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
expect(page).to have_content("Log #{lettings_log.id} is no longer a duplicate and has been removed from the list")
expect(page).to have_content("You changed the tenant code.")
end
end
end
end

23
spec/features/sales_log_spec.rb

@ -244,5 +244,28 @@ RSpec.describe "Sales Log Features" do
expect(page).to have_current_path("/sales-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
expect(page).to have_link("Back to sales logs", href: "/sales-logs")
end
it "allows deduplicating logs by changing the answers on the duplicate log" do
expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
click_link("Change", href: "/sales-logs/#{duplicate_log.id}/purchaser-code?first_remaining_duplicate_id=#{sales_log.id}&original_log_id=#{sales_log.id}&referrer=duplicate_logs")
fill_in("sales-log-purchid-field", with: "something else")
click_button("Save and continue")
expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
expect(page).to have_link("Back to Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}")
expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
expect(page).to have_content("Log #{duplicate_log.id} is no longer a duplicate and has been removed from the list")
expect(page).to have_content("You changed the purchaser code.")
end
it "allows deduplicating logs by changing the answers on the original log" do
click_link("Change", href: "/sales-logs/#{sales_log.id}/purchaser-code?first_remaining_duplicate_id=#{duplicate_log.id}&original_log_id=#{sales_log.id}&referrer=duplicate_logs")
fill_in("sales-log-purchid-field", with: "something else")
click_button("Save and continue")
expect(page).to have_current_path("/sales-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
expect(page).to have_link("Back to Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}")
expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
expect(page).to have_content("Log #{sales_log.id} is no longer a duplicate and has been removed from the list")
expect(page).to have_content("You changed the purchaser code.")
end
end
end

31
spec/models/lettings_log_spec.rb

@ -2836,8 +2836,9 @@ RSpec.describe LettingsLog do
end
context "when filtering duplicate logs" do
let(:log) { create(:lettings_log, :duplicate) }
let!(:duplicate_log) { create(:lettings_log, :duplicate) }
let(:organisation) { create(:organisation) }
let(:log) { create(:lettings_log, :duplicate, owning_organisation: organisation) }
let!(:duplicate_log) { create(:lettings_log, :duplicate, owning_organisation: organisation) }
it "returns all duplicate logs for given log" do
expect(described_class.duplicate_logs(log).count).to eq(1)
@ -2852,7 +2853,7 @@ RSpec.describe LettingsLog do
end
context "when there is a deleted duplicate log" do
let!(:deleted_duplicate_log) { create(:lettings_log, :duplicate, discarded_at: Time.zone.now) }
let!(:deleted_duplicate_log) { create(:lettings_log, :duplicate, discarded_at: Time.zone.now, owning_organisation: organisation) }
it "does not return the deleted log as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(deleted_duplicate_log)
@ -2860,7 +2861,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with a different start date" do
let!(:different_start_date_log) { create(:lettings_log, :duplicate, startdate: Time.zone.tomorrow) }
let!(:different_start_date_log) { create(:lettings_log, :duplicate, startdate: Time.zone.tomorrow, owning_organisation: organisation) }
it "does not return a log with a different start date as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_start_date_log)
@ -2868,7 +2869,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with a different age1" do
let!(:different_age1) { create(:lettings_log, :duplicate, age1: 50) }
let!(:different_age1) { create(:lettings_log, :duplicate, age1: 50, owning_organisation: organisation) }
it "does not return a log with a different age1 as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_age1)
@ -2876,7 +2877,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with a different sex1" do
let!(:different_sex1) { create(:lettings_log, :duplicate, sex1: "F") }
let!(:different_sex1) { create(:lettings_log, :duplicate, sex1: "F", owning_organisation: organisation) }
it "does not return a log with a different sex1 as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_sex1)
@ -2884,7 +2885,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with a different ecstat1" do
let!(:different_ecstat1) { create(:lettings_log, :duplicate, ecstat1: 1) }
let!(:different_ecstat1) { create(:lettings_log, :duplicate, ecstat1: 1, owning_organisation: organisation) }
it "does not return a log with a different ecstat1 as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_ecstat1)
@ -2892,7 +2893,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with a different tcharge" do
let!(:different_tcharge) { create(:lettings_log, :duplicate, brent: 100) }
let!(:different_tcharge) { create(:lettings_log, :duplicate, brent: 100, owning_organisation: organisation) }
it "does not return a log with a different tcharge as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_tcharge)
@ -2900,7 +2901,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with a different tenancycode" do
let!(:different_tenancycode) { create(:lettings_log, :duplicate, tenancycode: "different") }
let!(:different_tenancycode) { create(:lettings_log, :duplicate, tenancycode: "different", owning_organisation: organisation) }
it "does not return a log with a different tenancycode as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_tenancycode)
@ -2908,7 +2909,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with a different postcode_full" do
let!(:different_postcode_full) { create(:lettings_log, :duplicate, postcode_full: "B1 1AA") }
let!(:different_postcode_full) { create(:lettings_log, :duplicate, postcode_full: "B1 1AA", owning_organisation: organisation) }
it "does not return a log with a different postcode_full as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_postcode_full)
@ -2916,7 +2917,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with nil values for duplicate check fields" do
let!(:duplicate_check_fields_not_given) { create(:lettings_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, postcode_known: 2, postcode_full: nil) }
let!(:duplicate_check_fields_not_given) { create(:lettings_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, postcode_known: 2, postcode_full: nil, owning_organisation: organisation) }
it "does not return a log with nil values as a duplicate" do
log.update!(age1: nil, sex1: nil, ecstat1: nil, postcode_known: 2, postcode_full: nil)
@ -2925,7 +2926,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with nil values for tenancycode" do
let!(:tenancycode_not_given) { create(:lettings_log, :duplicate, tenancycode: nil) }
let!(:tenancycode_not_given) { create(:lettings_log, :duplicate, tenancycode: nil, owning_organisation: organisation) }
it "returns the log as a duplicate if tenancy code is nil" do
log.update!(tenancycode: nil)
@ -2934,7 +2935,7 @@ RSpec.describe LettingsLog do
end
context "when there is a log with age1 not known" do
let!(:age1_not_known) { create(:lettings_log, :duplicate, age1_known: 1, age1: nil) }
let!(:age1_not_known) { create(:lettings_log, :duplicate, age1_known: 1, age1: nil, owning_organisation: organisation) }
it "returns the log as a duplicate if age1 is not known" do
log.update!(age1_known: 1, age1: nil)
@ -2946,8 +2947,8 @@ RSpec.describe LettingsLog do
let(:scheme) { create(:scheme) }
let(:location) { create(:location, scheme:) }
let(:location_2) { create(:location, scheme:) }
let(:supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:) }
let!(:duplicate_supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:) }
let(:supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:, owning_organisation: organisation) }
let!(:duplicate_supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:, owning_organisation: organisation) }
it "returns the log as a duplicate" do
expect(described_class.duplicate_logs(supported_housing_log)).to include(duplicate_supported_housing_log)

29
spec/models/sales_log_spec.rb

@ -161,8 +161,9 @@ RSpec.describe SalesLog, type: :model do
end
context "when filtering duplicate logs" do
let(:log) { create(:sales_log, :duplicate) }
let!(:duplicate_log) { create(:sales_log, :duplicate) }
let(:organisation) { create(:organisation) }
let(:log) { create(:sales_log, :duplicate, owning_organisation: organisation) }
let!(:duplicate_log) { create(:sales_log, :duplicate, owning_organisation: organisation) }
it "returns all duplicate logs for given log" do
expect(described_class.duplicate_logs(log).count).to eq(1)
@ -177,7 +178,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a deleted duplicate log" do
let!(:deleted_duplicate_log) { create(:sales_log, :duplicate, discarded_at: Time.zone.now) }
let!(:deleted_duplicate_log) { create(:sales_log, :duplicate, discarded_at: Time.zone.now, owning_organisation: organisation) }
it "does not return the deleted log as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(deleted_duplicate_log)
@ -185,7 +186,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log with a different sale date" do
let!(:different_sale_date_log) { create(:sales_log, :duplicate, saledate: Time.zone.tomorrow) }
let!(:different_sale_date_log) { create(:sales_log, :duplicate, saledate: Time.zone.tomorrow, owning_organisation: organisation) }
it "does not return a log with a different sale date as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_sale_date_log)
@ -193,7 +194,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log with a different age1" do
let!(:different_age1) { create(:sales_log, :duplicate, age1: 50) }
let!(:different_age1) { create(:sales_log, :duplicate, age1: 50, owning_organisation: organisation) }
it "does not return a log with a different age1 as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_age1)
@ -201,7 +202,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log with a different sex1" do
let!(:different_sex1) { create(:sales_log, :duplicate, sex1: "M") }
let!(:different_sex1) { create(:sales_log, :duplicate, sex1: "M", owning_organisation: organisation) }
it "does not return a log with a different sex1 as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_sex1)
@ -209,7 +210,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log with a different ecstat1" do
let!(:different_ecstat1) { create(:sales_log, :duplicate, ecstat1: 0) }
let!(:different_ecstat1) { create(:sales_log, :duplicate, ecstat1: 0, owning_organisation: organisation) }
it "does not return a log with a different ecstat1 as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_ecstat1)
@ -217,7 +218,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log with a different purchid" do
let!(:different_purchid) { create(:sales_log, :duplicate, purchid: "different") }
let!(:different_purchid) { create(:sales_log, :duplicate, purchid: "different", owning_organisation: organisation) }
it "does not return a log with a different purchid as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_purchid)
@ -225,7 +226,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log with a different postcode_full" do
let!(:different_postcode_full) { create(:sales_log, :duplicate, postcode_full: "B1 1AA") }
let!(:different_postcode_full) { create(:sales_log, :duplicate, postcode_full: "B1 1AA", owning_organisation: organisation) }
it "does not return a log with a different postcode_full as a duplicate" do
expect(described_class.duplicate_logs(log)).not_to include(different_postcode_full)
@ -233,7 +234,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log with nil values for duplicate check fields" do
let!(:duplicate_check_fields_not_given) { create(:sales_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, pcodenk: 1, postcode_full: nil) }
let!(:duplicate_check_fields_not_given) { create(:sales_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, pcodenk: 1, postcode_full: nil, owning_organisation: organisation) }
it "does not return a log with nil values as a duplicate" do
log.update!(age1: nil, sex1: nil, ecstat1: nil, pcodenk: 1, postcode_full: nil)
@ -242,7 +243,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log with nil values for purchid" do
let!(:purchid_not_given) { create(:sales_log, :duplicate, purchid: nil) }
let!(:purchid_not_given) { create(:sales_log, :duplicate, purchid: nil, owning_organisation: organisation) }
it "returns the log as a duplicate if purchid is nil" do
log.update!(purchid: nil)
@ -251,7 +252,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log age not known" do
let!(:age1_not_known) { create(:sales_log, :duplicate, age1_known: 1, age1: nil) }
let!(:age1_not_known) { create(:sales_log, :duplicate, age1_known: 1, age1: nil, owning_organisation: organisation) }
it "returns the log as a duplicate if age is not known" do
log.update!(age1_known: 1, age1: nil)
@ -260,7 +261,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log age pefers not to say" do
let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil) }
let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil, owning_organisation: organisation) }
it "returns the log as a duplicate if age is prefers not to say" do
log.update!(age1_known: 2, age1: nil)
@ -269,7 +270,7 @@ RSpec.describe SalesLog, type: :model do
end
context "when there is a log age pefers not to say and not known" do
let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil) }
let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil, owning_organisation: organisation) }
it "does not return the log as a duplicate if age is prefers not to say" do
log.update!(age1_known: 1, age1: nil)

67
spec/requests/duplicate_logs_controller_spec.rb

@ -34,7 +34,8 @@ RSpec.describe DuplicateLogsController, type: :request do
sign_in user
end
context "with multiple duplicate lettings logs" do
context "when viewing lettings logs duplicates" do
context "when there are multiple duplicate logs" do
let(:duplicate_logs) { create_list(:lettings_log, 2, :completed) }
before do
@ -57,6 +58,9 @@ RSpec.describe DuplicateLogsController, type: :request do
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: 21)
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
@ -67,7 +71,36 @@ RSpec.describe DuplicateLogsController, type: :request do
end
end
context "with multiple duplicate sales logs" do
context "when there are no more duplicate logs" do
before do
allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
get "/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}"
end
it "displays check your answers for each log with correct questions" do
expect(page).to have_content("Q5 - Tenancy start date", count: 1)
expect(page).to have_content("Q7 - Tenant code", count: 1)
expect(page).to have_content("Q12 - Postcode", count: 1)
expect(page).to have_content("Q32 - Lead tenant’s age", count: 1)
expect(page).to have_content("Q33 - Lead tenant’s gender identity", count: 1)
expect(page).to have_content("Q37 - Lead tenant’s working situation", count: 1)
expect(page).to have_content("Household rent and charges", count: 1)
expect(page).to have_link("Change", count: 7)
expect(page).to have_link("Change", href: "/lettings-logs/#{lettings_log.id}/tenant-code?original_log_id=#{lettings_log.id}&referrer=interruption_screen")
end
it "displays buttons to return to log" do
expect(page).to have_link("Back to Log #{lettings_log.id}", href: "/lettings-logs/#{lettings_log.id}")
end
it "displays no duplicates banner" do
expect(page).to have_content("This log had the same answers but it is no longer a duplicate. Make sure the answers are correct.")
end
end
end
context "when viewing sales logs duplicates" do
context "when there are multiple duplicate logs" do
let(:duplicate_logs) { create_list(:sales_log, 2, :completed) }
before do
@ -89,6 +122,9 @@ RSpec.describe DuplicateLogsController, type: :request do
expect(page).to have_content("Q25 - Buyer 1's working situation", count: 3)
expect(page).to have_content("Q15 - Postcode", count: 3)
expect(page).to have_link("Change", count: 18)
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
@ -98,6 +134,33 @@ RSpec.describe DuplicateLogsController, type: :request do
expect(page).to have_link("Keep this log and delete duplicates", href: "/sales-logs/#{duplicate_logs.second.id}/delete-duplicates?original_log_id=#{sales_log.id}")
end
end
context "when there are no more duplicate logs" do
before do
allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none)
get "/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}"
end
it "displays check your answers for each log with correct questions" do
expect(page).to have_content("Q1 - Sale completion date", count: 1)
expect(page).to have_content("Q2 - Purchaser code", count: 1)
expect(page).to have_content("Q20 - Lead buyer’s age", count: 1)
expect(page).to have_content("Q21 - Buyer 1’s gender identity", count: 1)
expect(page).to have_content("Q25 - Buyer 1's working situation", count: 1)
expect(page).to have_content("Q15 - Postcode", count: 1)
expect(page).to have_link("Change", count: 6)
expect(page).to have_link("Change", href: "/sales-logs/#{sales_log.id}/purchaser-code?original_log_id=#{sales_log.id}&referrer=interruption_screen")
end
it "displays buttons to return to log" do
expect(page).to have_link("Back to Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}")
end
it "displays no duplicates banner" do
expect(page).to have_content("This log had the same answers but it is no longer a duplicate. Make sure the answers are correct.")
end
end
end
end
end

82
spec/requests/form_controller_spec.rb

@ -732,6 +732,88 @@ RSpec.describe FormController, type: :request do
end
end
end
context "when the question was accessed from a duplicate logs screen" do
let(:lettings_log) { create(:lettings_log, :duplicate, created_by: user) }
let(:duplicate_log) { create(:lettings_log, :duplicate, created_by: user) }
let(:referrer) { "/lettings-logs/#{lettings_log.id}/lead-tenant-age?referrer=duplicate_logs&first_remaining_duplicate_id=#{duplicate_log.id}&original_log_id=#{lettings_log.id}" }
let(:params) do
{
id: lettings_log.id,
lettings_log: {
page: "lead_tenant_age",
age1: 20,
age1_known: 1,
},
}
end
before do
post "/lettings-logs/#{lettings_log.id}/lead-tenant-age", params:, headers: headers.merge({ "HTTP_REFERER" => referrer })
end
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to redirect_to("/lettings-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
end
context "and the answer didn't change" do
let(:params) do
{
id: lettings_log.id,
lettings_log: {
page: "lead_tenant_age",
age1: lettings_log.age1,
age1_known: lettings_log.age1_known,
},
}
end
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to have_http_status(:ok)
end
end
end
context "when the sales question was accessed from a duplicate logs screen" do
let(:sales_log) { create(:sales_log, :duplicate, created_by: user) }
let(:duplicate_log) { create(:sales_log, :duplicate, created_by: user) }
let(:referrer) { "/sales-logs/#{sales_log.id}/buyer-1-age?referrer=duplicate_logs&first_remaining_duplicate_id=#{duplicate_log.id}&original_log_id=#{sales_log.id}" }
let(:params) do
{
id: sales_log.id,
sales_log: {
page: "buyer_1_age",
age1: 20,
age1_known: 1,
},
}
end
before do
post "/sales-logs/#{sales_log.id}/buyer-1-age", params:, headers: headers.merge({ "HTTP_REFERER" => referrer })
end
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to redirect_to("/sales-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
end
context "and the answer didn't change" do
let(:params) do
{
id: sales_log.id,
sales_log: {
page: "buyer_1_age",
age1: sales_log.age1,
age1_known: sales_log.age1_known,
},
}
end
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to have_http_status(:ok)
end
end
end
end
context "with checkbox questions" do

8
spec/requests/sales_logs_controller_spec.rb

@ -354,8 +354,8 @@ RSpec.describe SalesLogsController, type: :request do
let(:user) { create(:user, organisation:) }
let(:bulk_upload) { create(:bulk_upload, :sales, user:) }
let!(:included_log) { create(:sales_log, :in_progress, bulk_upload:, owning_organisation: organisation) }
let!(:excluded_log) { create(:sales_log, :in_progress, owning_organisation: organisation) }
let!(:included_log) { create(:sales_log, :in_progress, purchid: "included_id", bulk_upload:, owning_organisation: organisation) }
let!(:excluded_log) { create(:sales_log, :in_progress, purchid: "excluded_id", owning_organisation: organisation) }
before do
create(:bulk_upload_error, bulk_upload:, col: "A", row: 1)
@ -364,8 +364,8 @@ RSpec.describe SalesLogsController, type: :request do
it "returns logs only associated with the bulk upload" do
get "/sales-logs?bulk_upload_id[]=#{bulk_upload.id}"
expect(page).to have_content(included_log.id)
expect(page).not_to have_content(excluded_log.id)
expect(page).to have_content(included_log.purchid)
expect(page).not_to have_content(excluded_log.purchid)
end
it "dislays bulk upload banner" do

Loading…
Cancel
Save