Browse Source

CLDC-2497 Review duplicate logs from logs index page (#1776)

* create helper module to find all logs for a given user that have duplicates
write associated tests
update methods on user model to retrieve logs related to a given user
create a scope on logs to retrieve logs created by a given user

* check for duplicates and display notification banner on the index page where appropriate
tests to cover the desired behaviour

* alter duplicate logs helper to return duplicates in a format that can be passed through the params
update associated tests

* create user journey to view duplicate sets adn navigate to a given set
create a new route and add it to the link from the logs index page
create controller method and related view file

* update path for duplicate logs in routes, update factories, refactor duplicates scope to eliminate raw sql

* write tests for the duplicate logs index page

* correct linting errors

* minor amendments from code review.

* temp

* Refactor

* Create a scope for duplicate groups

* fixes

* lint

* Update controller specs

* Put duplicate sets count behind a feature flag

* refactor index

* Update duplicate_sets_count

* Update scopes

* tests

* pass in original log id

* tests

* Add duplicate index page content

---------

Co-authored-by: Kat <katrina@kosiak.co.uk>
pull/1852/head
Arthur Campbell 1 year ago committed by GitHub
parent
commit
3c2a583586
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 23
      app/controllers/duplicate_logs_controller.rb
  2. 27
      app/controllers/lettings_logs_controller.rb
  3. 23
      app/controllers/organisations_controller.rb
  4. 27
      app/controllers/sales_logs_controller.rb
  5. 23
      app/helpers/duplicate_logs_helper.rb
  6. 51
      app/models/lettings_log.rb
  7. 4
      app/models/log.rb
  8. 8
      app/models/organisation.rb
  9. 24
      app/models/sales_log.rb
  10. 8
      app/models/user.rb
  11. 4
      app/services/feature_toggle.rb
  12. 41
      app/views/duplicate_logs/index.html.erb
  13. 6
      app/views/logs/index.html.erb
  14. 6
      app/views/organisations/logs.html.erb
  15. 3
      config/locales/en.yml
  16. 4
      config/routes.rb
  17. 5
      spec/factories/lettings_log.rb
  18. 1
      spec/factories/sales_log.rb
  19. 136
      spec/helpers/duplicate_logs_helper_spec.rb
  20. 183
      spec/models/lettings_log_spec.rb
  21. 157
      spec/models/sales_log_spec.rb
  22. 74
      spec/requests/duplicate_logs_controller_spec.rb
  23. 8
      spec/requests/form_controller_spec.rb
  24. 50
      spec/requests/lettings_logs_controller_spec.rb

23
app/controllers/duplicate_logs_controller.rb

@ -1,8 +1,11 @@
class DuplicateLogsController < ApplicationController
include DuplicateLogsHelper
before_action :authenticate_user!
before_action :find_resource_by_named_id
before_action :find_duplicate_logs
before_action :find_duplicates_for_a_log
before_action :find_original_log
before_action :find_all_duplicates, only: [:index]
def show
if @log
@ -22,6 +25,13 @@ class DuplicateLogsController < ApplicationController
render "logs/delete_duplicates"
end
def index
render_not_found if @duplicates.blank?
@duplicate_sets_count = @duplicates[:lettings].count + @duplicates[:sales].count
render_not_found if @duplicate_sets_count.zero?
end
private
def find_resource_by_named_id
@ -32,7 +42,7 @@ private
end
end
def find_duplicate_logs
def find_duplicates_for_a_log
return unless @log
@duplicate_logs = if @log.lettings?
@ -42,6 +52,15 @@ private
end
end
def find_all_duplicates
return @duplicates = duplicates_for_user(current_user) if current_user.data_provider?
organisation = current_user.support? ? Organisation.find(params[:organisation_id]) : current_user.organisation
return unless organisation
@duplicates = duplicates_for_organisation(organisation)
end
def duplicate_check_question_ids
if @log.lettings?
["owning_organisation_id",

27
app/controllers/lettings_logs_controller.rb

@ -1,4 +1,6 @@
class LettingsLogsController < LogsController
include DuplicateLogsHelper
rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
before_action :find_resource, only: %i[update show]
@ -11,20 +13,17 @@ class LettingsLogsController < LogsController
before_action :redirect_if_bulk_upload_resolved, only: [:index]
def index
respond_to do |format|
format.html do
all_logs = current_user.lettings_logs.visible
unpaginated_filtered_logs = filter_manager.filtered_logs(all_logs, search_term, session_filters)
@delete_logs_path = delete_logs_lettings_logs_path(search: search_term)
@pagy, @logs = pagy(unpaginated_filtered_logs)
@searched = search_term.presence
@total_count = all_logs.size
@unresolved_count = all_logs.unresolved.created_by(current_user).count
@filter_type = "lettings_logs"
render "logs/index"
end
end
all_logs = current_user.lettings_logs.visible
unpaginated_filtered_logs = filter_manager.filtered_logs(all_logs, search_term, session_filters)
@delete_logs_path = delete_logs_lettings_logs_path(search: search_term)
@pagy, @logs = pagy(unpaginated_filtered_logs)
@searched = search_term.presence
@total_count = all_logs.size
@unresolved_count = all_logs.unresolved.created_by(current_user).count
@filter_type = "lettings_logs"
@duplicate_sets_count = FeatureToggle.duplicate_summary_enabled? && !current_user.support? ? duplicate_sets_count(current_user, current_user.organisation) : 0
render "logs/index"
end
def create

23
app/controllers/organisations_controller.rb

@ -1,6 +1,7 @@
class OrganisationsController < ApplicationController
include Pagy::Backend
include Modules::SearchFilter
include DuplicateLogsHelper
before_action :authenticate_user!
before_action :find_resource, except: %i[index new create]
@ -96,18 +97,15 @@ class OrganisationsController < ApplicationController
organisation_logs = LettingsLog.visible.where(owning_organisation_id: @organisation.id)
unpaginated_filtered_logs = filter_manager.filtered_logs(organisation_logs, search_term, session_filters)
respond_to do |format|
format.html do
@search_term = search_term
@pagy, @logs = pagy(unpaginated_filtered_logs)
@delete_logs_path = delete_lettings_logs_organisation_path(search: @search_term)
@searched = search_term.presence
@total_count = organisation_logs.size
@log_type = :lettings
@filter_type = "lettings_logs"
render "logs", layout: "application"
end
end
@search_term = search_term
@pagy, @logs = pagy(unpaginated_filtered_logs)
@delete_logs_path = delete_lettings_logs_organisation_path(search: @search_term)
@searched = search_term.presence
@total_count = organisation_logs.size
@log_type = :lettings
@filter_type = "lettings_logs"
@duplicate_sets_count = FeatureToggle.duplicate_summary_enabled? ? duplicate_sets_count(current_user, @organisation) : 0
render "logs", layout: "application"
end
def download_lettings_csv
@ -136,6 +134,7 @@ class OrganisationsController < ApplicationController
@total_count = organisation_logs.size
@log_type = :sales
@filter_type = "sales_logs"
@duplicate_sets_count = FeatureToggle.duplicate_summary_enabled? ? duplicate_sets_count(current_user, @organisation) : 0
render "logs", layout: "application"
end

27
app/controllers/sales_logs_controller.rb

@ -1,4 +1,6 @@
class SalesLogsController < LogsController
include DuplicateLogsHelper
rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv]
@ -13,20 +15,17 @@ class SalesLogsController < LogsController
end
def index
respond_to do |format|
format.html do
all_logs = current_user.sales_logs.visible
unpaginated_filtered_logs = filter_manager.filtered_logs(all_logs, search_term, session_filters)
@delete_logs_path = delete_logs_sales_logs_path(search: search_term)
@search_term = search_term
@pagy, @logs = pagy(unpaginated_filtered_logs)
@searched = search_term.presence
@total_count = all_logs.size
@filter_type = "sales_logs"
render "logs/index"
end
end
all_logs = current_user.sales_logs.visible
unpaginated_filtered_logs = filter_manager.filtered_logs(all_logs, search_term, session_filters)
@delete_logs_path = delete_logs_sales_logs_path(search: search_term)
@search_term = search_term
@pagy, @logs = pagy(unpaginated_filtered_logs)
@searched = search_term.presence
@total_count = all_logs.size
@filter_type = "sales_logs"
@duplicate_sets_count = FeatureToggle.duplicate_summary_enabled? && !current_user.support? ? duplicate_sets_count(current_user, current_user.organisation) : 0
render "logs/index"
end
def show

23
app/helpers/duplicate_logs_helper.rb

@ -27,4 +27,27 @@ module DuplicateLogsHelper
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
def duplicates_for_user(user)
{
lettings: user.duplicate_lettings_logs_sets,
sales: user.duplicate_sales_logs_sets,
}
end
def duplicates_for_organisation(organisation)
{
lettings: organisation.duplicate_lettings_logs_sets,
sales: organisation.duplicate_sales_logs_sets,
}
end
def duplicate_sets_count(user, organisation)
duplicates = user.data_provider? ? duplicates_for_user(user) : duplicates_for_organisation(organisation)
duplicates[:lettings].count + duplicates[:sales].count
end
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
end

51
app/models/lettings_log.rb

@ -45,31 +45,58 @@ class LettingsLog < Log
scope :filter_by_location_postcode, ->(postcode_full) { left_joins(:location).where("REPLACE(locations.postcode, ' ', '') ILIKE ?", "%#{postcode_full.delete(' ')}%") }
scope :search_by, lambda { |param|
filter_by_location_postcode(param)
.or(filter_by_tenant_code(param))
.or(filter_by_propcode(param))
.or(filter_by_postcode(param))
.or(filter_by_id(param))
.or(filter_by_tenant_code(param))
.or(filter_by_propcode(param))
.or(filter_by_postcode(param))
.or(filter_by_id(param))
}
scope :after_date, ->(date) { where("lettings_logs.startdate >= ?", date) }
scope :unresolved, -> { where(unresolved: true) }
scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) }
scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) }
scope :filter_by_managing_organisation, ->(managing_organisation, _user = nil) { where(managing_organisation:) }
scope :age1_answered, -> { where.not(age1: nil).or(where(age1_known: 1)) }
scope :tcharge_answered, -> { where.not(tcharge: nil).or(where(household_charge: 1)).or(where(is_carehome: 1)) }
scope :chcharge_answered, -> { where.not(chcharge: nil).or(where(is_carehome: [nil, 0])) }
scope :location_for_log_answered, ->(log) { where(location_id: log.location_id).or(where(needstype: 1)) }
scope :postcode_for_log_answered, ->(log) { where(postcode_full: log.postcode_full).or(where(needstype: 2)) }
scope :location_answered, -> { where.not(location_id: nil).or(where(needstype: 1)) }
scope :postcode_answered, -> { where.not(postcode_full: nil).or(where(needstype: 2)) }
scope :duplicate_logs, lambda { |log|
visible
.where(log.slice(*DUPLICATE_LOG_ATTRIBUTES))
.where.not(id: log.id)
.where.not(startdate: nil)
.where.not(sex1: nil)
.where.not(ecstat1: nil)
.where.not(needstype: nil)
.where("age1 IS NOT NULL OR age1_known = 1")
.where("tcharge IS NOT NULL OR household_charge = 1 OR is_carehome = 1")
.where("chcharge IS NOT NULL OR is_carehome IS NULL OR is_carehome = 0")
.where("location_id = ? OR needstype = 1", log.location_id)
.where("postcode_full = ? OR needstype = 2", log.postcode_full)
.age1_answered
.tcharge_answered
.chcharge_answered
.location_for_log_answered(log)
.postcode_for_log_answered(log)
.where(log.slice(*DUPLICATE_LOG_ATTRIBUTES))
}
scope :duplicate_sets, lambda { |created_by_id = nil|
scope = visible
.group(*DUPLICATE_LOG_ATTRIBUTES, :postcode_full, :location_id)
.where.not(startdate: nil)
.where.not(sex1: nil)
.where.not(ecstat1: nil)
.where.not(needstype: nil)
.age1_answered
.tcharge_answered
.chcharge_answered
.location_answered
.postcode_answered
.having(
"COUNT(*) > 1",
)
if created_by_id
scope = scope.having("MAX(CASE WHEN created_by_id = ? THEN 1 ELSE 0 END) >= 1", created_by_id)
end
scope.pluck("ARRAY_AGG(id)")
}
AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze

