Browse Source

CLDC-2962 hide uneditable duplicate sets (#2027)

* feat: only show editable duplicate sets for user and org

* feat: simplify and refactor

* feat: update tests

* feat: add and update tests

* feat: reset not routed questions twice to catch newly not routed questions after the first set are cleared

* Revert "feat: reset not routed questions twice to catch newly not routed questions after the first set are cleared"

This reverts commit 3007284670.

* TEST FOR PO TO BE REVERTED

* TEST FOR PO TO BE REVERTED (set back to dec 31)

* TEST UPDATE TESTS

* REVERT THIS TEST UPDATE

* REVERT THIS TEST UPDATE

* feat: timecop tests

* refactor: lint

* feat: add update methods

* feat: update tests (and TO BE REVERTED: change collection year)

* feat: update tests

* feat: update tests

* feat: update form end_date tests REVERT LATER

* feat: update tests

* refactor: lint

* feat: revert to previous end dates now passed PO review

* feat: use scoping to simplify editable set logic
CLDC-2632.2-set-uprn-known-no^2
natdeanlewissoftwire 1 year ago committed by GitHub
parent
commit
118be80f8b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 8
      app/helpers/duplicate_logs_helper.rb
  2. 16
      app/models/form_handler.rb
  3. 4
      app/models/lettings_log.rb
  4. 8
      app/models/organisation.rb
  5. 4
      app/models/sales_log.rb
  6. 8
      app/models/user.rb
  7. 39
      spec/lib/tasks/blank_migrated_soctenant_values_spec.rb
  8. 11
      spec/models/lettings_log_spec.rb
  9. 32
      spec/requests/duplicate_logs_controller_spec.rb
  10. 9
      spec/requests/form_controller_spec.rb
  11. 35
      spec/requests/lettings_logs_controller_spec.rb
  12. 75
      spec/requests/sales_logs_controller_spec.rb
  13. 7
      spec/services/bulk_upload/lettings/log_creator_spec.rb
  14. 7
      spec/services/bulk_upload/lettings/validator_spec.rb
  15. 9
      spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb
  16. 8
      spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
  17. 7
      spec/services/bulk_upload/processor_spec.rb
  18. 4
      spec/services/bulk_upload/sales/year2023/row_parser_spec.rb
  19. 7
      spec/services/csv/missing_addresses_csv_service_spec.rb
  20. 7
      spec/services/imports/sales_logs_field_import_service_spec.rb

8
app/helpers/duplicate_logs_helper.rb

@ -33,15 +33,15 @@ module DuplicateLogsHelper
def duplicates_for_user(user)
{
lettings: user.duplicate_lettings_logs_sets,
sales: user.duplicate_sales_logs_sets,
lettings: user.editable_duplicate_lettings_logs_sets,
sales: user.editable_duplicate_sales_logs_sets,
}
end
def duplicates_for_organisation(organisation)
{
lettings: organisation.duplicate_lettings_logs_sets,
sales: organisation.duplicate_sales_logs_sets,
lettings: organisation.editable_duplicate_lettings_logs_sets,
sales: organisation.editable_duplicate_sales_logs_sets,
}
end

16
app/models/form_handler.rb

@ -178,6 +178,22 @@ class FormHandler
end
end
def lettings_earliest_open_for_editing_collection_start_date(now: Time.zone.now)
if lettings_in_edit_crossover_period?(now:)
collection_start_date(now) - 1.year
else
collection_start_date(now)
end
end
def sales_earliest_open_for_editing_collection_start_date(now: Time.zone.now)
if sales_in_edit_crossover_period?(now:)
collection_start_date(now) - 1.year
else
collection_start_date(now)
end
end
private
def get_all_forms

4
app/models/lettings_log.rb

@ -659,7 +659,7 @@ private
not_required << "tshortfall" if tshortfall_unknown?
not_required << "tenancylength" if tenancylength_optional?
not_required |= %w[address_line2 county postcode_full] if startdate && startdate.year >= 2023
not_required |= %w[address_line2 county postcode_full] if startdate && collection_start_year_for_date(startdate) >= 2023
not_required
end
@ -780,6 +780,6 @@ private
end
def should_process_uprn_change?
uprn && startdate && (uprn_changed? || startdate_changed?) && startdate.year >= 2023
uprn && startdate && (uprn_changed? || startdate_changed?) && collection_start_year_for_date(startdate) >= 2023
end
end

8
app/models/organisation.rb

@ -133,12 +133,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) : [] }
def editable_duplicate_lettings_logs_sets
lettings_logs.after_date(FormHandler.instance.lettings_earliest_open_for_editing_collection_start_date).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) : [] }
def editable_duplicate_sales_logs_sets
sales_logs.after_date(FormHandler.instance.sales_earliest_open_for_editing_collection_start_date).duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] }
end
def recently_absorbed_organisations_grouped_by_merge_date

