baarkerlounger
/
submit-social-housing-lettings-and-sales-data
mirror of https://github.com/communitiesuk/submit-social-housing-lettings-and-sales-data.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

CLDC-2235 Bulk upload now support invalid options (#1564) 

# Context

- https://digital.dclg.gov.uk/jira/browse/CLDC-2235
- Support invalid options for bulk upload. one use case when user for a new collection year supplies a value only valid in the previous collection year

# Changes

- this validation works before `log.valid?` clears any fields
- as a result there is the potential to get 2 errors on a field for when it becomes blanked and invalid option occur together
- bulk upload validations are now split so that they run before or after `log.valid?`. this is due to the fact that `log.valid?` heavily mutates the `log` object. so we want to validate both before and after the data mutates depending on what needs to be checked
- errors must be duplicated and merged as calling `valid?` clears any existing errors on the object
- all validations are assigned a specific context otherwise they are added to the default context and will also be called when a context is given
pull/1566/head
Phil Lee 2 years ago committed by GitHub
parent
210a25eaf5
commit
3f27008034
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 124 additions and 108 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 110
      app/services/bulk_upload/lettings/year2022/row_parser.rb
  2. 112
      app/services/bulk_upload/lettings/year2023/row_parser.rb
  3. 8
      spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb
  4. 2
      spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb

110
app/services/bulk_upload/lettings/year2022/row_parser.rb
Unescape View File

@ -278,58 +278,58 @@ class BulkUpload::Lettings::Year2022::RowParser
attribute :field_134, :integer attribute :field_134, :integer
validates :field_1, presence: { message: I18n.t("validations.not_answered", question: "letting type") }, validates :field_1, presence: { message: I18n.t("validations.not_answered", question: "letting type") },
inclusion: { in: (1..12).to_a, message: I18n.t("validations.invalid_option", question: "letting type") } inclusion: { in: (1..12).to_a, message: I18n.t("validations.invalid_option", question: "letting type") }, on: :after_log
validates :field_4, presence: { if: proc { [2, 4, 6, 8, 10, 12].include?(field_1) } } validates :field_4, presence: { if: proc { [2, 4, 6, 8, 10, 12].include?(field_1) } }, on: :after_log
validates :field_12, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 1 must be a number or the letter R" } validates :field_12, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 1 must be a number or the letter R" }, on: :after_log
validates :field_13, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 2 must be a number or the letter R" }, allow_blank: true validates :field_13, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 2 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_14, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 3 must be a number or the letter R" }, allow_blank: true validates :field_14, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 3 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_15, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 4 must be a number or the letter R" }, allow_blank: true validates :field_15, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 4 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_16, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 5 must be a number or the letter R" }, allow_blank: true validates :field_16, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 5 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_17, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 6 must be a number or the letter R" }, allow_blank: true validates :field_17, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 6 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_18, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 7 must be a number or the letter R" }, allow_blank: true validates :field_18, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 7 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_19, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 8 must be a number or the letter R" }, allow_blank: true validates :field_19, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 8 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_96, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (day)") } validates :field_96, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (day)") }, on: :after_log
validates :field_97, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (month)") } validates :field_97, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (month)") }, on: :after_log
validates :field_98, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (year)") } validates :field_98, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (year)") }, on: :after_log
validates :field_98, format: { with: /\A\d{2}\z/, message: I18n.t("validations.setup.startdate.year_not_two_digits") } validates :field_98, format: { with: /\A\d{2}\z/, message: I18n.t("validations.setup.startdate.year_not_two_digits") }, on: :after_log
validate :validate_data_types validate :validate_data_types, on: :after_log
validate :validate_nulls validate :validate_nulls, on: :after_log
validate :validate_relevant_collection_window validate :validate_relevant_collection_window, on: :after_log
validate :validate_la_with_local_housing_referral validate :validate_la_with_local_housing_referral, on: :after_log
validate :validate_cannot_be_la_referral_if_general_needs_and_la validate :validate_cannot_be_la_referral_if_general_needs_and_la, on: :after_log
validate :validate_leaving_reason_for_renewal validate :validate_leaving_reason_for_renewal, on: :after_log
validate :validate_lettings_type_matches_bulk_upload validate :validate_lettings_type_matches_bulk_upload, on: :after_log
validate :validate_only_one_housing_needs_type validate :validate_only_one_housing_needs_type, on: :after_log
validate :validate_no_disabled_needs_conjunction validate :validate_no_disabled_needs_conjunction, on: :after_log
validate :validate_dont_know_disabled_needs_conjunction validate :validate_dont_know_disabled_needs_conjunction, on: :after_log
validate :validate_no_and_dont_know_disabled_needs_conjunction validate :validate_no_and_dont_know_disabled_needs_conjunction, on: :after_log
validate :validate_owning_org_data_given validate :validate_owning_org_data_given, on: :after_log
validate :validate_owning_org_exists validate :validate_owning_org_exists, on: :after_log
validate :validate_owning_org_owns_stock validate :validate_owning_org_owns_stock, on: :after_log
validate :validate_owning_org_permitted validate :validate_owning_org_permitted, on: :after_log
validate :validate_managing_org_data_given validate :validate_managing_org_data_given, on: :after_log
validate :validate_managing_org_exists validate :validate_managing_org_exists, on: :after_log
validate :validate_managing_org_related validate :validate_managing_org_related, on: :after_log
validate :validate_scheme_related validate :validate_scheme_related, on: :after_log
validate :validate_scheme_exists validate :validate_scheme_exists, on: :after_log
validate :validate_scheme_data_given validate :validate_scheme_data_given, on: :after_log
validate :validate_location_related validate :validate_location_related, on: :after_log
validate :validate_location_exists validate :validate_location_exists, on: :after_log
validate :validate_location_data_given validate :validate_location_data_given, on: :after_log
validate :validate_created_by_exists validate :validate_created_by_exists, on: :after_log
validate :validate_created_by_related validate :validate_created_by_related, on: :after_log
validate :validate_rent_type validate :validate_rent_type, on: :after_log
validate :validate_valid_radio_option validate :validate_valid_radio_option, on: :before_log
def self.question_for_field(field) def self.question_for_field(field)
QUESTIONS[field] QUESTIONS[field]
@ -340,9 +340,13 @@ class BulkUpload::Lettings::Year2022::RowParser
return true if blank_row? return true if blank_row?
super(:before_log)
before_errors = errors.dup
log.valid? log.valid?
super super(:after_log)
errors.merge!(before_errors)
log.errors.each do |error| log.errors.each do |error|
fields = field_mapping_for_errors[error.attribute] || [] fields = field_mapping_for_errors[error.attribute] || []