4
app/models/log.rb

@ -81,6 +81,10 @@ class Log < ApplicationRecord
collection_start_year
end
def setup_completed?
form.setup_sections.all? { |sections| sections.subsections.all? { |subsection| subsection.status(self) == :completed } }
end
def lettings?
false
end

8
app/models/organisation.rb

@ -132,4 +132,12 @@ class Organisation < ApplicationRecord
:active
end
def duplicate_lettings_logs_sets
lettings_logs.duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] }
end
def duplicate_sales_logs_sets
sales_logs.duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] }
end
end

24
app/models/sales_log.rb

@ -42,6 +42,7 @@ class SalesLog < Log
}
scope :filter_by_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) }
scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) }
scope :age1_answered, -> { where.not(age1: nil).or(where(age1_known: [1, 2])) }
scope :duplicate_logs, lambda { |log|
visible.where(log.slice(*DUPLICATE_LOG_ATTRIBUTES))
.where.not(id: log.id)
@ -49,10 +50,27 @@ class SalesLog < Log
.where.not(sex1: nil)
.where.not(ecstat1: nil)
.where.not(postcode_full: nil)
.where("age1 IS NOT NULL OR age1_known = 1 OR age1_known = 2")
.age1_answered
}
scope :after_date, ->(date) { where("saledate >= ?", date) }
scope :duplicate_sets, lambda { |created_by_id = nil|
scope = visible
.group(*DUPLICATE_LOG_ATTRIBUTES)
.where.not(saledate: nil)
.where.not(sex1: nil)
.where.not(ecstat1: nil)
.where.not(postcode_full: nil)
.age1_answered
.having("COUNT(*) > 1")
if created_by_id
scope = scope.having("MAX(CASE WHEN created_by_id = ? THEN 1 ELSE 0 END) >= 1", created_by_id)
end
scope.pluck("ARRAY_AGG(id)")
}
OPTIONAL_FIELDS = %w[purchid othtype].freeze
RETIREMENT_AGES = { "M" => 65, "F" => 60, "X" => 65 }.freeze
DUPLICATE_LOG_ATTRIBUTES = %w[owning_organisation_id purchid saledate age1_known age1 sex1 ecstat1 postcode_full].freeze
@ -120,10 +138,6 @@ class SalesLog < Log
collection_start_year < 2023
end
def setup_completed?
form.setup_sections.all? { |sections| sections.subsections.all? { |subsection| subsection.status(self) == :completed } }
end
def unresolved
false
end