4
app/models/sales_log.rb

@ -115,7 +115,7 @@ class SalesLog < Log
not_required << "mortlen" if mortlen_optional?
not_required << "frombeds" if frombeds_optional?
not_required |= %w[address_line2 county postcode_full] if saledate && saledate.year >= 2023
not_required |= %w[address_line2 county postcode_full] if saledate && collection_start_year_for_date(saledate) >= 2023
not_required
end
@ -393,7 +393,7 @@ class SalesLog < Log
end
def should_process_uprn_change?
uprn && saledate && (uprn_changed? || saledate_changed?) && saledate.year >= 2023
uprn && saledate && (uprn_changed? || saledate_changed?) && collection_start_year_for_date(saledate) >= 2023
end
def value_with_discount

8
app/models/user.rb

@ -203,12 +203,12 @@ 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) : [] }
def editable_duplicate_lettings_logs_sets
lettings_logs.after_date(FormHandler.instance.lettings_earliest_open_for_editing_collection_start_date).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) : [] }
def editable_duplicate_sales_logs_sets
sales_logs.after_date(FormHandler.instance.sales_earliest_open_for_editing_collection_start_date).duplicate_sets(id).map { |array_str| array_str ? array_str.map(&:to_i) : [] }
end
protected

39
spec/lib/tasks/blank_migrated_soctenant_values_spec.rb

@ -30,21 +30,30 @@ RSpec.describe "blank_migrated_soctenant_values" do
expect(sales_log.values_updated_at).not_to be_nil
end
it "does not blank soctenant (and subsequent questions) values from 2022 logs" do
sales_log.old_id = "404"
sales_log.frombeds = nil
sales_log.fromprop = 0 # don't know
sales_log.socprevten = 10 # don't know
sales_log.soctenant = 0 # don't know
sales_log.saledate = Time.zone.local(2022, 5, 5)
sales_log.save!
task.invoke
sales_log.reload
expect(sales_log.soctenant).to eq(0)
expect(sales_log.frombeds).to eq(nil)
expect(sales_log.fromprop).to eq(0)
expect(sales_log.socprevten).to eq(10)
expect(sales_log.values_updated_at).to be_nil
context "with 2022 logs" do
around do |example|
Timecop.freeze(Time.zone.local(2022, 5, 5)) do
Singleton.__init__(FormHandler)
example.run
end
end
it "does not blank soctenant (and subsequent questions) values" do
sales_log.old_id = "404"
sales_log.frombeds = nil
sales_log.fromprop = 0 # don't know
sales_log.socprevten = 10 # don't know
sales_log.soctenant = 0 # don't know
sales_log.saledate = Time.zone.local(2022, 5, 5)
sales_log.save!
task.invoke
sales_log.reload
expect(sales_log.soctenant).to eq(0)
expect(sales_log.frombeds).to eq(nil)
expect(sales_log.fromprop).to eq(0)
expect(sales_log.socprevten).to eq(10)
expect(sales_log.values_updated_at).to be_nil
end
end
it "does not blank soctenant (and subsequent questions) values from non imported logs" do

11
spec/models/lettings_log_spec.rb

