From 4520b3a1ccebd761e92c0e56b1dcf7b61ea4be7f Mon Sep 17 00:00:00 2001 From: magicmilo Date: Tue, 26 Oct 2021 16:03:48 +0100 Subject: [PATCH 01/12] added armed forces active response validation --- app/models/case_log.rb | 11 ++++++++++ spec/helpers/check_answers_helper_spec.rb | 6 ++++-- spec/models/case_log_spec.rb | 25 +++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index aad4d23b0..f56f20e2f 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -55,6 +55,17 @@ class CaseLogValidator < ActiveModel::Validator end end + def validate_armed_forces_active_response(record) + # binding.pry + if record.armed_forces == "Yes - a regular" && record.armed_forces_active.blank? + record.errors.add :armed_forces_active, "You must answer the armed forces active question if the tenant has served as a regular in the armed forces" + end + + if record.armed_forces != "Yes - a regular" && !record.armed_forces_active.blank? + record.errors.add :armed_forces_active, "You must not answer the armed forces active question if the tenant has not served as a regular in the armed forces" + end + end + def validate(record) # If we've come from the form UI we only want to validate the specific fields # that have just been submitted. If we're submitting a log via API or Bulk Upload diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb index cc619a041..e53c54957 100644 --- a/spec/helpers/check_answers_helper_spec.rb +++ b/spec/helpers/check_answers_helper_spec.rb @@ -11,7 +11,9 @@ RSpec.describe CheckAnswersHelper do ) end let(:case_log_with_met_radio_condition) do - FactoryBot.create(:case_log, armed_forces: "Yes - a regular", armed_forces_injured: "No") + FactoryBot.create(:case_log, armed_forces: "Yes - a regular", + armed_forces_injured: "No", + armed_forces_active: "Yes") end let(:subsection) { "income_and_benefits" } let(:subsection_with_numeric_conditionals) { "household_characteristics" } @@ -56,7 +58,7 @@ RSpec.describe CheckAnswersHelper do subsection_with_radio_conditionals, case_log_with_met_radio_condition, form, - )).to equal(3) + )).to equal(4) end end diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 265296e40..5fcb41bec 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -96,6 +96,31 @@ RSpec.describe Form, type: :model do }.to raise_error(ActiveRecord::RecordInvalid) end end + + context "armed forces active validation" do + it "must be answered if ever served in the forces as a regular" do + expect { + CaseLog.create!(armed_forces: "Yes - a regular", + armed_forces_active: nil) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "must not be answered if not ever served as a regular" do + expect { + CaseLog.create!(armed_forces: "No", + armed_forces_active: "Yes") + }.to raise_error() + end + + #Crossover over tests here as injured must be answered as well for no error + it "must be answered if ever served in the forces as a regular" do + expect { + CaseLog.create!(armed_forces: "Yes - a regular", + armed_forces_active: "Yes", + armed_forces_injured: "Yes") + }.not_to raise_error() + end + end end describe "status" do From ede51029e9c6a40c61bbfcf346aeb4cbd0e2a8e4 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Tue, 26 Oct 2021 16:06:56 +0100 Subject: [PATCH 02/12] rubocop --- app/models/case_log.rb | 2 +- spec/helpers/check_answers_helper_spec.rb | 4 ++-- spec/models/case_log_spec.rb | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index f56f20e2f..9e5f4e453 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -61,7 +61,7 @@ class CaseLogValidator < ActiveModel::Validator record.errors.add :armed_forces_active, "You must answer the armed forces active question if the tenant has served as a regular in the armed forces" end - if record.armed_forces != "Yes - a regular" && !record.armed_forces_active.blank? + if record.armed_forces != "Yes - a regular" && record.armed_forces_active.present? record.errors.add :armed_forces_active, "You must not answer the armed forces active question if the tenant has not served as a regular in the armed forces" end end diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb index e53c54957..cac582f93 100644 --- a/spec/helpers/check_answers_helper_spec.rb +++ b/spec/helpers/check_answers_helper_spec.rb @@ -12,8 +12,8 @@ RSpec.describe CheckAnswersHelper do end let(:case_log_with_met_radio_condition) do FactoryBot.create(:case_log, armed_forces: "Yes - a regular", - armed_forces_injured: "No", - armed_forces_active: "Yes") + armed_forces_injured: "No", + armed_forces_active: "Yes") end let(:subsection) { "income_and_benefits" } let(:subsection_with_numeric_conditionals) { "household_characteristics" } diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 5fcb41bec..3907a8176 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -109,16 +109,16 @@ RSpec.describe Form, type: :model do expect { CaseLog.create!(armed_forces: "No", armed_forces_active: "Yes") - }.to raise_error() + }.to raise_error end - #Crossover over tests here as injured must be answered as well for no error + # Crossover over tests here as injured must be answered as well for no error it "must be answered if ever served in the forces as a regular" do expect { CaseLog.create!(armed_forces: "Yes - a regular", armed_forces_active: "Yes", armed_forces_injured: "Yes") - }.not_to raise_error() + }.not_to raise_error end end end From 3d2699e557525e960fadf11e4ccae00ef95dee81 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Wed, 27 Oct 2021 09:33:58 +0100 Subject: [PATCH 03/12] added simple validation for outstanding rent --- app/models/case_log.rb | 9 +++++++++ spec/models/case_log_spec.rb | 16 ++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index aad4d23b0..ef4655a7a 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -55,6 +55,15 @@ class CaseLogValidator < ActiveModel::Validator end end + def validate_outstanding_rent_amount(record) + if record.outstanding_rent_or_charges == "Yes" && record.outstanding_amount.blank? + record.errors.add :outstanding_amount, "You must answer the oustanding amout question if you have outstanding rent or charges." + end + if record.outstanding_rent_or_charges == "No" && !record.outstanding_amount.blank? + record.errors.add :outstanding_amount, "You must not answer the oustanding amout question if you don't have outstanding rent or charges." + end + end + def validate(record) # If we've come from the form UI we only want to validate the specific fields # that have just been submitted. If we're submitting a log via API or Bulk Upload diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 265296e40..aaa7f8c84 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -96,6 +96,22 @@ RSpec.describe Form, type: :model do }.to raise_error(ActiveRecord::RecordInvalid) end end + + context "outstanding rent or charges validation" do + it "must be anwered if answered yes to outstanding rent or charges" do + expect { + CaseLog.create!(outstanding_rent_or_charges: "Yes", + outstanding_amount: nil) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "must be not be anwered if answered no to outstanding rent or charges" do + expect { + CaseLog.create!(outstanding_rent_or_charges: "No", + outstanding_amount: 99) + }.to raise_error(ActiveRecord::RecordInvalid) + end + end end describe "status" do From 2c1f246350a1676ebd95fd0a12dc8be635edc754 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:08:14 +0100 Subject: [PATCH 04/12] CLDC-473: Add net income uc proportion validation (#62) * Add validate_net_income_uc_proportion validation/ * refactor validation method; * Update net income options in doc * rubocop * refactor validate_net_income_uc_proportion and increase the persons checked to 8 * rename tenant economic status field --- app/models/case_log.rb | 13 ++++++++++++ config/forms/2021_2022.json | 2 +- .../20211027091521_rename_person1_fields.rb | 7 +++++++ db/schema.rb | 4 ++-- docs/api/DLUHC-CORE-Data.v1.json | 16 ++++++++++----- spec/fixtures/complete_case_log.json | 2 +- spec/models/case_log_spec.rb | 20 +++++++++++++++++++ 7 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20211027091521_rename_person1_fields.rb diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 0b347a784..99285ca3f 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -55,6 +55,19 @@ class CaseLogValidator < ActiveModel::Validator end end + EMPLOYED_STATUSES = ["Full-time - 30 hours or more", "Part-time - Less than 30 hours"].freeze + def validate_net_income_uc_proportion(record) + (1..8).any? do |n| + economic_status = record["person_#{n}_economic_status"] + is_employed = EMPLOYED_STATUSES.include?(economic_status) + relationship = record["person_#{n}_relationship"] + is_partner_or_main = relationship == "Partner" || (relationship.nil? && economic_status.present?) + if is_employed && is_partner_or_main && record.net_income_uc_proportion == "All" + record.errors.add :net_income_uc_proportion, "income is from Universal Credit, state pensions or benefits cannot be All if the tenant or the partner works part or full time" + end + end + end + def validate_household_pregnancy(record) if (record.pregnancy == "Yes" || record.pregnancy == "Prefer not to say") && !women_of_child_bearing_age_in_household(record) record.errors.add :pregnancy, "You must answer no as there are no female tenants aged 16-50 in the property" diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index e658da607..ed99d51cf 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -121,7 +121,7 @@ "header": "", "description": "", "questions": { - "tenant_economic_status": { + "person_1_economic_status": { "check_answer_label": "Work", "header": "Which of these best describes the tenant's working situation?", "hint_text": "", diff --git a/db/migrate/20211027091521_rename_person1_fields.rb b/db/migrate/20211027091521_rename_person1_fields.rb new file mode 100644 index 000000000..a9b807e36 --- /dev/null +++ b/db/migrate/20211027091521_rename_person1_fields.rb @@ -0,0 +1,7 @@ +class RenamePerson1Fields < ActiveRecord::Migration[6.1] + def change + change_table :case_logs, bulk: true do |t| + t.rename :tenant_economic_status, :person_1_economic_status + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 2d4f30da1..8d2405443 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_10_26_123542) do +ActiveRecord::Schema.define(version: 2021_10_27_091521) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -26,7 +26,7 @@ ActiveRecord::Schema.define(version: 2021_10_26_123542) do t.string "tenant_nationality" t.string "previous_housing_situation" t.string "armed_forces" - t.string "tenant_economic_status" + t.string "person_1_economic_status" t.integer "household_number_of_other_members" t.string "person_2_relationship" t.integer "person_2_age" diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index a62d1f184..f50e096fb 100644 --- a/docs/api/DLUHC-CORE-Data.v1.json +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -274,7 +274,7 @@ "tenant_nationality": "UK national resident in UK", "previous_housing_situation": "Private sector tenancy", "armed_forces": "Yes - a regular", - "tenant_economic_status": "Full-time - 30 hours or more", + "person_1_economic_status": "Full-time - 30 hours or more", "household_number_of_other_members": 7, "person_2_relationship": "Partner", "person_2_age": 32, @@ -459,7 +459,7 @@ "type": "string", "minLength": 1 }, - "tenant_economic_status": { + "person_1_economic_status": { "type": "string", "minLength": 1, "enum": [ @@ -945,7 +945,13 @@ }, "net_income_uc_proportion": { "type": "string", - "minLength": 1 + "minLength": 1, + "enum": [ + "All", + "Some", + "None", + "Do not know" + ] }, "housing_benefit": { "type": "string", @@ -1092,7 +1098,7 @@ "tenant_nationality", "previous_housing_situation", "armed_forces", - "tenant_economic_status", + "person_1_economic_status", "household_number_of_other_members", "person_2_relationship", "person_2_age", @@ -1202,4 +1208,4 @@ }, "securitySchemes": {} } -} \ No newline at end of file +} diff --git a/spec/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json index 8a539e91c..541c63970 100644 --- a/spec/fixtures/complete_case_log.json +++ b/spec/fixtures/complete_case_log.json @@ -8,7 +8,7 @@ "tenant_nationality": "UK national resident in UK", "previous_housing_situation": "Private sector tenancy", "armed_forces": "Yes - a regular", - "tenant_economic_status": "Full-time - 30 hours or more", + "person_1_economic_status": "Full-time - 30 hours or more", "household_number_of_other_members": 7, "person_2_relationship": "Partner", "person_2_age": 32, diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index c9339d719..ae85b2fd1 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -96,6 +96,26 @@ RSpec.describe Form, type: :model do }.to raise_error(ActiveRecord::RecordInvalid) end end + + context "tenant’s income is from Universal Credit, state pensions or benefits" do + it "Cannot be All if person 1 works full time" do + expect { + CaseLog.create!(net_income_uc_proportion: "All", person_1_economic_status: "Full-time - 30 hours or more") + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "Cannot be All if person 1 works part time" do + expect { + CaseLog.create!(net_income_uc_proportion: "All", person_1_economic_status: "Part-time - Less than 30 hours") + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "Cannot be 1 All if any of persons 2-4 are person 1's partner and work part or full time" do + expect { + CaseLog.create!(net_income_uc_proportion: "All", person_2_relationship: "Partner", person_2_economic_status: "Part-time - Less than 30 hours") + }.to raise_error(ActiveRecord::RecordInvalid) + end + end end describe "status" do From d4497883aecb2690cfac8445121336e227ab09b7 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Wed, 27 Oct 2021 11:25:48 +0100 Subject: [PATCH 05/12] rubocop --- app/models/case_log.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index ef4655a7a..b7e7e1c0f 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -59,7 +59,7 @@ class CaseLogValidator < ActiveModel::Validator if record.outstanding_rent_or_charges == "Yes" && record.outstanding_amount.blank? record.errors.add :outstanding_amount, "You must answer the oustanding amout question if you have outstanding rent or charges." end - if record.outstanding_rent_or_charges == "No" && !record.outstanding_amount.blank? + if record.outstanding_rent_or_charges == "No" && record.outstanding_amount.present? record.errors.add :outstanding_amount, "You must not answer the oustanding amout question if you don't have outstanding rent or charges." end end From c39271d4867f110748cd76361f9a61829cad149a Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:48:33 +0100 Subject: [PATCH 06/12] CLDC-466: Add fixed term tenancy validation (#66) * Add fixed term tenancy validation * refactor validate_fixed_term_tenancy * add fixed_term_tenancy pattern to api docs and make it dynamically not required --- app/models/case_log.rb | 18 ++++++++++ docs/api/DLUHC-CORE-Data.v1.json | 3 +- spec/fixtures/complete_case_log.json | 2 +- spec/models/case_log_spec.rb | 51 ++++++++++++++++++++++++++-- 4 files changed, 70 insertions(+), 4 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 99285ca3f..0ff5e9c95 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -74,6 +74,20 @@ class CaseLogValidator < ActiveModel::Validator end end + def validate_fixed_term_tenancy(record) + is_present = record.fixed_term_tenancy.present? + is_in_range = record.fixed_term_tenancy.to_i.between?(2, 99) + is_secure = record.tenancy_type == "Fixed term – Secure" + is_ast = record.tenancy_type == "Fixed term – Assured Shorthold Tenancy (AST)" + conditions = [ + { condition: !(is_secure || is_ast) && is_present, error: "You must only answer the fixed term tenancy length question if the tenancy type is fixed term" }, + { condition: is_ast && !is_in_range, error: "Fixed term – Assured Shorthold Tenancy (AST) should be between 2 and 99 years" }, + { condition: is_secure && (!is_in_range && is_present), error: "Fixed term – Secure should be between 2 and 99 years or not specified" }, + ] + + conditions.each { |condition| condition[:condition] ? (record.errors.add :fixed_term_tenancy, condition[:error]) : nil } + end + def validate(record) # If we've come from the form UI we only want to validate the specific fields # that have just been submitted. If we're submitting a log via API or Bulk Upload @@ -172,6 +186,10 @@ private dynamically_not_required << "net_income_frequency" end + if tenancy_type == "Fixed term – Secure" + dynamically_not_required << "fixed_term_tenancy" + end + required.delete_if { |key, _value| dynamically_not_required.include?(key) } end end diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index f50e096fb..64d5ca517 100644 --- a/docs/api/DLUHC-CORE-Data.v1.json +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -873,7 +873,8 @@ }, "fixed_term_tenancy": { "type": "string", - "minLength": 1 + "minLength": 1, + "pattern": "((?!1|0)([0-9][0-9]))+" }, "tenancy_type": { "type": "string", diff --git a/spec/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json index 541c63970..9238bba6a 100644 --- a/spec/fixtures/complete_case_log.json +++ b/spec/fixtures/complete_case_log.json @@ -51,7 +51,7 @@ "tenancy_code": "BZ757", "tenancy_start_date": "12/03/2019", "starter_tenancy": "No", - "fixed_term_tenancy": "No", + "fixed_term_tenancy": "5", "tenancy_type": "Fixed term – Secure", "letting_type": "Affordable Rent - General Needs", "letting_provider": "This landlord", diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index ae85b2fd1..e8241bd14 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -82,14 +82,14 @@ RSpec.describe Form, type: :model do end context "armed forces injured validation" do - it "must be anwered if tenant was a regular or reserve in armed forces" do + it "must be answered if tenant was a regular or reserve in armed forces" do expect { CaseLog.create!(armed_forces: "Yes - a regular", armed_forces_injured: nil) }.to raise_error(ActiveRecord::RecordInvalid) end - it "must be anwered if tenant was not a regular or reserve in armed forces" do + it "must be answered if tenant was not a regular or reserve in armed forces" do expect { CaseLog.create!(armed_forces: "No", armed_forces_injured: "Yes") @@ -116,6 +116,53 @@ RSpec.describe Form, type: :model do }.to raise_error(ActiveRecord::RecordInvalid) end end + context "fixed term tenancy length" do + it "Must not be completed if Type of main tenancy is not responded with either Secure or Assured shorthold " do + expect { + CaseLog.create!(tenancy_type: "Other", + fixed_term_tenancy: 10) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "Must be completed and between 2 and 99 if type of tenancy is Assured shorthold" do + expect { + CaseLog.create!(tenancy_type: "Fixed term – Assured Shorthold Tenancy (AST)", + fixed_term_tenancy: 1) + }.to raise_error(ActiveRecord::RecordInvalid) + + expect { + CaseLog.create!(tenancy_type: "Fixed term – Assured Shorthold Tenancy (AST)", + fixed_term_tenancy: nil) + }.to raise_error(ActiveRecord::RecordInvalid) + + expect { + CaseLog.create!(tenancy_type: "Fixed term – Assured Shorthold Tenancy (AST)", + fixed_term_tenancy: 2) + }.not_to raise_error + end + + it "Must be empty or between 2 and 99 if type of tenancy is Secure" do + expect { + CaseLog.create!(tenancy_type: "Fixed term – Secure", + fixed_term_tenancy: 1) + }.to raise_error(ActiveRecord::RecordInvalid) + + expect { + CaseLog.create!(tenancy_type: "Fixed term – Secure", + fixed_term_tenancy: 100) + }.to raise_error(ActiveRecord::RecordInvalid) + + expect { + CaseLog.create!(tenancy_type: "Fixed term – Secure", + fixed_term_tenancy: nil) + }.not_to raise_error + + expect { + CaseLog.create!(tenancy_type: "Fixed term – Secure", + fixed_term_tenancy: 2) + }.not_to raise_error + end + end end describe "status" do From c5e6506e48feedb2dbad2bc5957f593fc57ef604 Mon Sep 17 00:00:00 2001 From: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> Date: Wed, 27 Oct 2021 12:12:13 +0100 Subject: [PATCH 07/12] Database Cleaner (#67) --- Gemfile | 18 +++++++++++------- Gemfile.lock | 15 +++++++++++---- spec/rails_helper.rb | 44 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index 3039202f0..0084b7546 100644 --- a/Gemfile +++ b/Gemfile @@ -29,15 +29,8 @@ gem "discard" group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem "byebug", platforms: %i[mri mingw x64_mingw] - gem "capybara", require: false gem "dotenv-rails" - gem "factory_bot_rails" gem "pry-byebug" - gem "selenium-webdriver", require: false - gem "simplecov", require: false - %w[rspec-core rspec-expectations rspec-mocks rspec-rails rspec-support].each do |lib| - gem lib, git: "https://github.com/rspec/#{lib}.git", branch: "main", require: false - end end group :development do @@ -54,5 +47,16 @@ group :development do gem "scss_lint-govuk" end +group :test do + gem "capybara", require: false + gem "factory_bot_rails" + gem "selenium-webdriver", require: false + gem "simplecov", require: false + gem "database_cleaner-active_record", require: false + %w[rspec-core rspec-expectations rspec-mocks rspec-rails rspec-support].each do |lib| + gem lib, git: "https://github.com/rspec/#{lib}.git", branch: "main", require: false + end +end + # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] diff --git a/Gemfile.lock b/Gemfile.lock index b48165040..8abc24776 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -26,7 +26,7 @@ GIT GIT remote: https://github.com/rspec/rspec-rails.git - revision: 3f0e35085f5765decf96fee179ec9a2e132a67c1 + revision: cfe4db707cc5a0c9437aa90e3059256f30368da4 branch: main specs: rspec-rails (5.1.0.pre) @@ -40,7 +40,7 @@ GIT GIT remote: https://github.com/rspec/rspec-support.git - revision: 28526172c42302858c18681d7d7580490d885b4d + revision: 2a17194b9fc868a4b4a41d7168103dca825c3004 branch: main specs: rspec-support (3.11.0.pre) @@ -115,8 +115,9 @@ GEM msgpack (~> 1.0) builder (3.2.4) byebug (11.1.3) - capybara (3.35.3) + capybara (3.36.0) addressable + matrix mini_mime (>= 0.1.3) nokogiri (~> 1.8) rack (>= 1.6.0) @@ -127,6 +128,10 @@ GEM coderay (1.1.3) concurrent-ruby (1.1.9) crass (1.0.6) + database_cleaner-active_record (2.0.1) + activerecord (>= 5.a) + database_cleaner-core (~> 2.0.0) + database_cleaner-core (2.0.1) deep_merge (1.2.1) diff-lcs (1.4.4) discard (1.2.0) @@ -145,7 +150,7 @@ GEM ffi (1.15.4) globalid (0.5.2) activesupport (>= 5.0) - govuk-components (2.1.3) + govuk-components (2.1.4) activemodel (>= 6.0) railties (>= 6.0) view_component (~> 2.39.0) @@ -172,6 +177,7 @@ GEM mail (2.7.1) mini_mime (>= 0.1.1) marcel (1.0.2) + matrix (0.4.2) method_source (1.0.0) mini_mime (1.1.2) minitest (5.14.4) @@ -334,6 +340,7 @@ DEPENDENCIES bootsnap (>= 1.4.4) byebug capybara + database_cleaner-active_record discard dotenv-rails factory_bot_rails diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 726c09017..b873a5929 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -6,6 +6,7 @@ require File.expand_path("../config/environment", __dir__) abort("The Rails environment is running in production mode!") if Rails.env.production? require "rspec/rails" require "capybara/rspec" +require 'database_cleaner/active_record' # Comment to run `js: true specs` with visible browser interaction Capybara.javascript_driver = :selenium_headless @@ -42,7 +43,48 @@ RSpec.configure do |config| # If you're not using ActiveRecord, or you'd prefer not to run each of your # examples within a transaction, remove the following line or assign false # instead of true. - config.use_transactional_fixtures = true + # config.use_transactional_fixtures = true + + config.before(:suite) do + if config.use_transactional_fixtures? + raise(<<-MSG) + Delete line `config.use_transactional_fixtures = true` from rails_helper.rb + (or set it to false) to prevent uncommitted transactions being used in + JavaScript-dependent specs. + + During testing, the app-under-test that the browser driver connects to + uses a different database connection to the database connection used by + the spec. The app's database connection would not be able to access + uncommitted transaction data setup over the spec's database connection. + MSG + end + DatabaseCleaner.clean_with(:truncation) + end + + config.before(:each) do + DatabaseCleaner.strategy = :transaction + end + + config.before(:each, type: :feature) do + # :rack_test driver's Rack app under test shares database connection + # with the specs, so continue to use transaction strategy for speed. + driver_shares_db_connection_with_specs = Capybara.current_driver == :rack_test + + unless driver_shares_db_connection_with_specs + # Driver is probably for an external browser with an app + # under test that does *not* share a database connection with the + # specs, so use truncation strategy. + DatabaseCleaner.strategy = :truncation + end + end + + config.before(:each) do + DatabaseCleaner.start + end + + config.append_after(:each) do + DatabaseCleaner.clean + end # You can uncomment this line to turn off ActiveRecord support entirely. # config.use_active_record = false From 096cb8fae71aa4518d582fba632d960023da605b Mon Sep 17 00:00:00 2001 From: magicmilo Date: Wed, 27 Oct 2021 14:14:21 +0100 Subject: [PATCH 08/12] replace tenant to person_1 in examples and required of API Json --- docs/api/DLUHC-CORE-Data.v1.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index a62d1f184..81081b52c 100644 --- a/docs/api/DLUHC-CORE-Data.v1.json +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -109,7 +109,7 @@ "Invalid Age": { "value": { "errors": { - "tenant_age": [ + "person_1_age": [ "Tenant age must be between 0 and 120" ] } @@ -219,7 +219,7 @@ "reasonable_preference_reason": [ "If reasonable preference is Yes, a reason must be given" ], - "tenant_age": [ + "person_1_age": [ "Tenant age must be between 0 and 120" ] } @@ -268,8 +268,8 @@ "x-examples": { "example-1": { "tenant_code": "T657", - "tenant_age": 35, - "tenant_gender": "Female", + "person_1_age": 35, + "person_1_gender": "Female", "tenant_ethnic_group": "White: English/Scottish/Welsh/Northern Irish/British", "tenant_nationality": "UK national resident in UK", "previous_housing_situation": "Private sector tenancy", From 512e3a5a82e949c185db336336df970f8cccef87 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Wed, 27 Oct 2021 14:21:05 +0100 Subject: [PATCH 09/12] added required armed forces fields to example --- docs/api/DLUHC-CORE-Data.v1.json | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index 64d5ca517..35a028ace 100644 --- a/docs/api/DLUHC-CORE-Data.v1.json +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -1207,6 +1207,17 @@ ] } }, - "securitySchemes": {} + "securitySchemes": { + "API Key - 1": { + "name": "API Key", + "type": "apiKey", + "in": "query" + }, + "API Key - 2": { + "name": "API Key", + "type": "apiKey", + "in": "query" + } + } } -} +} \ No newline at end of file From 073ccfe633b668d162f994f5dc65b0c7e35c72c0 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Wed, 27 Oct 2021 14:47:10 +0100 Subject: [PATCH 10/12] rubocop --- Gemfile | 2 +- app/models/case_log.rb | 2 +- spec/models/case_log_spec.rb | 8 ++++---- spec/rails_helper.rb | 3 ++- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Gemfile b/Gemfile index 0084b7546..c249007c2 100644 --- a/Gemfile +++ b/Gemfile @@ -49,10 +49,10 @@ end group :test do gem "capybara", require: false + gem "database_cleaner-active_record", require: false gem "factory_bot_rails" gem "selenium-webdriver", require: false gem "simplecov", require: false - gem "database_cleaner-active_record", require: false %w[rspec-core rspec-expectations rspec-mocks rspec-rails rspec-support].each do |lib| gem lib, git: "https://github.com/rspec/#{lib}.git", branch: "main", require: false end diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 556013773..2c818c736 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -54,7 +54,7 @@ class CaseLogValidator < ActiveModel::Validator record.errors.add :armed_forces_injured, "You must not answer the armed forces injury question if the tenant has not served in the armed forces or prefer not to say was chosen" end end - + EMPLOYED_STATUSES = ["Full-time - 30 hours or more", "Part-time - Less than 30 hours"].freeze def validate_net_income_uc_proportion(record) (1..8).any? do |n| diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index cad16431f..e0d03ed53 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -115,7 +115,7 @@ RSpec.describe Form, type: :model do }.to raise_error(ActiveRecord::RecordInvalid) end end - + context "fixed term tenancy length" do it "Must not be completed if Type of main tenancy is not responded with either Secure or Assured shorthold " do expect { @@ -163,7 +163,7 @@ RSpec.describe Form, type: :model do }.not_to raise_error end end - + context "armed forces active validation" do it "must be answered if ever served in the forces as a regular" do expect { @@ -176,7 +176,7 @@ RSpec.describe Form, type: :model do expect { CaseLog.create!(armed_forces: "No", armed_forces_active: "Yes") - }.to raise_error + }.to raise_error(ActiveRecord::RecordInvalid) end # Crossover over tests here as injured must be answered as well for no error @@ -185,7 +185,7 @@ RSpec.describe Form, type: :model do CaseLog.create!(armed_forces: "Yes - a regular", armed_forces_active: "Yes", armed_forces_injured: "Yes") - }.not_to raise_error + }.not_to raise_error end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index b873a5929..7a6a17442 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -6,7 +6,7 @@ require File.expand_path("../config/environment", __dir__) abort("The Rails environment is running in production mode!") if Rails.env.production? require "rspec/rails" require "capybara/rspec" -require 'database_cleaner/active_record' +require "database_cleaner/active_record" # Comment to run `js: true specs` with visible browser interaction Capybara.javascript_driver = :selenium_headless @@ -58,6 +58,7 @@ RSpec.configure do |config| uncommitted transaction data setup over the spec's database connection. MSG end + DatabaseCleaner.clean_with(:truncation) end From fba24747474fc2e9d110e70666a81a6c046f3891 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Wed, 27 Oct 2021 14:51:39 +0100 Subject: [PATCH 11/12] rubocop --- app/models/case_log.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 1446c967c..7ec3ae1bc 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -63,7 +63,7 @@ class CaseLogValidator < ActiveModel::Validator record.errors.add :outstanding_amount, "You must not answer the oustanding amout question if you don't have outstanding rent or charges." end end - + EMPLOYED_STATUSES = ["Full-time - 30 hours or more", "Part-time - Less than 30 hours"].freeze def validate_net_income_uc_proportion(record) (1..8).any? do |n| From 055cbf8fc5a96efd91d67c3ab48313fb977631e0 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 28 Oct 2021 09:15:45 +0100 Subject: [PATCH 12/12] CLDC-464: Other tenancy type validation (#68) * Add other_tenancy_type conditional field * Extract private validdate_other_field_method * Implement other tenancy type validation * Add missing migration file and update API doc --- app/models/case_log.rb | 28 ++++++++++++++----- config/forms/2021_2022.json | 8 ++++++ ...1027123535_add_other_tenancy_type_field.rb | 7 +++++ db/schema.rb | 3 +- docs/api/DLUHC-CORE-Data.v1.json | 7 ++++- spec/models/case_log_spec.rb | 27 +++++++++++++++++- 6 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20211027123535_add_other_tenancy_type_field.rb diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 7ec3ae1bc..8d912354b 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -30,13 +30,7 @@ class CaseLogValidator < ActiveModel::Validator end def validate_other_reason_for_leaving_last_settled_home(record) - if record.reason_for_leaving_last_settled_home == "Other" && record.other_reason_for_leaving_last_settled_home.blank? - record.errors.add :other_reason_for_leaving_last_settled_home, "If reason for leaving settled home is other then the other reason must be provided" - end - - if record.reason_for_leaving_last_settled_home != "Other" && record.other_reason_for_leaving_last_settled_home.present? - record.errors.add :other_reason_for_leaving_last_settled_home, "The other reason must not be provided if the reason for leaving settled home was not other" - end + validate_other_field(record, "reason_for_leaving_last_settled_home", "other_reason_for_leaving_last_settled_home") end def validate_reason_for_leaving_last_settled_home(record) @@ -107,6 +101,10 @@ class CaseLogValidator < ActiveModel::Validator conditions.each { |condition| condition[:condition] ? (record.errors.add :fixed_term_tenancy, condition[:error]) : nil } end + def validate_other_tenancy_type(record) + validate_other_field(record, "tenancy_type", "other_tenancy_type") + end + def validate(record) # If we've come from the form UI we only want to validate the specific fields # that have just been submitted. If we're submitting a log via API or Bulk Upload @@ -126,6 +124,18 @@ class CaseLogValidator < ActiveModel::Validator private + def validate_other_field(record, main_field, other_field) + main_field_label = main_field.humanize(capitalize: false) + other_field_label = other_field.humanize(capitalize: false) + if record[main_field] == "Other" && record[other_field].blank? + record.errors.add other_field.to_sym, "If #{main_field_label} is other then #{other_field_label} must be provided" + end + + if record[main_field] != "Other" && record[other_field].present? + record.errors.add other_field.to_sym, "#{other_field_label} must not be provided if #{main_field_label} was not other" + end + end + def women_of_child_bearing_age_in_household(record) (1..8).any? do |n| next if record["person_#{n}_gender"].nil? || record["person_#{n}_age"].nil? @@ -209,6 +219,10 @@ private dynamically_not_required << "fixed_term_tenancy" end + if tenancy_type != "Other" + dynamically_not_required << "other_tenancy_type" + end + required.delete_if { |key, _value| dynamically_not_required.include?(key) } end end diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index ed99d51cf..f4cf447c3 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -903,7 +903,15 @@ "3": "Lifetime – Assured", "4": "License agreement", "5": "Other" + }, + "conditional_for": { + "other_tenancy_type": ["Other"] } + }, + "other_tenancy_type": { + "header": "Please state the tenancy type", + "hint_text": "", + "type": "text" } } }, diff --git a/db/migrate/20211027123535_add_other_tenancy_type_field.rb b/db/migrate/20211027123535_add_other_tenancy_type_field.rb new file mode 100644 index 000000000..38b8106c6 --- /dev/null +++ b/db/migrate/20211027123535_add_other_tenancy_type_field.rb @@ -0,0 +1,7 @@ +class AddOtherTenancyTypeField < ActiveRecord::Migration[6.1] + def change + change_table :case_logs, bulk: true do |t| + t.column :other_tenancy_type, :string + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 8d2405443..346f047c2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_10_27_091521) do +ActiveRecord::Schema.define(version: 2021_10_27_123535) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -132,6 +132,7 @@ ActiveRecord::Schema.define(version: 2021_10_27_091521) do t.boolean "reasonable_preference_reason_avoid_hardship" t.boolean "reasonable_preference_reason_do_not_know" t.datetime "discarded_at" + t.string "other_tenancy_type" t.index ["discarded_at"], name: "index_case_logs_on_discarded_at" end diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index e95b730ca..7a4d495e2 100644 --- a/docs/api/DLUHC-CORE-Data.v1.json +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -1089,6 +1089,10 @@ }, "reasonable_preference_reason_do_not_know": { "type": "boolean" + }, + "other_tenancy-type": { + "type": "string", + "example": "private tenancy" } }, "required": [ @@ -1203,7 +1207,8 @@ "reasonable_preference_reason_unsatisfactory_housing", "reasonable_preference_reason_medical_grounds", "reasonable_preference_reason_avoid_hardship", - "reasonable_preference_reason_do_not_know" + "reasonable_preference_reason_do_not_know", + "other_tenancy-type" ] } }, diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 0b8d7fa1f..d95f000a7 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -201,7 +201,32 @@ RSpec.describe Form, type: :model do expect { CaseLog.create!(armed_forces: "Yes - a regular", armed_forces_active: "Yes", - armed_forces_injured: "Yes") + armed_forces_injured: "Yes")} + end + end + + context "other tenancy type validation" do + it "must be provided if tenancy type was given as other" do + expect { + CaseLog.create!(tenancy_type: "Other", + other_tenancy_type: nil) + }.to raise_error(ActiveRecord::RecordInvalid) + + expect { + CaseLog.create!(tenancy_type: "Other", + other_tenancy_type: "type") + }.not_to raise_error + end + + it "must not be provided if tenancy type is not other" do + expect { + CaseLog.create!(tenancy_type: "Fixed", + other_tenancy_type: "the other reason provided") + }.to raise_error(ActiveRecord::RecordInvalid) + + expect { + CaseLog.create!(tenancy_type: "Fixed", + other_tenancy_type: nil) }.not_to raise_error end end