8
app/models/user.rb

@ -203,6 +203,14 @@ class User < ApplicationRecord
super && active?
end
def duplicate_lettings_logs_sets
lettings_logs.duplicate_sets(id).map { |array_str| array_str ? array_str.map(&:to_i) : [] }
end
def duplicate_sales_logs_sets
sales_logs.duplicate_sets(id).map { |array_str| array_str ? array_str.map(&:to_i) : [] }
end
protected
# Checks whether a password is needed or not. For validations only.

4
app/services/feature_toggle.rb

@ -33,4 +33,8 @@ class FeatureToggle
def self.deduplication_flow_enabled?
!Rails.env.production? && !Rails.env.staging?
end
def self.duplicate_summary_enabled?
!Rails.env.production?
end
end

41
app/views/duplicate_logs/index.html.erb

@ -0,0 +1,41 @@
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<h1 class="govuk-heading-l"><%= duplicate_list_header(@duplicates[:lettings].count + @duplicates[:sales].count) %></h1>
<p class="govuk-body">
These logs are duplicates because they have the same answers for certain fields.
</p>
<p class="govuk-body">
Review each set of logs and either delete any duplicates or change any incorrect answers.
</p>
</div>
</div>
<%= govuk_table do |table| %>
<%= table.head do |head| %>
<%= head.row do |row| %>
<% row.cell header: true, text: "Type of logs" %>
<% row.cell header: true, text: "Log IDs" %>
<% row.cell header: true %>
<% end %>
<% end %>
<%= table.body do |body| %>
<% @duplicates[:lettings].each do |duplicate_set| %>
<% body.row do |row| %>
<% row.cell text: "Lettings" %>
<% row.cell text: duplicate_set.map { |id| "Log #{id}" }.join(", ") %>
<% row.cell do %>
<%= govuk_link_to "Review logs", lettings_log_duplicate_logs_path(duplicate_set.first, original_log_id: duplicate_set.first) %>
<% end %>
<% end %>
<% end %>
<% @duplicates[:sales].each do |duplicate_set| %>
<% body.row do |row| %>
<% row.cell text: "Sales" %>
<% row.cell text: duplicate_set.map { |id| "Log #{id}" }.join(", ") %>
<% row.cell do %>
<%= govuk_link_to "Review logs", sales_log_duplicate_logs_path(duplicate_set.first, original_log_id: duplicate_set.first) %>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>