112
app/services/bulk_upload/lettings/year2023/row_parser.rb
Unescape View File

@ -280,59 +280,59 @@ class BulkUpload::Lettings::Year2023::RowParser
attribute :field_134, :decimal attribute :field_134, :decimal
validates :field_5, presence: { message: I18n.t("validations.not_answered", question: "letting type") }, validates :field_5, presence: { message: I18n.t("validations.not_answered", question: "letting type") },
inclusion: { in: (1..12).to_a, message: I18n.t("validations.invalid_option", question: "letting type") } inclusion: { in: (1..12).to_a, message: I18n.t("validations.invalid_option", question: "letting type") }, on: :after_log
validates :field_16, presence: { if: proc { [2, 4, 6, 8, 10, 12].include?(field_5) } } validates :field_16, presence: { if: proc { [2, 4, 6, 8, 10, 12].include?(field_5) } }, on: :after_log
validates :field_46, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 1 must be a number or the letter R" } validates :field_46, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 1 must be a number or the letter R" }, on: :after_log
validates :field_52, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 2 must be a number or the letter R" }, allow_blank: true validates :field_52, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 2 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_56, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 3 must be a number or the letter R" }, allow_blank: true validates :field_56, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 3 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_60, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 4 must be a number or the letter R" }, allow_blank: true validates :field_60, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 4 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_64, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 5 must be a number or the letter R" }, allow_blank: true validates :field_64, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 5 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_68, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 6 must be a number or the letter R" }, allow_blank: true validates :field_68, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 6 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_72, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 7 must be a number or the letter R" }, allow_blank: true validates :field_72, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 7 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_76, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 8 must be a number or the letter R" }, allow_blank: true validates :field_76, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 8 must be a number or the letter R" }, allow_blank: true, on: :after_log
validates :field_6, presence: { message: I18n.t("validations.not_answered", question: "property renewal") } validates :field_6, presence: { message: I18n.t("validations.not_answered", question: "property renewal") }, on: :after_log
validates :field_7, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (day)") } validates :field_7, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (day)") }, on: :after_log
validates :field_8, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (month)") } validates :field_8, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (month)") }, on: :after_log
validates :field_9, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (year)") } validates :field_9, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (year)") }, on: :after_log
validates :field_9, format: { with: /\A\d{2}\z/, message: I18n.t("validations.setup.startdate.year_not_two_digits") } validates :field_9, format: { with: /\A\d{2}\z/, message: I18n.t("validations.setup.startdate.year_not_two_digits") }, on: :after_log
validate :validate_needs_type_present validate :validate_needs_type_present, on: :after_log
validate :validate_data_types validate :validate_data_types, on: :after_log
validate :validate_nulls validate :validate_nulls, on: :after_log
validate :validate_relevant_collection_window validate :validate_relevant_collection_window, on: :after_log
validate :validate_la_with_local_housing_referral validate :validate_la_with_local_housing_referral, on: :after_log
validate :validate_cannot_be_la_referral_if_general_needs_and_la validate :validate_cannot_be_la_referral_if_general_needs_and_la, on: :after_log
validate :validate_leaving_reason_for_renewal validate :validate_leaving_reason_for_renewal, on: :after_log
validate :validate_lettings_type_matches_bulk_upload validate :validate_lettings_type_matches_bulk_upload, on: :after_log
validate :validate_only_one_housing_needs_type validate :validate_only_one_housing_needs_type, on: :after_log
validate :validate_no_disabled_needs_conjunction validate :validate_no_disabled_needs_conjunction, on: :after_log
validate :validate_dont_know_disabled_needs_conjunction validate :validate_dont_know_disabled_needs_conjunction, on: :after_log
validate :validate_no_and_dont_know_disabled_needs_conjunction validate :validate_no_and_dont_know_disabled_needs_conjunction, on: :after_log
validate :validate_owning_org_data_given validate :validate_owning_org_data_given, on: :after_log
validate :validate_owning_org_exists validate :validate_owning_org_exists, on: :after_log
validate :validate_owning_org_owns_stock validate :validate_owning_org_owns_stock, on: :after_log
validate :validate_owning_org_permitted validate :validate_owning_org_permitted, on: :after_log
validate :validate_managing_org_data_given validate :validate_managing_org_data_given, on: :after_log
validate :validate_managing_org_exists validate :validate_managing_org_exists, on: :after_log
validate :validate_managing_org_related validate :validate_managing_org_related, on: :after_log
validate :validate_scheme_related validate :validate_scheme_related, on: :after_log
validate :validate_scheme_exists validate :validate_scheme_exists, on: :after_log
validate :validate_scheme_data_given validate :validate_scheme_data_given, on: :after_log
validate :validate_location_related validate :validate_location_related, on: :after_log
validate :validate_location_exists validate :validate_location_exists, on: :after_log
validate :validate_location_data_given validate :validate_location_data_given, on: :after_log
validate :validate_created_by_exists validate :validate_created_by_exists, on: :after_log
validate :validate_created_by_related validate :validate_created_by_related, on: :after_log
validate :validate_valid_radio_option validate :validate_valid_radio_option, on: :before_log
def self.question_for_field(field) def self.question_for_field(field)
QUESTIONS[field] QUESTIONS[field]
@ -343,9 +343,13 @@ class BulkUpload::Lettings::Year2023::RowParser
return true if blank_row? return true if blank_row?
super(:before_log)
before_errors = errors.dup
log.valid? log.valid?
super super(:after_log)
errors.merge!(before_errors)
log.errors.each do |error| log.errors.each do |error|
fields = field_mapping_for_errors[error.attribute] || [] fields = field_mapping_for_errors[error.attribute] || []

8
spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb
Unescape View File

@ -507,6 +507,14 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
expect(parser.errors[:field_52]).to be_present expect(parser.errors[:field_52]).to be_present
end end
end end
context "when not a valid option" do
let(:attributes) { setup_section_params.merge({ bulk_upload:, field_52: "99" }) }
it "has error for invalid option" do
expect(parser.errors[:field_52]).to include("Enter a valid value for What is the tenant's main reason for the household leaving their last settled home?")
end
end
end end
end end

2
spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
Unescape View File

@ -509,7 +509,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
let(:attributes) { setup_section_params.merge({ field_102: "7" }) } let(:attributes) { setup_section_params.merge({ field_102: "7" }) }
it "returns an error" do it "returns an error" do
expect(parser.errors[:field_102]).to be_present expect(parser.errors[:field_102]).to include("Enter a valid value for What is the tenant’s main reason for the household leaving their last settled home?")
end end
end end
end end

Write Preview
Loading…
Cancel
Save