From 0d114031220dccd9c3a61d07c026c9be57c028da Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
<94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Thu, 11 May 2023 16:56:34 +0100
Subject: [PATCH 1/4] CLDC-2315 Add bulk upload inferences (#1592)
* feat: bulk upload complete => redirect to all logs regardless of filter memory
* feat: derive variables if buyer not interviewed
* feat: remove redundant derivation that is overrided by other derivation
* feat: update url for lettings
* tests: tests inferences
* tests: update
* feat: update links
* feat: update urls to use url helper
* feat: update urls to use url helper
* feat: update logic
* test: update
* test: update
* test: update
* feat: infer ecstat1 as 10 not 0
* feat: update tests
---
app/mailers/bulk_upload_mailer.rb | 4 +-
.../bulk_upload/sales/year2022/row_parser.rb | 72 ++++++++++++-------
spec/mailers/bulk_upload_mailer_spec.rb | 2 +-
.../sales/year2022/row_parser_spec.rb | 42 +++++++++++
4 files changed, 92 insertions(+), 28 deletions(-)
diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb
index 883e89fed..c6e59b15e 100644
--- a/app/mailers/bulk_upload_mailer.rb
+++ b/app/mailers/bulk_upload_mailer.rb
@@ -27,9 +27,9 @@ class BulkUploadMailer < NotifyMailer
def send_bulk_upload_complete_mail(user:, bulk_upload:)
url = if bulk_upload.lettings?
- lettings_logs_url
+ lettings_logs_url("[years][]" => "", "[status][]" => "", user: :all, organisation_select: :all)
else
- sales_logs_url
+ sales_logs_url("[years][]" => "", "[status][]" => "", user: :all)
end
n_logs = pluralize(bulk_upload.logs.count, "log")
diff --git a/app/services/bulk_upload/sales/year2022/row_parser.rb b/app/services/bulk_upload/sales/year2022/row_parser.rb
index 9cc2f1f02..851ed4bfd 100644
--- a/app/services/bulk_upload/sales/year2022/row_parser.rb
+++ b/app/services/bulk_upload/sales/year2022/row_parser.rb
@@ -339,6 +339,10 @@ class BulkUpload::Sales::Year2022::RowParser
private
+ def buyer_not_interviewed?
+ field_6 == 1
+ end
+
def shared_ownership?
field_113 == 1
end
@@ -478,7 +482,7 @@ private
attributes["purchid"] = field_1
attributes["saledate"] = saledate
- attributes["noint"] = 2 if field_6 == 1
+ attributes["noint"] = field_6
attributes["details_known_2"] = details_known?(2)
attributes["details_known_3"] = details_known?(3)
@@ -517,7 +521,7 @@ private
attributes["relat5"] = field_22
attributes["relat6"] = field_23
- attributes["ecstat1"] = field_24
+ attributes["ecstat1"] = buyer_not_interviewed? && field_24.blank? ? 10 : field_24
attributes["ecstat2"] = field_25
attributes["ecstat3"] = field_26
attributes["ecstat4"] = field_27
@@ -526,18 +530,18 @@ private
attributes["ethnic_group"] = ethnic_group_from_ethnic
attributes["ethnic"] = field_30
- attributes["national"] = field_31
- attributes["income1nk"] = field_32.present? ? 0 : 1
+ attributes["national"] = buyer_not_interviewed? && field_31.blank? ? 13 : field_31
+ attributes["income1nk"] = income1nk
attributes["income1"] = field_32
attributes["income2nk"] = field_33.present? ? 0 : 1
attributes["income2"] = field_33
- attributes["inc1mort"] = field_34
+ attributes["inc1mort"] = buyer_not_interviewed? && field_32.blank? ? 2 : field_34
attributes["inc2mort"] = field_35
- attributes["savingsnk"] = field_36.present? ? 0 : 1
+ attributes["savingsnk"] = savingsnk
attributes["savings"] = field_36
- attributes["prevown"] = field_37
+ attributes["prevown"] = buyer_not_interviewed? && field_37.blank? ? 3 : field_37
- attributes["prevten"] = field_39
+ attributes["prevten"] = buyer_not_interviewed? && field_39.blank? ? 0 : field_39
attributes["prevloc"] = field_40
attributes["previous_la_known"] = previous_la_known
attributes["ppcodenk"] = field_43
@@ -549,8 +553,8 @@ private
attributes["pregother"] = field_47
attributes["pregblank"] = 1 if [field_44, field_45, field_46, field_47].all?(&:blank?)
- attributes["disabled"] = field_48
- attributes["wheel"] = field_49
+ attributes["disabled"] = buyer_not_interviewed? && field_48.blank? ? 3 : field_48
+ attributes["wheel"] = buyer_not_interviewed? && field_49.blank? ? 3 : field_49
attributes["beds"] = field_50
attributes["proptype"] = field_51
attributes["builtype"] = field_52
@@ -620,6 +624,22 @@ private
attributes
end
+ def income1nk
+ if field_32.present?
+ 0
+ else
+ buyer_not_interviewed? ? 1 : nil
+ end
+ end
+
+ def savingsnk
+ if field_36.present?
+ 0
+ else
+ buyer_not_interviewed? ? 1 : nil
+ end
+ end
+
def saledate
Date.new(field_4 + 2000, field_3, field_2) if field_2.present? && field_3.present? && field_4.present?
rescue Date::Error
@@ -686,21 +706,23 @@ private
end
def ethnic_group_from_ethnic
- return nil if field_30.blank?
-
- case field_30
- when 1, 2, 3, 18
- 0
- when 4, 5, 6, 7
- 1
- when 8, 9, 10, 11, 15
- 2
- when 12, 13, 14
- 3
- when 16, 19
- 4
- when 17
- 17
+ if field_30.blank?
+ buyer_not_interviewed? ? 17 : nil
+ else
+ case field_30
+ when 1, 2, 3, 18
+ 0
+ when 4, 5, 6, 7
+ 1
+ when 8, 9, 10, 11, 15
+ 2
+ when 12, 13, 14
+ 3
+ when 16, 19
+ 4
+ when 17
+ 17
+ end
end
end
diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb
index 20f78de4d..e2e32ab96 100644
--- a/spec/mailers/bulk_upload_mailer_spec.rb
+++ b/spec/mailers/bulk_upload_mailer_spec.rb
@@ -46,7 +46,7 @@ RSpec.describe BulkUploadMailer do
filename: bulk_upload.filename,
upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
success_description: "The lettings 2022/23 data you uploaded has been checked. The 0 logs you uploaded are now complete.",
- logs_link: lettings_logs_url,
+ logs_link: lettings_logs_url("[years][]" => "", "[status][]" => "", user: :all, organisation_select: :all),
},
)
diff --git a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb
index 7fdf8da4d..4d37280dc 100644
--- a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb
+++ b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb
@@ -540,4 +540,46 @@ RSpec.describe BulkUpload::Sales::Year2022::RowParser do
end
end
end
+
+ describe "inferences" do
+ context "when buyer not interviewed and optional values nil" do
+ let(:attributes) { valid_attributes.merge(field_6: "1", field_24: nil, field_30: nil, field_31: nil, field_32: nil, field_34: nil, field_36: nil, field_37: nil, field_39: nil, field_48: nil, field_49: nil) }
+
+ it "infers correctly" do
+ log = parser.log
+ expect(log["noint"]).to eq(1)
+ expect(log["ecstat1"]).to eq(10)
+ expect(log["ethnic"]).to eq(nil)
+ expect(log["ethnic_group"]).to eq(17)
+ expect(log["national"]).to eq(13)
+ expect(log["income1nk"]).to eq(1)
+ expect(log["inc1mort"]).to eq(2)
+ expect(log["savingsnk"]).to eq(1)
+ expect(log["prevown"]).to eq(3)
+ expect(log["prevten"]).to eq(0)
+ expect(log["disabled"]).to eq(3)
+ expect(log["wheel"]).to eq(3)
+ end
+ end
+
+ context "when buyer not interviewed and optional values present" do
+ let(:attributes) { valid_attributes.merge(field_6: "1", field_24: "1", field_30: "1", field_31: "1", field_32: "1", field_34: "1", field_36: "1", field_37: "1", field_39: "1", field_48: "1", field_49: "1") }
+
+ it "does not override variables correctly" do
+ log = parser.log
+ expect(log["noint"]).to eq(1)
+ expect(log["ecstat1"]).to eq(1)
+ expect(log["ethnic"]).to eq(1)
+ expect(log["ethnic_group"]).to eq(0)
+ expect(log["national"]).to eq(1)
+ expect(log["income1nk"]).to eq(0)
+ expect(log["inc1mort"]).to eq(1)
+ expect(log["savingsnk"]).to eq(0)
+ expect(log["prevown"]).to eq(1)
+ expect(log["prevten"]).to eq(1)
+ expect(log["disabled"]).to eq(1)
+ expect(log["wheel"]).to eq(1)
+ end
+ end
+ end
end
From 9a4c912bf97e916a874a451f84001f6fcd851d39 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Fri, 12 May 2023 10:50:30 +0100
Subject: [PATCH 2/4] Update routing and prevten inferrance (#1597)
---
app/models/derived_variables/lettings_log_variables.rb | 4 ++--
app/models/form/lettings/pages/referral.rb | 2 +-
app/models/form/lettings/pages/referral_prp.rb | 2 +-
.../form/lettings/pages/referral_supported_housing.rb | 2 +-
.../form/lettings/pages/referral_supported_housing_prp.rb | 2 +-
app/models/lettings_log.rb | 4 ++--
config/forms/2022_2023.json | 8 ++++----
7 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/app/models/derived_variables/lettings_log_variables.rb b/app/models/derived_variables/lettings_log_variables.rb
index 937dd7c2e..3a7350a8a 100644
--- a/app/models/derived_variables/lettings_log_variables.rb
+++ b/app/models/derived_variables/lettings_log_variables.rb
@@ -76,8 +76,8 @@ module DerivedVariables::LettingsLogVariables
self.voiddate = startdate
self.unitletas = form.start_date.year >= 2023 ? UNITLETAS_MAPPING_23_24[rent_type] : UNITLETAS_MAPPING[rent_type]
if is_general_needs?
- self.prevten = 32 if managing_organisation&.provider_type == "PRP"
- self.prevten = 30 if managing_organisation&.provider_type == "LA"
+ self.prevten = 32 if owning_organisation&.provider_type == "PRP"
+ self.prevten = 30 if owning_organisation&.provider_type == "LA"
end
end
diff --git a/app/models/form/lettings/pages/referral.rb b/app/models/form/lettings/pages/referral.rb
index 3850fd400..9bda8c77e 100644
--- a/app/models/form/lettings/pages/referral.rb
+++ b/app/models/form/lettings/pages/referral.rb
@@ -2,7 +2,7 @@ class Form::Lettings::Pages::Referral < ::Form::Page
def initialize(id, hsh, subsection)
super
@id = "referral"
- @depends_on = [{ "managing_organisation_provider_type" => "LA", "needstype" => 1, "renewal" => 0 }]
+ @depends_on = [{ "owning_organisation_provider_type" => "LA", "needstype" => 1, "renewal" => 0 }]
end
def questions
diff --git a/app/models/form/lettings/pages/referral_prp.rb b/app/models/form/lettings/pages/referral_prp.rb
index b1f650d8e..d9c37b6d6 100644
--- a/app/models/form/lettings/pages/referral_prp.rb
+++ b/app/models/form/lettings/pages/referral_prp.rb
@@ -2,7 +2,7 @@ class Form::Lettings::Pages::ReferralPrp < ::Form::Page
def initialize(id, hsh, subsection)
super
@id = "referral_prp"
- @depends_on = [{ "managing_organisation_provider_type" => "PRP", "needstype" => 1, "renewal" => 0 }]
+ @depends_on = [{ "owning_organisation_provider_type" => "PRP", "needstype" => 1, "renewal" => 0 }]
end
def questions
diff --git a/app/models/form/lettings/pages/referral_supported_housing.rb b/app/models/form/lettings/pages/referral_supported_housing.rb
index 80d69b8cd..8464ae366 100644
--- a/app/models/form/lettings/pages/referral_supported_housing.rb
+++ b/app/models/form/lettings/pages/referral_supported_housing.rb
@@ -2,7 +2,7 @@ class Form::Lettings::Pages::ReferralSupportedHousing < ::Form::Page
def initialize(id, hsh, subsection)
super
@id = "referral_supported_housing"
- @depends_on = [{ "managing_organisation_provider_type" => "LA", "needstype" => 2, "renewal" => 0 }]
+ @depends_on = [{ "owning_organisation_provider_type" => "LA", "needstype" => 2, "renewal" => 0 }]
end
def questions
diff --git a/app/models/form/lettings/pages/referral_supported_housing_prp.rb b/app/models/form/lettings/pages/referral_supported_housing_prp.rb
index 244f85fcd..289ae5eb6 100644
--- a/app/models/form/lettings/pages/referral_supported_housing_prp.rb
+++ b/app/models/form/lettings/pages/referral_supported_housing_prp.rb
@@ -2,7 +2,7 @@ class Form::Lettings::Pages::ReferralSupportedHousingPrp < ::Form::Page
def initialize(id, hsh, subsection)
super
@id = "referral_supported_housing_prp"
- @depends_on = [{ "managing_organisation_provider_type" => "PRP", "needstype" => 2, "renewal" => 0 }]
+ @depends_on = [{ "owning_organisation_provider_type" => "PRP", "needstype" => 2, "renewal" => 0 }]
end
def questions
diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb
index 6fce43f42..922df5977 100644
--- a/app/models/lettings_log.rb
+++ b/app/models/lettings_log.rb
@@ -493,8 +493,8 @@ class LettingsLog < Log
update(unresolved: false)
end
- def managing_organisation_provider_type
- managing_organisation&.provider_type
+ def owning_organisation_provider_type
+ owning_organisation&.provider_type
end
def reset_created_by!
diff --git a/config/forms/2022_2023.json b/config/forms/2022_2023.json
index 5a65e6f0e..161101e4f 100644
--- a/config/forms/2022_2023.json
+++ b/config/forms/2022_2023.json
@@ -7010,7 +7010,7 @@
},
"depends_on": [
{
- "managing_organisation_provider_type": "LA",
+ "owning_organisation_provider_type": "LA",
"needstype": 1,
"renewal": 0
}
@@ -7067,7 +7067,7 @@
},
"depends_on": [
{
- "managing_organisation_provider_type": "PRP",
+ "owning_organisation_provider_type": "PRP",
"needstype": 1,
"renewal": 0
}
@@ -7124,7 +7124,7 @@
},
"depends_on": [
{
- "managing_organisation_provider_type": "LA",
+ "owning_organisation_provider_type": "LA",
"needstype": 2,
"renewal": 0
}
@@ -7184,7 +7184,7 @@
},
"depends_on": [
{
- "managing_organisation_provider_type": "PRP",
+ "owning_organisation_provider_type": "PRP",
"needstype": 2,
"renewal": 0
}
From 6973ade8c42cd6ab98c53f1fcb492acc57b9eec3 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
<94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Fri, 12 May 2023 11:03:41 +0100
Subject: [PATCH 3/4] CLDC-2262 Bulk upload clear invalid data to create forms
(#1618)
* feat: wip blank fields and dependent fields on upload tos ee if valid and can upload with missing info - this is not the exact ac on the ticket yet
* feat: update seed
* feat: wip commit
* feat: add postcodenk error so can clear on validation
* feat: add postcode validation back
* feat: move la vals to shared and move and add tests
* feat: add correct pluralisation to warning message
* feat: add blank compound invalid fields methods
* feat: update validations
* feat: update pluralisation
* refactor: lint
* feat: clear errors associated with blanked values so log status is set correctly on creation
* feat: validate instead
* feat: avoid duplicated errors
* feat: dont auto-refuse income, different to imports
* feat: validate after every blank method
* feat: delete la validator spec
* tests: update
* refactor: erblinting
* refactor: cleanup
* refactor: move pluralizer to helper
* feat: copy update
* feat: rename
* feat: refactor to avoid redundant re-validations and test
* refactor: rubocop
* test: update
* test: update
* test: update
* update sidekiq
* feat: clear errors
* feat: run clearing twice in case first clear creates different errors
* feat: remove moved file
* feat: undo validation file tweaks as shared/specific overlap could do with a more general refactor
* feat: update tests
---
app/helpers/logs_helper.rb | 6 ++++
app/models/lettings_log.rb | 10 ++++++
app/models/log.rb | 28 +++++++++++++--
app/models/sales_log.rb | 7 +++-
.../validations/financial_validations.rb | 3 +-
.../bulk_upload/lettings/validator.rb | 5 +++
app/services/bulk_upload/processor.rb | 5 ++-
app/services/bulk_upload/sales/validator.rb | 12 ++++++-
.../confirm.html.erb | 2 +-
config/locales/en.yml | 3 +-
.../validations/financial_validations_spec.rb | 2 +-
.../bulk_upload/lettings/log_creator_spec.rb | 30 ++++++++++++++++
.../bulk_upload/sales/log_creator_spec.rb | 35 +++++++++++++++++++
13 files changed, 136 insertions(+), 12 deletions(-)
diff --git a/app/helpers/logs_helper.rb b/app/helpers/logs_helper.rb
index f525f4600..ca6fa838f 100644
--- a/app/helpers/logs_helper.rb
+++ b/app/helpers/logs_helper.rb
@@ -47,4 +47,10 @@ module LogsHelper
when :sales then sales_logs_csv_download_organisation_path(organisation, search:, codes_only:)
end
end
+
+ def pluralize_logs_and_errors_warning(log_count, error_count)
+ is_or_are = log_count == 1 ? "is" : "are"
+ need_or_needs = error_count == 1 ? "needs" : "need"
+ "There #{is_or_are} #{pluralize(log_count, 'log')} in this bulk upload with #{pluralize(error_count, 'error')} that still #{need_or_needs} to be fixed after upload."
+ end
end
diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb
index 922df5977..c1a3e4a16 100644
--- a/app/models/lettings_log.rb
+++ b/app/models/lettings_log.rb
@@ -529,6 +529,16 @@ class LettingsLog < Log
is_carehome == 1
end
+ def blank_compound_invalid_non_setup_fields!
+ super
+
+ self.postcode_known = nil if errors.attribute_names.include? :postcode_full
+
+ if errors.of_kind?(:earnings, :under_hard_min)
+ self.incfreq = nil
+ end
+ end
+
private
def reset_invalid_unresolved_log_fields!
diff --git a/app/models/log.rb b/app/models/log.rb
index 807b45c8b..2c6c0254a 100644
--- a/app/models/log.rb
+++ b/app/models/log.rb
@@ -98,10 +98,18 @@ class Log < ApplicationRecord
def blank_invalid_non_setup_fields!
setup_ids = form.setup_sections.flat_map(&:subsections).flat_map(&:questions).map(&:id)
- errors.each do |error|
- next if setup_ids.include?(error.attribute.to_s)
+ 2.times do
+ next if valid?
- public_send("#{error.attribute}=", nil)
+ errors.each do |error|
+ next if setup_ids.include?(error.attribute.to_s)
+
+ public_send("#{error.attribute}=", nil)
+ end
+
+ blank_compound_invalid_non_setup_fields!
+
+ errors.clear
end
end
@@ -136,6 +144,20 @@ class Log < ApplicationRecord
format_as_currency(field_value)
end
+ def blank_compound_invalid_non_setup_fields!
+ self.ppcodenk = nil if errors.attribute_names.include? :ppostcode_full
+
+ if errors.of_kind?(:uprn, :uprn_error)
+ self.uprn_known = nil
+ self.uprn_confirmed = nil
+ self.address_line1 = nil
+ self.address_line2 = nil
+ self.town_or_city = nil
+ self.postcode_full = nil
+ self.county = nil
+ end
+ end
+
def creation_method
bulk_upload_id ? "bulk upload" : "single log"
end
diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb
index 41b7bd5b1..8002ac815 100644
--- a/app/models/sales_log.rb
+++ b/app/models/sales_log.rb
@@ -2,7 +2,6 @@ class SalesLogValidator < ActiveModel::Validator
include Validations::Sales::SetupValidations
include Validations::Sales::HouseholdValidations
include Validations::Sales::PropertyValidations
- include Validations::SharedValidations
include Validations::Sales::FinancialValidations
include Validations::Sales::SaleInformationValidations
include Validations::SharedValidations
@@ -386,4 +385,10 @@ class SalesLog < Log
when 3 then "outright sale"
end
end
+
+ def blank_compound_invalid_non_setup_fields!
+ super
+
+ self.pcodenk = nil if errors.attribute_names.include? :postcode_full
+ end
end
diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb
index 0b8950ce2..7ec730b97 100644
--- a/app/models/validations/financial_validations.rb
+++ b/app/models/validations/financial_validations.rb
@@ -5,7 +5,8 @@ module Validations::FinancialValidations
# or 'validate_' to run on submit as well
def validate_outstanding_rent_amount(record)
if !record.has_housing_benefit_rent_shortfall? && record.tshortfall.present?
- record.errors.add :tshortfall, :no_outstanding_charges, message: I18n.t("validations.financial.tshortfall.outstanding_amount_not_required")
+ record.errors.add :tshortfall, :no_outstanding_charges, message: I18n.t("validations.financial.tshortfall.outstanding_amount_not_expected")
+ record.errors.add :hbrentshortfall, :no_outstanding_charges, message: I18n.t("validations.financial.hbrentshortfall.outstanding_amount_not_expected")
end
end
diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb
index ae272a0fe..d56919bb4 100644
--- a/app/services/bulk_upload/lettings/validator.rb
+++ b/app/services/bulk_upload/lettings/validator.rb
@@ -44,6 +44,11 @@ class BulkUpload::Lettings::Validator
return false if any_setup_errors?
return false if row_parsers.any?(&:block_log_creation?)
return false if any_logs_already_exist? && FeatureToggle.bulk_upload_duplicate_log_check_enabled?
+
+ row_parsers.each do |row_parser|
+ row_parser.log.blank_invalid_non_setup_fields!
+ end
+
return false if any_logs_invalid?
true
diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb
index 49d44261f..66c6b9d10 100644
--- a/app/services/bulk_upload/processor.rb
+++ b/app/services/bulk_upload/processor.rb
@@ -14,14 +14,13 @@ class BulkUpload::Processor
if validator.any_setup_errors?
send_setup_errors_mail
+
elsif validator.create_logs?
create_logs
if created_logs_but_incompleted?
send_how_fix_upload_mail
- end
-
- if created_logs_and_all_completed?
+ elsif created_logs_and_all_completed?
bulk_upload.unpend
send_success_mail
end
diff --git a/app/services/bulk_upload/sales/validator.rb b/app/services/bulk_upload/sales/validator.rb
index ee87530ca..ede32266b 100644
--- a/app/services/bulk_upload/sales/validator.rb
+++ b/app/services/bulk_upload/sales/validator.rb
@@ -38,7 +38,13 @@ class BulkUpload::Sales::Validator
return false if any_setup_errors?
return false if row_parsers.any?(&:block_log_creation?)
- row_parsers.all? { |row_parser| row_parser.log.valid? }
+ row_parsers.each do |row_parser|
+ row_parser.log.blank_invalid_non_setup_fields!
+ end
+
+ return false if any_logs_invalid?
+
+ true
end
def any_setup_errors?
@@ -51,6 +57,10 @@ class BulkUpload::Sales::Validator
private
+ def any_logs_invalid?
+ row_parsers.any? { |row_parser| row_parser.log.invalid? }
+ end
+
def csv_parser
@csv_parser ||= case bulk_upload.year
when 2022
diff --git a/app/views/bulk_upload_lettings_resume/confirm.html.erb b/app/views/bulk_upload_lettings_resume/confirm.html.erb
index af73b33e9..a61a73651 100644
--- a/app/views/bulk_upload_lettings_resume/confirm.html.erb
+++ b/app/views/bulk_upload_lettings_resume/confirm.html.erb
@@ -7,7 +7,7 @@
Bulk upload for lettings (<%= @bulk_upload.year_combo %>)
Are you sure you want to upload all logs from this bulk upload?
- There are <%= pluralize(@bulk_upload.logs.count, "log") %> in this bulk upload with <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> that still need to be fixed after upload.
+ <%= pluralize_logs_and_errors_warning(@bulk_upload.logs.count, @bulk_upload.bulk_upload_errors.count) %>
<%= govuk_warning_text(icon_fallback_text: "Danger") do %>
You can not delete logs once you create them
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 1ac1fb6c3..8e7eb6e14 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -256,10 +256,11 @@ en:
financial:
tshortfall:
- outstanding_amount_not_required: "You cannot answer the outstanding amount question if you don’t have outstanding rent or charges"
+ outstanding_amount_not_expected: "You cannot answer the outstanding amount question if you don’t have outstanding rent or charges"
more_than_rent: "Enter a value less less than the basic rent amount"
must_be_positive: "Enter a value over £0.01 as you told us there is an outstanding amount"
hbrentshortfall:
+ outstanding_amount_not_expected: "Answer must be ‘yes’ as you have answered the outstanding amount question"
outstanding_no_benefits: "Answer cannot be ‘yes’ to outstanding amount for basic rent or charges if tenant does not receive housing benefit or Universal Credit or you‘re not sure"
benefits:
part_or_full_time: "Answer cannot be ‘all’ for income from Universal Credit, state pensions or benefits if the tenant or their partner works part-time or full-time"
diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb
index 4bac948d0..02a2f752f 100644
--- a/spec/models/validations/financial_validations_spec.rb
+++ b/spec/models/validations/financial_validations_spec.rb
@@ -81,7 +81,7 @@ RSpec.describe Validations::FinancialValidations do
record.tshortfall = 99
financial_validator.validate_outstanding_rent_amount(record)
expect(record.errors["tshortfall"])
- .to include(match I18n.t("validations.financial.tshortfall.outstanding_amount_not_required"))
+ .to include(match I18n.t("validations.financial.tshortfall.outstanding_amount_not_expected"))
end
end
diff --git a/spec/services/bulk_upload/lettings/log_creator_spec.rb b/spec/services/bulk_upload/lettings/log_creator_spec.rb
index adf4d3845..9c28cc81e 100644
--- a/spec/services/bulk_upload/lettings/log_creator_spec.rb
+++ b/spec/services/bulk_upload/lettings/log_creator_spec.rb
@@ -82,6 +82,36 @@ RSpec.describe BulkUpload::Lettings::LogCreator do
end
end
+ context "when a valid csv with row with compound errors on non setup field" do
+ let(:file) { Tempfile.new }
+ let(:path) { file.path }
+ let(:log) do
+ build(
+ :lettings_log,
+ :completed,
+ earnings: 0,
+ incfreq: 1,
+ )
+ end
+
+ before do
+ file.write(BulkUpload::LettingsLogToCsv.new(log:, col_offset: 0).to_2023_csv_row)
+ file.rewind
+ end
+
+ it "creates the log" do
+ expect { service.call }.to change(LettingsLog, :count).by(1)
+ end
+
+ it "blanks invalid fields" do
+ service.call
+
+ record = LettingsLog.last
+ expect(record.earnings).to be_blank
+ expect(record.incfreq).to be_blank
+ end
+ end
+
context "when pre-creating logs" do
subject(:service) { described_class.new(bulk_upload:, path:) }
diff --git a/spec/services/bulk_upload/sales/log_creator_spec.rb b/spec/services/bulk_upload/sales/log_creator_spec.rb
index 440fcca16..2abda527d 100644
--- a/spec/services/bulk_upload/sales/log_creator_spec.rb
+++ b/spec/services/bulk_upload/sales/log_creator_spec.rb
@@ -75,6 +75,41 @@ RSpec.describe BulkUpload::Sales::LogCreator do
end
end
+ context "when a valid csv with row with compound errors on non setup field" do
+ let(:file) { Tempfile.new }
+ let(:path) { file.path }
+ let(:log) do
+ build(
+ :sales_log,
+ :completed,
+ ownershipsch: 2,
+ pcodenk: 0,
+ ppcodenk: 0,
+ postcode_full: "AA11AA",
+ ppostcode_full: "BB22BB",
+ )
+ end
+
+ before do
+ file.write(BulkUpload::SalesLogToCsv.new(log:, col_offset: 0).to_2022_csv_row)
+ file.rewind
+ end
+
+ it "creates the log" do
+ expect { service.call }.to change(SalesLog, :count).by(1)
+ end
+
+ it "blanks invalid field" do
+ service.call
+
+ record = SalesLog.last
+ expect(record.pcodenk).to be_blank
+ expect(record.postcode_full).to be_blank
+ expect(record.ppcodenk).to be_blank
+ expect(record.ppostcode_full).to be_blank
+ end
+ end
+
context "when pre-creating logs" do
subject(:service) { described_class.new(bulk_upload:, path:) }
From 9ab2d2054cf6cb116e5fd11bd4d53feddd1355e8 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
<94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Fri, 12 May 2023 11:03:54 +0100
Subject: [PATCH 4/4] CLDC-2082 Add merge orgs new phone number page (#1619)
* feat: add new telephone page with failing error tests
* feat: update tests
* refactor: lint
* feat: remove skip link
---
app/controllers/merge_requests_controller.rb | 7 ++
...new_organisation_telephone_number.html.erb | 15 ++++
.../new_organisation_type.html.erb | 5 ++
config/locales/en.yml | 3 +-
config/routes.rb | 1 +
...ation_telephone_number_to_merge_request.rb | 5 ++
db/schema.rb | 3 +-
.../merge_requests_controller_spec.rb | 68 +++++++++++++++++--
8 files changed, 100 insertions(+), 7 deletions(-)
create mode 100644 app/views/merge_requests/new_organisation_type.html.erb
create mode 100644 db/migrate/20230505105327_add_new_organisation_telephone_number_to_merge_request.rb
diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index da7b9ae75..d717f6ee7 100644
--- a/app/controllers/merge_requests_controller.rb
+++ b/app/controllers/merge_requests_controller.rb
@@ -9,6 +9,7 @@ class MergeRequestsController < ApplicationController
new_organisation_name
new_organisation_address
new_organisation_telephone_number
+ new_organisation_type
merge_date
]
before_action :authenticate_user!
@@ -19,6 +20,7 @@ class MergeRequestsController < ApplicationController
def new_organisation_name; end
def new_organisation_address; end
def new_organisation_telephone_number; end
+ def new_organisation_type; end
def merge_date; end
def create
@@ -83,6 +85,8 @@ private
new_organisation_address_merge_request_path(@merge_request)
when "new_organisation_address"
new_organisation_telephone_number_merge_request_path(@merge_request)
+ when "new_organisation_telephone_number"
+ new_organisation_type_merge_request_path(@merge_request)
end
end
@@ -115,6 +119,7 @@ private
:new_organisation_address_line1,
:new_organisation_address_line2,
:new_organisation_postcode,
+ :new_organisation_telephone_number,
)
if merge_params[:requesting_organisation_id].present? && (current_user.data_coordinator? || current_user.data_provider?)
@@ -150,6 +155,8 @@ private
end
when "new_organisation_name"
@merge_request.errors.add(:new_organisation_name, :blank) if merge_request_params[:new_organisation_name].blank?
+ when "new_organisation_telephone_number"
+ @merge_request.errors.add(:new_organisation_telephone_number, :blank) if merge_request_params[:new_organisation_telephone_number].blank?
end
end
diff --git a/app/views/merge_requests/new_organisation_telephone_number.html.erb b/app/views/merge_requests/new_organisation_telephone_number.html.erb
index 059ca752d..6c0c1d81e 100644
--- a/app/views/merge_requests/new_organisation_telephone_number.html.erb
+++ b/app/views/merge_requests/new_organisation_telephone_number.html.erb
@@ -3,3 +3,18 @@
<% content_for :title, title %>
<%= govuk_back_link href: new_organisation_address_merge_request_path(@merge_request) %>
<% end %>
+
+<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %>
+ <%= f.govuk_error_summary %>
+
+ What is <%= @merge_request.new_organisation_name.possessive %> telephone number?
+
+
+ <%= f.govuk_text_field :new_organisation_telephone_number, label: nil, width: "two-thirds" %>
+ <%= f.hidden_field :page, value: "new_organisation_telephone_number" %>
+
+ <%= f.govuk_submit %>
+
+
+