6
app/views/logs/index.html.erb

@ -8,6 +8,12 @@
organisation: @organisation,
) %>
<% if @duplicate_sets_count&.positive? %>
<%= govuk_notification_banner(title_text: "Important", text: govuk_link_to("Review logs", duplicate_logs_path)) do |banner| %>
<% banner.with_heading(text: I18n.t("notification.duplicate_sets", count: @duplicate_sets_count)) %>
<% end %>
<% end %>
<% if current_page?(controller: 'lettings_logs', action: 'index') %>
<% if @unresolved_count > 0 %>
<%= govuk_notification_banner(

6
app/views/organisations/logs.html.erb

@ -10,6 +10,12 @@
organisation: @organisation,
) %>
<% if @duplicate_sets_count&.positive? %>
<%= govuk_notification_banner(title_text: "Important", text: govuk_link_to("Review logs", organisation_duplicates_path(@organisation))) do |banner| %>
<% banner.with_heading(text: I18n.t("notification.duplicate_sets", count: @duplicate_sets_count)) %>
<% end %>
<% end %>
<% if current_user.support? %>
<%= render SubNavigationComponent.new(
items: secondary_items(request.path, @organisation.id),

3
config/locales/en.yml

@ -189,6 +189,9 @@ en:
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>"
duplicate_sets:
one: "There is %{count} set of duplicate logs"
other: "There are %{count} sets of duplicate logs"
validations:
organisation:

4
config/routes.rb

@ -105,6 +105,8 @@ Rails.application.routes.draw do
end
end
resources :duplicate_logs, only: [:index], path: "/duplicate-logs"
resources :users do
get "edit-dpo", to: "users#dpo"
get "edit-key-contact", to: "users#key_contact"
@ -117,6 +119,8 @@ Rails.application.routes.draw do
end
resources :organisations do
get "duplicates", to: "duplicate_logs#index"
member do
get "details", to: "organisations#details"
get "data-sharing-agreement", to: "organisations#data_sharing_agreement"

5
spec/factories/lettings_log.rb

@ -35,12 +35,13 @@ FactoryBot.define do
hhmemb { 1 }
end
trait :duplicate do
setup_completed
status { 1 }
needstype { 1 }
tenancycode { "same tenancy code" }
postcode_full { "A1 1AA" }
uprn_known { 0 }
declaration { 1 }
age1_known { 0 }
age1 { 18 }
sex1 { "M" }
ecstat1 { 0 }
@ -51,8 +52,6 @@ FactoryBot.define do
supcharg { 35 }
tcharge { 325 }
propcode { "same property code" }
renewal { 1 }
rent_type { 1 }
startdate { Time.zone.today }
end
trait :completed do

1
spec/factories/sales_log.rb

@ -41,6 +41,7 @@ FactoryBot.define do
type { 8 }
jointpur { 2 }
saledate { Time.zone.today }
age1_known { 1 }
age1 { 20 }
sex1 { "F" }
ecstat1 { 1 }

136
spec/helpers/duplicate_logs_helper_spec.rb

@ -0,0 +1,136 @@
require "rails_helper"
RSpec.describe DuplicateLogsHelper do
describe "#duplicates_for_user" do
let(:org) { create(:organisation) }
let(:other_org) { create(:organisation) }
let(:current_user) { create(:user, organisation: org) }
let(:user_same_org) { create(:user, organisation: org) }
let(:user_other_org) { create(:user, organisation: other_org) }
let!(:lettings_log) { create(:lettings_log, :duplicate, created_by: current_user) }
let!(:sales_log) { create(:sales_log, :duplicate, created_by: current_user) }
let(:result) { duplicates_for_user(current_user) }
context "when there are no duplicates" do
it "returns empty duplicates" do
expect(result).to eq({ lettings: [], sales: [] })
end
end
context "when there are duplicates in another org" do
before do
create(:lettings_log, :duplicate, created_by: user_other_org)
create(:sales_log, :duplicate, created_by: user_other_org)
end
it "does not locate duplicates" do
expect(result).to eq({ lettings: [], sales: [] })
end
end
context "when another user in the same org has created a duplicate lettings log" do
let!(:duplicate_lettings_log) { create(:lettings_log, :duplicate, created_by: user_same_org) }
it "returns the ids of the duplicates in a hash under the lettings key" do
expect(result).to be_a Hash
expect(result[:lettings]).to match_array [[lettings_log.id, duplicate_lettings_log.id]]
end
end
context "when another user in the same org has created a duplicate sales log" do
let!(:duplicate_sales_log) { create(:sales_log, :duplicate, created_by: user_same_org) }
it "returns the ids of the duplicates in a hash under the sales key" do
expect(result).to be_a Hash
expect(result[:sales]).to match_array [[sales_log.id, duplicate_sales_log.id]]
end
end
context "when there is a set of duplicate lettings logs not associated with the user" do
before do
create_list(:lettings_log, 3, :duplicate, tenancycode: "other", owning_organisation: org)
end
it "returns the ids of the duplicates in a hash under the lettings key" do
expect(result).to be_a Hash
expect(result[:lettings]).to be_empty
end
end
context "when there is a set of duplicate sales logs not associated with the user" do
before do
create_list(:sales_log, 3, :duplicate, purchid: "other", owning_organisation: org)
end
it "returns the ids of the duplicates in a hash under the sales key" do
expect(result).to be_a Hash
expect(result[:sales]).to be_empty
end
end
context "when there are multiple sets of duplicates across lettings and sales" do
let!(:duplicate_lettings_log) { create(:lettings_log, :duplicate, created_by: user_same_org) }
let!(:duplicate_sales_log) { create(:sales_log, :duplicate, created_by: user_same_org) }
let!(:further_sales_log) { create(:sales_log, :duplicate, purchid: "further", created_by: current_user) }
let!(:further_duplicate_sales_logs) { create_list(:sales_log, 2, :duplicate, purchid: "further", created_by: user_same_org) }
it "returns them all with no repeats" do
expected_sales_duplicates_result = [
[sales_log.id, duplicate_sales_log.id],
[further_sales_log.id, *further_duplicate_sales_logs.map(&:id)],
]
expect(result[:lettings]).to match_array [[lettings_log.id, duplicate_lettings_log.id]]
expect(result[:sales]).to match_array expected_sales_duplicates_result
end
end
end
describe "#duplicates_for_organisation" do
let(:organisation) { create(:organisation) }
let(:sales_logs) { SalesLog.filter_by_organisation(organisation) }
context "when there are no duplicates" do
it "returns empty duplicates" do
expect(duplicates_for_organisation(organisation)).to eq({ lettings: [], sales: [] })
end
end
context "when there are multiple sets of sales duplicates" do
let!(:duplicate_sales_logs) { create_list(:sales_log, 4, :duplicate, purchid: "set 1", owning_organisation: organisation) }
let!(:duplicate_sales_logs_too) { create_list(:sales_log, 5, :duplicate, postcode_full: "B1 1BB", owning_organisation: organisation) }
let!(:duplicate_sales_logs_3) { create_list(:sales_log, 3, :duplicate, age1: 38, owning_organisation: organisation) }
let!(:duplicate_lettings_logs) { create_list(:lettings_log, 4, :duplicate, tenancycode: "set 1", owning_organisation: organisation) }
let!(:duplicate_lettings_logs_too) { create_list(:lettings_log, 5, :duplicate, postcode_full: "B1 1BB", owning_organisation: organisation) }
let!(:duplicate_lettings_logs_3) { create_list(:lettings_log, 3, :duplicate, age1: 38, owning_organisation: organisation) }
before do
create_list(:sales_log, 3, :duplicate, discarded_at: Time.zone.now, status: 4, owning_organisation: organisation)
create_list(:lettings_log, 3, :duplicate, discarded_at: Time.zone.now, status: 4, owning_organisation: organisation)
end
it "returns them all with no repeats" do
expected_sales_duplicates_result = [
duplicate_sales_logs.map(&:id),
duplicate_sales_logs_too.map(&:id),
duplicate_sales_logs_3.map(&:id),
]
expected_lettings_duplicates_result = [
duplicate_lettings_logs.map(&:id),
duplicate_lettings_logs_too.map(&:id),
duplicate_lettings_logs_3.map(&:id),
]
expect(duplicates_for_organisation(organisation)[:lettings]).to match_array(
expected_lettings_duplicates_result.map { |nested_array| match_array(nested_array) },
)
expect(duplicates_for_organisation(organisation)[:sales]).to match_array(
expected_sales_duplicates_result.map { |nested_array| match_array(nested_array) },
)
end
end
end
end

183
spec/models/lettings_log_spec.rb

@ -2980,6 +2980,189 @@ RSpec.describe LettingsLog do
end
end
end
context "when getting list of duplicate logs" do
let(:organisation) { create(:organisation) }
let!(:log) { create(:lettings_log, :duplicate, owning_organisation: organisation) }
let!(:duplicate_log) { create(:lettings_log, :duplicate, owning_organisation: organisation) }
let(:duplicate_sets) { described_class.duplicate_sets }
it "returns a list of duplicates in the same organisation" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
context "when there is a deleted duplicate log" do
before do
create(:lettings_log, :duplicate, discarded_at: Time.zone.now, status: 4)
end
it "does not return the deleted log as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different start date" do
before do
create(:lettings_log, :duplicate, startdate: Time.zone.tomorrow)
end
it "does not return a log with a different start date as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different age1" do
before do
create(:lettings_log, :duplicate, age1: 50)
end
it "does not return a log with a different age1 as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different sex1" do
before do
create(:lettings_log, :duplicate, sex1: "F")
end
it "does not return a log with a different sex1 as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different ecstat1" do
before do
create(:lettings_log, :duplicate, ecstat1: 1)
end
it "does not return a log with a different ecstat1 as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different tcharge" do
before do
create(:lettings_log, :duplicate, brent: 100)
end
it "does not return a log with a different tcharge as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(duplicate_log.id, log.id)
end
end
context "when there is a log with a different tenancycode" do
before do
create(:lettings_log, :duplicate, tenancycode: "different")
end
it "does not return a log with a different tenancycode as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different postcode_full" do
before do
create(:lettings_log, :duplicate, postcode_full: "B1 1AA")
end
it "does not return a log with a different postcode_full as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with nil values for duplicate check fields" do
before do
create(:lettings_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, postcode_known: 2, postcode_full: nil)
end
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)
expect(duplicate_sets).to be_empty
end
end
context "when there is a log with nil values for tenancycode" do
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)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, tenancycode_not_given.id)
end
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, owning_organisation: organisation) }
it "returns the log as a duplicate if age1 is not known" do
log.update!(age1_known: 1, age1: nil)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(age1_not_known.id, log.id)
end
end
context "when there is a duplicate supported housing log" 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:, 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(duplicate_sets.count).to eq(2)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
expect(duplicate_sets.second).to contain_exactly(duplicate_supported_housing_log.id, supported_housing_log.id)
end
it "does not return the log if the locations are different" do
duplicate_supported_housing_log.update!(location: location_2)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
it "does not compare tcharge if there are no household charges" do
supported_housing_log.update!(household_charge: 1, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil, owning_organisation: organisation)
duplicate_supported_housing_log.update!(household_charge: 1, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil, owning_organisation: organisation)
expect(duplicate_sets.count).to eq(2)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
expect(duplicate_sets.second).to contain_exactly(supported_housing_log.id, duplicate_supported_housing_log.id)
end
it "does not return logs not associated with the user if user is given" do
user = create(:user)
supported_housing_log.update!(created_by: user, owning_organisation: user.organisation)
duplicate_supported_housing_log.update!(owning_organisation: user.organisation)
duplicate_sets = described_class.duplicate_sets(user.id)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(supported_housing_log.id, duplicate_supported_housing_log.id)
end
it "compares chcharge if it's a carehome" do
supported_housing_log.update!(is_carehome: 1, chcharge: 100, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil, owning_organisation: organisation)
duplicate_supported_housing_log.update!(is_carehome: 1, chcharge: 100, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil, owning_organisation: organisation)
expect(duplicate_sets.count).to eq(2)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
expect(duplicate_sets.second).to contain_exactly(supported_housing_log.id, duplicate_supported_housing_log.id)
end
it "does not return a duplicate if carehome charge is not given" do
supported_housing_log.update!(is_carehome: 1, chcharge: nil, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil, owning_organisation: organisation)
duplicate_supported_housing_log.update!(is_carehome: 1, chcharge: nil, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil, owning_organisation: organisation)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
end
end
describe "#retirement_age_for_person" do