@ -1576,7 +1576,7 @@ RSpec.describe LettingsLog do
expect { lettings_log.update!(renewal: 1) }.to change(lettings_log, :underoccupation_benefitcap).to 2
Timecop.return
Singleton.__init__(FormHandler)
expect { lettings_log.update!(startdate: Time.zone.local(2023, 1, 1)) }.to change(lettings_log, :underoccupation_benefitcap).from(2).to nil
expect { lettings_log.update!(startdate: Time.zone.local(2023, 4, 1)) }.to change(lettings_log, :underoccupation_benefitcap).from(2).to nil
end
context "when the log is general needs" do
@ -2122,10 +2122,15 @@ RSpec.describe LettingsLog do
context "and the location no local authorities associated with the location_code" do
before do
Timecop.freeze(Time.zone.local(2022, 4, 2))
location.update!(location_code: "E01231231")
lettings_log.update!(location:)
end
after do
Timecop.return
end
it "returns the correct la" do
expect(location.location_code).to eq("E01231231")
expect(lettings_log["location_id"]).to eq(location.id)
@ -2334,7 +2339,7 @@ RSpec.describe LettingsLog do
end
end
context "when saledate is before 2023" do
context "when startdate is before 2023" do
let(:lettings_log) { build(:lettings_log, startdate: Time.zone.parse("2022-07-01")) }
it "returns optional fields" do
@ -2347,7 +2352,7 @@ RSpec.describe LettingsLog do
end
end
context "when saledate is after 2023" do
context "when startdate is after 2023" do
let(:lettings_log) { build(:lettings_log, startdate: Time.zone.parse("2023-07-01")) }
it "returns optional fields" do

32
spec/requests/duplicate_logs_controller_spec.rb

@ -717,13 +717,13 @@ RSpec.describe DuplicateLogsController, type: :request do
before do
allow(Organisation).to receive(:find).with(user.organisation_id.to_s).and_return(user.organisation)
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]])
allow(user.organisation).to receive(:editable_duplicate_lettings_logs_sets).and_return([[1, 2], [3, 4, 5]])
allow(user.organisation).to receive(:editable_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)
expect(user.organisation).to receive(:editable_duplicate_lettings_logs_sets)
expect(user.organisation).to receive(:editable_duplicate_sales_logs_sets)
get organisation_duplicates_path(organisation_id: user.organisation_id)
end
@ -759,8 +759,8 @@ RSpec.describe DuplicateLogsController, type: :request do
context "when there are no duplicate logs" do
before do
allow(Organisation).to receive(:find).with(user.organisation_id.to_s).and_return(user.organisation)
allow(user.organisation).to receive(:duplicate_lettings_logs_sets).and_return([])
allow(user.organisation).to receive(:duplicate_sales_logs_sets).and_return([])
allow(user.organisation).to receive(:editable_duplicate_lettings_logs_sets).and_return([])
allow(user.organisation).to receive(:editable_duplicate_sales_logs_sets).and_return([])
get organisation_duplicates_path(organisation_id: user.organisation_id)
end
@ -780,13 +780,13 @@ RSpec.describe DuplicateLogsController, type: :request 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]])
allow(user.organisation).to receive(:editable_duplicate_lettings_logs_sets).and_return([[1, 2], [3, 4, 5]])
allow(user.organisation).to receive(:editable_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)
expect(user.organisation).to receive(:editable_duplicate_lettings_logs_sets)
expect(user.organisation).to receive(:editable_duplicate_sales_logs_sets)
get duplicate_logs_path(organisation_id: user.organisation.id)
end
end
@ -795,13 +795,13 @@ RSpec.describe DuplicateLogsController, type: :request 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]])
allow(user).to receive(:editable_duplicate_lettings_logs_sets).and_return([[1, 2], [3, 4, 5]])
allow(user).to receive(:editable_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)
expect(user).to receive(:editable_duplicate_lettings_logs_sets)
expect(user).to receive(:editable_duplicate_sales_logs_sets)
get duplicate_logs_path
end
@ -836,8 +836,8 @@ RSpec.describe DuplicateLogsController, type: :request do
context "when there are no duplicate logs" do
before do
allow(user).to receive(:duplicate_lettings_logs_sets).and_return([])
allow(user).to receive(:duplicate_sales_logs_sets).and_return([])
allow(user).to receive(:editable_duplicate_lettings_logs_sets).and_return([])
allow(user).to receive(:editable_duplicate_sales_logs_sets).and_return([])
get duplicate_logs_path
end

9
spec/requests/form_controller_spec.rb