157
spec/models/sales_log_spec.rb

@ -279,6 +279,163 @@ RSpec.describe SalesLog, type: :model do
end
end
context "when getting list of duplicate logs" do
let(:organisation) { create(:organisation) }
let!(:log) { create(:sales_log, :duplicate, owning_organisation: organisation) }
let!(:duplicate_log) { create(:sales_log, :duplicate, owning_organisation: organisation) }
let(:duplicate_sets) { described_class.duplicate_sets }
it "returns a list of duplicates in the same organisation" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
context "when there is a deleted duplicate log" do
before do
create(:sales_log, :duplicate, discarded_at: Time.zone.now, status: 4)
end
it "does not return the deleted log as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different sale date" do
before do
create(:sales_log, :duplicate, saledate: Time.zone.tomorrow)
end
it "does not return a log with a different sale date as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different age1" do
before do
create(:sales_log, :duplicate, age1: 50)
end
it "does not return a log with a different age1 as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different sex1" do
before do
create(:sales_log, :duplicate, sex1: "X")
end
it "does not return a log with a different sex1 as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different ecstat1" do
before do
create(:sales_log, :duplicate, ecstat1: 9)
end
it "does not return a log with a different ecstat1 as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different purchid" do
before do
create(:sales_log, :duplicate, purchid: "different")
end
it "does not return a log with a different purchid as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with a different postcode_full" do
before do
create(:sales_log, :duplicate, postcode_full: "B1 1AA")
end
it "does not return a log with a different postcode_full as a duplicate" do
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
context "when there is a log with nil values for duplicate check fields" do
before do
create(:sales_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, pcodenk: 1, postcode_full: nil)
end
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)
expect(duplicate_sets).to be_empty
end
end
context "when there is a log with nil values for purchid" do
let!(:purchid_not_given) { create(:sales_log, :duplicate, purchid: nil, owning_organisation: organisation) }
it "returns the log as a duplicate if tenancy code is nil" do
log.update!(purchid: nil)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, purchid_not_given.id)
end
end
context "when there is a log with age1 not known" do
let!(:age1_not_known) { create(:sales_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)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(age1_not_known.id, log.id)
end
end
context "when there is a log with age1 prefers not to say" do
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 age1 is prefers not to say" do
log.update!(age1_known: 2, age1: nil)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(age1_prefers_not_to_say.id, log.id)
end
end
context "when there is a log with age1 not known and prefers not to say" do
before do
create(:sales_log, :duplicate, age1_known: 2, age1: nil)
end
it "doe not return the log as a duplicate" do
log.update!(age1_known: 1, age1: nil)
expect(duplicate_sets).to be_empty
end
end
context "when user is given" do
let(:user) { create(:user) }
before do
create_list(:sales_log, 2, :duplicate, purchid: "other duplicates")
log.update!(created_by: user, owning_organisation: user.organisation)
end
it "does not return logs not associated with the given user" do
duplicate_log.update!(owning_organisation: user.organisation)
duplicate_sets = described_class.duplicate_sets(user.id)
expect(duplicate_sets.count).to eq(1)
expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id)
end
end
end
describe "derived variables" do
let(:sales_log) { create(:sales_log, :completed) }

74
spec/requests/duplicate_logs_controller_spec.rb

@ -313,4 +313,78 @@ RSpec.describe DuplicateLogsController, type: :request do
end
end
end
describe "GET #index" do
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
end
context "when the user is support" do
let(:user) { create(:user, :support) }
it "renders not found" do
get duplicate_logs_path(organisation_id: user.organisation.id)
expect(response).to have_http_status(:not_found)
end
end
context "when the user is a data coordinator" do
let(:user) { create(:user, :data_coordinator) }
before do
allow(user.organisation).to receive(:duplicate_lettings_logs_sets).and_return([[1, 2], [3, 4, 5]])
allow(user.organisation).to receive(:duplicate_sales_logs_sets).and_return([[11, 12]])
end
it "gets organisation duplicates" do
expect(user.organisation).to receive(:duplicate_lettings_logs_sets)
expect(user.organisation).to receive(:duplicate_sales_logs_sets)
get duplicate_logs_path(organisation_id: user.organisation.id)
end
end
context "when the user is a provider" do
let(:user) { create(:user) }
before do
allow(user).to receive(:duplicate_lettings_logs_sets).and_return([[1, 2], [3, 4, 5]])
allow(user).to receive(:duplicate_sales_logs_sets).and_return([[11, 12]])
end
it "calls the helper method to retrieve duplicates for the current user" do
expect(user).to receive(:duplicate_lettings_logs_sets)
expect(user).to receive(:duplicate_sales_logs_sets)
get duplicate_logs_path
end
describe "viewing the page" do
before do
get duplicate_logs_path
end
it "has the correct headers" do
expect(page).to have_content("Type of logs")
expect(page).to have_content("Log IDs")
end
it "has the correct number of rows for each log type" do
expect(page).to have_selector("tbody tr td", text: "Lettings", count: 2)
expect(page).to have_selector("tbody tr td", text: "Sales", count: 1)
end
it "shows the log ids for each set of duplicates" do
expect(page).to have_content("Log 1, Log 2")
expect(page).to have_content("Log 3, Log 4, Log 5")
expect(page).to have_content("Log 11, Log 12")
end
it "shows links for each set of duplciates" do
expect(page).to have_link("Review logs", href: lettings_log_duplicate_logs_path(1, original_log_id: 1))
expect(page).to have_link("Review logs", href: lettings_log_duplicate_logs_path(3, original_log_id: 3))
expect(page).to have_link("Review logs", href: sales_log_duplicate_logs_path(11, original_log_id: 11))
end
end
end
end
end