@ -254,6 +254,15 @@ RSpec.describe FormController, type: :request do
end
describe "GET" do
around do |example|
Timecop.freeze(Time.zone.local(2022, 5, 1)) do
Singleton.__init__(FormHandler)
example.run
end
Timecop.return
Singleton.__init__(FormHandler)
end
context "with form pages" do
context "when forms exist" do
let(:lettings_log) { create(:lettings_log, :setup_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation, created_by: user) }

35
spec/requests/lettings_logs_controller_spec.rb

@ -889,14 +889,12 @@ RSpec.describe LettingsLogsController, type: :request do
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
let!(:duplicate_logs) { FactoryBot.create_list(:lettings_log, 2, :duplicate, owning_organisation: user.organisation, created_by: user) }
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
expect(page).to have_link "Review logs", href: "/duplicate-logs?referrer=duplicate_logs_banner"
end
context "when there is one set of duplicates" do
@ -904,6 +902,20 @@ RSpec.describe LettingsLogsController, type: :request do
get lettings_logs_path
expect(page).to have_content "There is 1 set of duplicate logs"
end
context "when the set is not editable" do
before do
duplicate_logs.each do |log|
log.startdate = Time.zone.now - 3.years
log.save!(validate: false)
end
end
it "does not display the banner" do
get lettings_logs_path
expect(page).not_to have_content "duplicate logs"
end
end
end
context "when there are multiple sets of duplicates" do
@ -914,6 +926,21 @@ RSpec.describe LettingsLogsController, type: :request do
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"
expect(page).to have_link "Review logs", href: "/duplicate-logs?referrer=duplicate_logs_banner"
end
context "when one set is not editable" do
before do
log = duplicate_logs.first
log.startdate = Time.zone.now - 3.years
log.save!(validate: false)
end
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"
expect(page).to have_link "Review logs", href: "/duplicate-logs?referrer=duplicate_logs_banner"
end
end
end
end

75
spec/requests/sales_logs_controller_spec.rb

@ -235,6 +235,18 @@ RSpec.describe SalesLogsController, type: :request do
expect(page).to have_link("Download (CSV, codes only)", href: "/sales-logs/csv-download?codes_only=true")
end
context "when there are duplicate logs for this user" do
before do
FactoryBot.create_list(:sales_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 sales_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
SalesLog.destroy_all
@ -566,6 +578,12 @@ RSpec.describe SalesLogsController, 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 sales_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(:sales_log, 3, :completed, owning_organisation: user.organisation, created_by: user) }
let(:log_to_search) { FactoryBot.create(:sales_log, :completed, owning_organisation: user.organisation, created_by: user) }
@ -727,6 +745,63 @@ RSpec.describe SalesLogsController, type: :request do
end
end
end
context "and there are duplicate logs for this user" do
let!(:duplicate_logs) { FactoryBot.create_list(:lettings_log, 2, :duplicate, owning_organisation: user.organisation, created_by: user) }
it "displays a notification banner with a link to review logs" do
get sales_logs_path
expect(page).to have_content "duplicate logs"
expect(page).to have_link "Review logs", href: "/duplicate-logs?referrer=duplicate_logs_banner"
end
context "when there is one set of duplicates" do
it "displays the correct copy in the banner" do
get sales_logs_path
expect(page).to have_content "There is 1 set of duplicate logs"
end
context "when the set is not editable" do
before do
duplicate_logs.each do |log|
log.startdate = Time.zone.now - 3.years
log.save!(validate: false)
end
end
it "does not display the banner" do
get sales_logs_path
expect(page).not_to have_content "duplicate logs"
end
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 sales_logs_path
expect(page).to have_content "There are 2 sets of duplicate logs"
expect(page).to have_link "Review logs", href: "/duplicate-logs?referrer=duplicate_logs_banner"
end
context "when one set is not editable" do
before do
log = duplicate_logs.first
log.startdate = Time.zone.now - 3.years
log.save!(validate: false)
end
it "displays the correct copy in the banner" do
get sales_logs_path
expect(page).to have_content "There is 1 set of duplicate logs"
expect(page).to have_link "Review logs", href: "/duplicate-logs?referrer=duplicate_logs_banner"
end
end
end
end
end
end