8
spec/requests/form_controller_spec.rb

@ -797,7 +797,7 @@ RSpec.describe FormController, type: :request do
end
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to have_http_status(:ok)
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
end
end
end
@ -805,13 +805,13 @@ RSpec.describe FormController, type: :request do
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(: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}&referrer=duplicate_logs" }
let(:params) do
{
id: sales_log.id,
sales_log: {
page: "buyer_1_age",
age1: 20,
age1: 29,
age1_known: 1,
},
}
@ -838,7 +838,7 @@ RSpec.describe FormController, type: :request do
end
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to have_http_status(:ok)
expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
end
end
end

50
spec/requests/lettings_logs_controller_spec.rb

@ -238,8 +238,6 @@ RSpec.describe LettingsLogsController, type: :request do
let(:headers) { { "Accept" => "text/html" } }
context "when you visit the index page" do
let(:user) { FactoryBot.create(:user, :support) }
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
@ -308,6 +306,18 @@ RSpec.describe LettingsLogsController, type: :request do
expect(page).to have_link("Download (CSV, codes only)", href: "/lettings-logs/csv-download?codes_only=true")
end
context "when there are duplicate logs for this user" do
before do
FactoryBot.create_list(:lettings_log, 2, :duplicate, owning_organisation: user.organisation, created_by: user)
end
it "does not show a notification banner even if there are duplicate logs for this user" do
get lettings_logs_path
expect(page).not_to have_content "duplicate logs"
expect(page).not_to have_link "Review logs"
end
end
context "when there are no logs in the database" do
before do
LettingsLog.destroy_all
@ -615,6 +625,12 @@ RSpec.describe LettingsLogsController, type: :request do
expect(page).not_to have_link("Download (CSV, codes only)")
end
it "does not show a notification banner even if there are duplicate logs for this user" do
get lettings_logs_path
expect(page).not_to have_content "duplicate logs"
expect(page).not_to have_link "Review logs"
end
context "when using a search query" do
let(:logs) { FactoryBot.create_list(:lettings_log, 3, :completed, owning_organisation: user.organisation, created_by: user) }
let(:log_to_search) { FactoryBot.create(:lettings_log, :completed, owning_organisation: user.organisation, created_by: user) }
@ -871,6 +887,36 @@ RSpec.describe LettingsLogsController, type: :request do
end
end
end
context "and there are duplicate logs for this user" do
before do
FactoryBot.create_list(:lettings_log, 2, :duplicate, owning_organisation: user.organisation, created_by: user)
end
it "displays a notification banner with a link to review logs" do
get lettings_logs_path
expect(page).to have_content "duplicate logs"
expect(page).to have_link "Review logs" # add an href when routing done
end
context "when there is one set of duplicates" do
it "displays the correct copy in the banner" do
get lettings_logs_path
expect(page).to have_content "There is 1 set of duplicate logs"
end
end
context "when there are multiple sets of duplicates" do
before do
FactoryBot.create_list(:sales_log, 2, :duplicate, owning_organisation: user.organisation, created_by: user)
end
it "displays the correct copy in the banner" do
get lettings_logs_path
expect(page).to have_content "There are 2 sets of duplicate logs"
end
end
end
end
end

Loading…
Cancel
Save