7
spec/services/bulk_upload/lettings/log_creator_spec.rb

@ -9,6 +9,13 @@ RSpec.describe BulkUpload::Lettings::LogCreator do
let(:bulk_upload) { create(:bulk_upload, :lettings, user:) }
let(:path) { file_fixture("2022_23_lettings_bulk_upload.csv") }
around do |example|
Timecop.freeze(Time.zone.local(2023, 1, 1)) do
Singleton.__init__(FormHandler)
example.run
end
end
describe "#call" do
context "when a valid csv with new log" do
it "creates a new log" do

7
spec/services/bulk_upload/lettings/validator_spec.rb

@ -10,6 +10,13 @@ RSpec.describe BulkUpload::Lettings::Validator do
let(:path) { file.path }
let(:file) { Tempfile.new }
around do |example|
Timecop.freeze(Date.new(2023, 10, 1)) do
example.run
end
Timecop.return
end
describe "validations" do
context "when 2022" do
let(:bulk_upload) { create(:bulk_upload, user:, year: 2022) }

9
spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb

@ -153,11 +153,14 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
end
around do |example|
FormHandler.instance.use_real_forms!
Timecop.freeze(Date.new(2023, 3, 1)) do
FormHandler.instance.use_real_forms!
example.run
example.run
FormHandler.instance.use_fake_forms!
FormHandler.instance.use_fake_forms!
end
Timecop.return
end
describe "#blank_row?" do

8
spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb

@ -47,9 +47,11 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
end
around do |example|
FormHandler.instance.use_real_forms!
example.run
Timecop.freeze(Date.new(2023, 10, 1)) do
FormHandler.instance.use_real_forms!
example.run
end
Timecop.return
end
describe "#blank_row?" do

7
spec/services/bulk_upload/processor_spec.rb

@ -7,6 +7,13 @@ RSpec.describe BulkUpload::Processor do
let(:user) { create(:user, organisation: owning_org) }
let(:owning_org) { create(:organisation, old_visible_id: 123) }
around do |example|
Timecop.freeze(Time.utc(2023, 1, 1)) do
Singleton.__init__(FormHandler)
example.run
end
end
describe "#call" do
context "when errors exist from prior job run" do
let!(:existing_error) { create(:bulk_upload_error, bulk_upload:) }

4
spec/services/bulk_upload/sales/year2023/row_parser_spec.rb

@ -3,7 +3,7 @@ require "rails_helper"
RSpec.describe BulkUpload::Sales::Year2023::RowParser do
subject(:parser) { described_class.new(attributes) }
let(:now) { Time.zone.parse("01/03/2023") }
let(:now) { Time.zone.parse("01/05/2023") }
let(:attributes) { { bulk_upload: } }
let(:bulk_upload) { create(:bulk_upload, :sales, user:, year: 2023) }
@ -760,7 +760,7 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
let(:attributes) { setup_section_params.merge({ field_19: "3", data[:field] => nil }) }
it "cannot be blank" do
expect(parser.errors[data[:field]]).not_to be_blank
expect(parser.errors[data[:field]]).to be_present
end
end
end

7
spec/services/csv/missing_addresses_csv_service_spec.rb

@ -38,6 +38,13 @@ RSpec.describe Csv::MissingAddressesCsvService do
.to_return(status: 200, body: body_2, headers: {})
end
around do |example|
Timecop.freeze(Time.zone.local(2023, 4, 5)) do
Singleton.__init__(FormHandler)
example.run
end
end
def replace_entity_ids(lettings_log, export_template)
export_template.sub!(/\{id\}/, lettings_log.id.to_s)
end

7
spec/services/imports/sales_logs_field_import_service_spec.rb

@ -27,6 +27,13 @@ RSpec.describe Imports::SalesLogsFieldImportService do
.and_return(sales_log_file)
end
around do |example|
Timecop.freeze(Time.zone.local(2023, 1, 17)) do
Singleton.__init__(FormHandler)
example.run
end
end
context "when updating creation method" do
let(:field) { "creation_method" }
let(:sales_log) { SalesLog.find_by(old_id: sales_log_filename) }

Loading…
Cancel
Save