From 9b4c9ed98e2f4bb95ad1b234564b584293b2c648 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Mon, 25 Oct 2021 16:24:43 +0100 Subject: [PATCH 01/14] basic pregnancy validations --- app/models/case_log.rb | 25 +++++++++++++++++++++++++ spec/features/case_log_spec.rb | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 1f8743b9f..4bc71db3f 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -49,6 +49,31 @@ class CaseLogValidator < ActiveModel::Validator 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" + end + end + + def women_of_child_bearing_age_in_household(record) + unless record.tenant_gender.nil? || record.tenant_age.nil? + if record.tenant_gender == "Female" && record.tenant_age >= 16 && record.tenant_age <= 50 + return true + end + end + + p = 2 + while p <= 8 + unless record["person_#{p}_gender"].nil? || record["person_#{p}_age"].nil? + if record["person_#{p}_gender"] == "Female" && record["person_#{p}_age"] >= 16 && record["person_#{p}_age"] <= 50 + return true + end + end + p += 1 + end + return false + 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/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 3e16f8530..e915e97a6 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -93,6 +93,38 @@ RSpec.describe "Test Features" do ) end + context "Validate pregnancy questions" do + it "Cannot answer yes if no female tenants" do + expect { + CaseLog.create!(pregnancy: "Yes", tenant_gender: "Male", tenant_age: 20) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "Cannot answer yes if no female tenants within age range" do + expect { + CaseLog.create!(pregnancy: "Yes", tenant_gender: "Female", tenant_age: 51) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "Cannot answer prefer not to say if no valid tenants" do + expect { + CaseLog.create!(pregnancy: "Prefer not to say", tenant_gender: "Male", tenant_age: 20) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + # it "Can answer yes if valid tenants" do + # expect { + # CaseLog.create!(pregnancy: "Yes", tenant_gender: "Female", tenant_age: 20) + # }.to raise_error(ActiveRecord::RecordInvalid) + # end + + # it "Can answer yes if valid second tenant" do + # expect { + # CaseLog.create!(pregnancy: "Yes", tenant_gender: "Male", tenant_age: 99, person_2_gender: "Female", person_2_age: 20) + # }.to raise_error(ActiveRecord::RecordInvalid) + # end + end + it "can be accessed by url" do visit("/case_logs/#{id}/tenant_age") expect(page).to have_field("case-log-tenant-age-field") From e2041cec9c28a64c5ff4a8cd9c594b8bc38d60eb Mon Sep 17 00:00:00 2001 From: magicmilo Date: Mon, 25 Oct 2021 16:30:20 +0100 Subject: [PATCH 02/14] add valid tests --- app/models/case_log.rb | 1 - spec/features/case_log_spec.rb | 22 +++++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 4bc71db3f..43946e75e 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -71,7 +71,6 @@ class CaseLogValidator < ActiveModel::Validator end p += 1 end - return false end def validate(record) diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index e915e97a6..8d1435885 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -112,17 +112,17 @@ RSpec.describe "Test Features" do }.to raise_error(ActiveRecord::RecordInvalid) end - # it "Can answer yes if valid tenants" do - # expect { - # CaseLog.create!(pregnancy: "Yes", tenant_gender: "Female", tenant_age: 20) - # }.to raise_error(ActiveRecord::RecordInvalid) - # end - - # it "Can answer yes if valid second tenant" do - # expect { - # CaseLog.create!(pregnancy: "Yes", tenant_gender: "Male", tenant_age: 99, person_2_gender: "Female", person_2_age: 20) - # }.to raise_error(ActiveRecord::RecordInvalid) - # end + it "Can answer yes if valid tenants" do + expect { + CaseLog.create!(pregnancy: "Yes", tenant_gender: "Female", tenant_age: 20) + }.not_to raise_error(ActiveRecord::RecordInvalid) + end + + it "Can answer yes if valid second tenant" do + expect { + CaseLog.create!(pregnancy: "Yes", tenant_gender: "Male", tenant_age: 99, person_2_gender: "Female", person_2_age: 20) + }.not_to raise_error(ActiveRecord::RecordInvalid) + end end it "can be accessed by url" do From 12be4612ec4794d4f6cd4ff994d5778bf10fc180 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Mon, 25 Oct 2021 16:32:21 +0100 Subject: [PATCH 03/14] prettier --- spec/features/case_log_spec.rb | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 8d1435885..bdbcee847 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -96,31 +96,42 @@ RSpec.describe "Test Features" do context "Validate pregnancy questions" do it "Cannot answer yes if no female tenants" do expect { - CaseLog.create!(pregnancy: "Yes", tenant_gender: "Male", tenant_age: 20) + CaseLog.create!(pregnancy: "Yes", + tenant_gender: "Male", + tenant_age: 20) }.to raise_error(ActiveRecord::RecordInvalid) end it "Cannot answer yes if no female tenants within age range" do expect { - CaseLog.create!(pregnancy: "Yes", tenant_gender: "Female", tenant_age: 51) + CaseLog.create!(pregnancy: "Yes", + tenant_gender: "Female", + tenant_age: 51) }.to raise_error(ActiveRecord::RecordInvalid) end it "Cannot answer prefer not to say if no valid tenants" do expect { - CaseLog.create!(pregnancy: "Prefer not to say", tenant_gender: "Male", tenant_age: 20) + CaseLog.create!(pregnancy: "Prefer not to say", + tenant_gender: "Male", + tenant_age: 20) }.to raise_error(ActiveRecord::RecordInvalid) end it "Can answer yes if valid tenants" do expect { - CaseLog.create!(pregnancy: "Yes", tenant_gender: "Female", tenant_age: 20) + CaseLog.create!(pregnancy: "Yes", + tenant_gender: "Female", + tenant_age: 20) }.not_to raise_error(ActiveRecord::RecordInvalid) end it "Can answer yes if valid second tenant" do expect { - CaseLog.create!(pregnancy: "Yes", tenant_gender: "Male", tenant_age: 99, person_2_gender: "Female", person_2_age: 20) + CaseLog.create!(pregnancy: "Yes", + tenant_gender: "Male", tenant_age: 99, + person_2_gender: "Female", + person_2_age: 20) }.not_to raise_error(ActiveRecord::RecordInvalid) end end From 988b87ac4c9e73e5f35b159ddfaee4af4febaed7 Mon Sep 17 00:00:00 2001 From: Milo Date: Tue, 26 Oct 2021 12:53:11 +0100 Subject: [PATCH 04/14] Update app/models/case_log.rb Remove unnecessary if Co-authored-by: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> --- 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 43946e75e..7b8879026 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -57,7 +57,7 @@ class CaseLogValidator < ActiveModel::Validator def women_of_child_bearing_age_in_household(record) unless record.tenant_gender.nil? || record.tenant_age.nil? - if record.tenant_gender == "Female" && record.tenant_age >= 16 && record.tenant_age <= 50 + record.tenant_gender == "Female" && record.tenant_age >= 16 && record.tenant_age <= 50 return true end end From 8d3e31ee10e431c6bef73e530f6a1381dd2c81dd Mon Sep 17 00:00:00 2001 From: Milo Date: Tue, 26 Oct 2021 12:53:40 +0100 Subject: [PATCH 05/14] Update app/models/case_log.rb Use map rather than indexing Co-authored-by: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> --- app/models/case_log.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 7b8879026..cfdfca635 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -62,7 +62,11 @@ class CaseLogValidator < ActiveModel::Validator end end - p = 2 +(2..8).map do |n| + next if record["person_#{n}_gender"].nil? || record["person_#{n}_age"].nil? + + record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 +end while p <= 8 unless record["person_#{p}_gender"].nil? || record["person_#{p}_age"].nil? if record["person_#{p}_gender"] == "Female" && record["person_#{p}_age"] >= 16 && record["person_#{p}_age"] <= 50 From 357aea91c7a0489e0d30ed557427d65c94c1ca87 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Tue, 26 Oct 2021 13:25:37 +0100 Subject: [PATCH 06/14] clean up validation --- app/models/case_log.rb | 19 ++++--------------- spec/features/case_log_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index cfdfca635..c294c361c 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -57,23 +57,12 @@ class CaseLogValidator < ActiveModel::Validator def women_of_child_bearing_age_in_household(record) unless record.tenant_gender.nil? || record.tenant_age.nil? - record.tenant_gender == "Female" && record.tenant_age >= 16 && record.tenant_age <= 50 - return true - end + return record.tenant_gender == "Female" && (record.tenant_age >= 16 && record.tenant_age <= 50) end -(2..8).map do |n| - next if record["person_#{n}_gender"].nil? || record["person_#{n}_age"].nil? - - record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 -end - while p <= 8 - unless record["person_#{p}_gender"].nil? || record["person_#{p}_age"].nil? - if record["person_#{p}_gender"] == "Female" && record["person_#{p}_age"] >= 16 && record["person_#{p}_age"] <= 50 - return true - end - end - p += 1 + (2..8).any? do |n| + next if record["person_#{n}_gender"].nil? || record["person_#{n}_age"].nil? + record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 end end diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index bdbcee847..8984283a2 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -123,7 +123,7 @@ RSpec.describe "Test Features" do CaseLog.create!(pregnancy: "Yes", tenant_gender: "Female", tenant_age: 20) - }.not_to raise_error(ActiveRecord::RecordInvalid) + }.not_to raise_error end it "Can answer yes if valid second tenant" do @@ -132,7 +132,7 @@ RSpec.describe "Test Features" do tenant_gender: "Male", tenant_age: 99, person_2_gender: "Female", person_2_age: 20) - }.not_to raise_error(ActiveRecord::RecordInvalid) + }.not_to raise_error end end From 248c67bcb3f81b452cea0d3d0535e63dcb9dddf5 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Tue, 26 Oct 2021 13:56:37 +0100 Subject: [PATCH 07/14] Renamed tenant to person_1 everywhere --- app/models/case_log.rb | 12 ++-- config/forms/2021_2022.json | 8 +-- ...1026123542_change_to_person1_gender_age.rb | 8 +++ db/schema.rb | 15 +++-- docs/api/DLUHC-CORE-Data.v1.json | 16 ++--- spec/factories/case_log.rb | 2 +- spec/features/case_log_spec.rb | 62 +++++++++---------- spec/fixtures/complete_case_log.json | 4 +- spec/fixtures/forms/test_form.json | 10 +-- spec/helpers/check_answers_helper_spec.rb | 2 +- spec/models/case_log_spec.rb | 6 +- spec/models/form_spec.rb | 6 +- spec/requests/case_log_controller_spec.rb | 14 ++--- 13 files changed, 88 insertions(+), 77 deletions(-) create mode 100644 db/migrate/20211026123542_change_to_person1_gender_age.rb diff --git a/app/models/case_log.rb b/app/models/case_log.rb index c294c361c..2a1e4d7d2 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -3,9 +3,9 @@ class CaseLogValidator < ActiveModel::Validator # followed by field name this is how the metaprogramming of the method # name being call in the validate method works. - def validate_tenant_age(record) - if record.tenant_age && !/^[1-9][0-9]?$|^120$/.match?(record.tenant_age.to_s) - record.errors.add :tenant_age, "Tenant age must be between 0 and 120" + def validate_person_1_age(record) + if record.person_1_age && !/^[1-9][0-9]?$|^120$/.match?(record.person_1_age.to_s) + record.errors.add :person_1_age, "Tenant age must be between 0 and 120" end end @@ -56,11 +56,7 @@ class CaseLogValidator < ActiveModel::Validator end def women_of_child_bearing_age_in_household(record) - unless record.tenant_gender.nil? || record.tenant_age.nil? - return record.tenant_gender == "Female" && (record.tenant_age >= 16 && record.tenant_age <= 50) - end - - (2..8).any? do |n| + (1..8).any? do |n| next if record["person_#{n}_gender"].nil? || record["person_#{n}_age"].nil? record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 end diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 47378e9f0..84dc00f74 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -21,11 +21,11 @@ } } }, - "tenant_age": { + "person_1_age": { "header": "", "description": "", "questions": { - "tenant_age": { + "person_1_age": { "check_answer_label": "Tenant's age", "header": "What is the tenant's age?", "hint_text": "", @@ -36,11 +36,11 @@ } } }, - "tenant_gender": { + "person_1_gender": { "header": "", "description": "", "questions": { - "tenant_gender": { + "person_1_gender": { "check_answer_label": "Tenant's gender", "header": "Which of these best describes the tenant's gender identity?", "hint_text": "", diff --git a/db/migrate/20211026123542_change_to_person1_gender_age.rb b/db/migrate/20211026123542_change_to_person1_gender_age.rb new file mode 100644 index 000000000..1d0e90e17 --- /dev/null +++ b/db/migrate/20211026123542_change_to_person1_gender_age.rb @@ -0,0 +1,8 @@ +class ChangeToPerson1GenderAge < ActiveRecord::Migration[6.1] + def change + change_table :case_logs, bulk: true do |t| + t.rename :tenant_age, :person_1_age + t.rename :tenant_gender, :person_1_gender + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 20dbfc383..5386babb2 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_15_090040) do +ActiveRecord::Schema.define(version: 2021_10_26_123542) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -20,8 +20,8 @@ ActiveRecord::Schema.define(version: 2021_10_15_090040) do t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.string "tenant_code" - t.integer "tenant_age" - t.string "tenant_gender" + t.integer "person_1_age" + t.string "person_1_gender" t.string "tenant_ethnic_group" t.string "tenant_nationality" t.string "previous_housing_situation" @@ -84,7 +84,6 @@ ActiveRecord::Schema.define(version: 2021_10_15_090040) do t.string "property_void_date" t.string "property_major_repairs" t.string "property_major_repairs_date" - t.integer "property_number_of_times_relet" t.string "property_wheelchair_accessible" t.string "net_income" t.string "net_income_frequency" @@ -106,6 +105,8 @@ ActiveRecord::Schema.define(version: 2021_10_15_090040) do t.string "cbl_letting" t.string "chr_letting" t.string "cap_letting" + t.string "conditional_page_option" + t.string "continue_option" t.string "outstanding_rent_or_charges" t.string "other_reason_for_leaving_last_settled_home" t.boolean "accessibility_requirements_fully_wheelchair_accessible_housing" @@ -132,6 +133,12 @@ ActiveRecord::Schema.define(version: 2021_10_15_090040) do t.boolean "reasonable_preference_reason_avoid_hardship" t.boolean "reasonable_preference_reason_do_not_know" t.datetime "discarded_at" + t.integer "property_number_of_times_relet" + t.string "is_postcode_known" + t.string "select_your_la" + t.string "why_is_la_unkown" + t.string "do_you_know_la" + t.string "is_property_a_relet" 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 015e98a07..da88622ae 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", @@ -388,13 +388,13 @@ "type": "string", "minLength": 1 }, - "tenant_age": { + "person_1_age": { "type": "number", "description": "The age of the lead tenant", "maximum": 120, "minimum": 0 }, - "tenant_gender": { + "person_1_gender": { "type": "string", "minLength": 1, "enum": [ @@ -1042,8 +1042,8 @@ }, "required": [ "tenant_code", - "tenant_age", - "tenant_gender", + "person_1_age", + "person_1_gender", "tenant_ethnic_group", "tenant_nationality", "previous_housing_situation", diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index 044a4cbac..6e5960329 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -6,7 +6,7 @@ FactoryBot.define do tenant_code { "TH356" } property_postcode { "SW2 6HI" } previous_postcode { "P0 5ST" } - tenant_age { "12" } + person_1_age { "12" } end trait :completed do status { 2 } diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 8984283a2..6bd4d14b0 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -7,8 +7,8 @@ RSpec.describe "Test Features" do question_answers = { tenant_code: { type: "text", answer: "BZ737" }, - tenant_age: { type: "numeric", answer: 25 }, - tenant_gender: { type: "radio", answer: "Female" }, + person_1_age: { type: "numeric", answer: 25 }, + person_1_gender: { type: "radio", answer: "Female" }, household_number_of_other_members: { type: "numeric", answer: 2 }, } @@ -97,39 +97,39 @@ RSpec.describe "Test Features" do it "Cannot answer yes if no female tenants" do expect { CaseLog.create!(pregnancy: "Yes", - tenant_gender: "Male", - tenant_age: 20) + person_1_gender: "Male", + person_1_age: 20) }.to raise_error(ActiveRecord::RecordInvalid) end it "Cannot answer yes if no female tenants within age range" do expect { CaseLog.create!(pregnancy: "Yes", - tenant_gender: "Female", - tenant_age: 51) + person_1_gender: "Female", + person_1_age: 51) }.to raise_error(ActiveRecord::RecordInvalid) end it "Cannot answer prefer not to say if no valid tenants" do expect { CaseLog.create!(pregnancy: "Prefer not to say", - tenant_gender: "Male", - tenant_age: 20) + person_1_gender: "Male", + person_1_age: 20) }.to raise_error(ActiveRecord::RecordInvalid) end it "Can answer yes if valid tenants" do expect { CaseLog.create!(pregnancy: "Yes", - tenant_gender: "Female", - tenant_age: 20) + person_1_gender: "Female", + person_1_age: 20) }.not_to raise_error end it "Can answer yes if valid second tenant" do expect { CaseLog.create!(pregnancy: "Yes", - tenant_gender: "Male", tenant_age: 99, + person_1_gender: "Male", person_1_age: 99, person_2_gender: "Female", person_2_age: 20) }.not_to raise_error @@ -137,8 +137,8 @@ RSpec.describe "Test Features" do end it "can be accessed by url" do - visit("/case_logs/#{id}/tenant_age") - expect(page).to have_field("case-log-tenant-age-field") + visit("/case_logs/#{id}/person_1_age") + expect(page).to have_field("case-log-person-1-age-field") end it "updates model attributes correctly for each question" do @@ -183,8 +183,8 @@ RSpec.describe "Test Features" do end it "displays text answers in inputs if they are already saved" do - visit("/case_logs/#{id}/tenant_age") - expect(page).to have_field("case-log-tenant-age-field", with: "12") + visit("/case_logs/#{id}/person_1_age") + expect(page).to have_field("case-log-person-1-age-field", with: "12") end it "displays checkbox answers in inputs if they are already saved" do @@ -216,7 +216,7 @@ RSpec.describe "Test Features" do it "go back to tenant code page from tenant age page", js: true do visit("/case_logs/#{id}/tenant_code") click_button("Save and continue") - visit("/case_logs/#{id}/tenant_age") + visit("/case_logs/#{id}/person_1_age") click_link(text: "Back") expect(page).to have_field("case-log-tenant-code-field") end @@ -261,8 +261,8 @@ RSpec.describe "Test Features" do end it "should display answers given by the user for the question in the subsection" do - fill_in_number_question(empty_case_log.id, "tenant_age", 28) - choose("case-log-tenant-gender-non-binary-field") + fill_in_number_question(empty_case_log.id, "person_1_age", 28) + choose("case-log-person-1-gender-non-binary-field") click_button("Save and continue") visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") expect(page).to have_content("28") @@ -273,15 +273,15 @@ RSpec.describe "Test Features" do visit("case_logs/#{empty_case_log.id}/#{subsection}/check_answers") assert_selector "a", text: /Answer\z/, count: 4 assert_selector "a", text: "Change", count: 0 - expect(page).to have_link("Answer", href: "/case_logs/#{empty_case_log.id}/tenant_age") + expect(page).to have_link("Answer", href: "/case_logs/#{empty_case_log.id}/person_1_age") end it "should have a change link for answered questions" do - fill_in_number_question(empty_case_log.id, "tenant_age", 28) + fill_in_number_question(empty_case_log.id, "person_1_age", 28) visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") assert_selector "a", text: /Answer\z/, count: 3 assert_selector "a", text: "Change", count: 1 - expect(page).to have_link("Change", href: "/case_logs/#{empty_case_log.id}/tenant_age") + expect(page).to have_link("Change", href: "/case_logs/#{empty_case_log.id}/person_1_age") end it "should have a link pointing to the first question if no questions are answered" do @@ -360,19 +360,19 @@ RSpec.describe "Test Features" do describe "Question validation" do context "given an invalid tenant age" do it " of less than 0 it shows validation" do - visit("/case_logs/#{id}/tenant_age") - fill_in_number_question(empty_case_log.id, "tenant_age", -5) + visit("/case_logs/#{id}/person_1_age") + fill_in_number_question(empty_case_log.id, "person_1_age", -5) expect(page).to have_selector("#error-summary-title") - expect(page).to have_selector("#case-log-tenant-age-error") - expect(page).to have_selector("#case-log-tenant-age-field-error") + expect(page).to have_selector("#case-log-person-1-age-error") + expect(page).to have_selector("#case-log-person-1-age-field-error") end it " of greater than 120 it shows validation" do - visit("/case_logs/#{id}/tenant_age") - fill_in_number_question(empty_case_log.id, "tenant_age", 121) + visit("/case_logs/#{id}/person_1_age") + fill_in_number_question(empty_case_log.id, "person_1_age", 121) expect(page).to have_selector("#error-summary-title") - expect(page).to have_selector("#case-log-tenant-age-error") - expect(page).to have_selector("#case-log-tenant-age-field-error") + expect(page).to have_selector("#case-log-person-1-age-error") + expect(page).to have_selector("#case-log-person-1-age-field-error") end end end @@ -406,8 +406,8 @@ RSpec.describe "Test Features" do end it "can route based on multiple conditions" do - visit("/case_logs/#{id}/tenant_gender") - choose("case-log-tenant-gender-female-field", allow_label_click: true) + visit("/case_logs/#{id}/person_1_gender") + choose("case-log-person-1-gender-female-field", allow_label_click: true) click_button("Save and continue") visit("/case_logs/#{id}/conditional_question") choose("case-log-pregnancy-yes-field", allow_label_click: true) diff --git a/spec/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json index 195de85f5..8a539e91c 100644 --- a/spec/fixtures/complete_case_log.json +++ b/spec/fixtures/complete_case_log.json @@ -2,8 +2,8 @@ "case_log": { "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", diff --git a/spec/fixtures/forms/test_form.json b/spec/fixtures/forms/test_form.json index f5bd9cb48..35590cb3e 100644 --- a/spec/fixtures/forms/test_form.json +++ b/spec/fixtures/forms/test_form.json @@ -16,9 +16,9 @@ } } }, - "tenant_age": { + "person_1_age": { "questions": { - "tenant_age": { + "person_1_age": { "check_answer_label": "Tenant's age", "header": "What is the tenant's age?", "type": "numeric", @@ -28,9 +28,9 @@ } } }, - "tenant_gender": { + "person_1_gender": { "questions": { - "tenant_gender": { + "person_1_gender": { "check_answer_label": "Tenant's gender", "header": "Which of these best describes the tenant's gender identity?", "type": "radio", @@ -252,7 +252,7 @@ } }, "conditional_route_to": { - "rent": { "pregnancy": "Yes", "tenant_gender": "Female" }, + "rent": { "pregnancy": "Yes", "person_1_gender": "Female" }, "conditional_question_yes_page": { "pregnancy": "Yes" }, "conditional_question_no_page": { "pregnancy": "No" } }, diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb index cc619a041..d13f12e27 100644 --- a/spec/helpers/check_answers_helper_spec.rb +++ b/spec/helpers/check_answers_helper_spec.rb @@ -153,7 +153,7 @@ RSpec.describe CheckAnswersHelper do it "it includes conditional pages and questions that were displayed" do case_log["pregnancy"] = "Yes" - case_log["tenant_gender"] = "Female" + case_log["person_1_gender"] = "Female" result = total_questions(conditional_routing_subsection, case_log, form) expected_keys = %w[pregnancy] expect(result.keys).to match_array(expected_keys) diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 927b153d1..c9463e3ec 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -3,15 +3,15 @@ require "rails_helper" RSpec.describe Form, type: :model do describe "#new" do it "validates age is a number" do - expect { CaseLog.create!(tenant_age: "random") }.to raise_error(ActiveRecord::RecordInvalid) + expect { CaseLog.create!(person_1_age: "random") }.to raise_error(ActiveRecord::RecordInvalid) end it "validates age is under 120" do - expect { CaseLog.create!(tenant_age: 121) }.to raise_error(ActiveRecord::RecordInvalid) + expect { CaseLog.create!(person_1_age: 121) }.to raise_error(ActiveRecord::RecordInvalid) end it "validates age is over 0" do - expect { CaseLog.create!(tenant_age: 0) }.to raise_error(ActiveRecord::RecordInvalid) + expect { CaseLog.create!(person_1_age: 0) }.to raise_error(ActiveRecord::RecordInvalid) end it "validates number of relets is a number" do diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index b26e2014c..f11431c8b 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -5,9 +5,9 @@ RSpec.describe Form, type: :model do let(:form) { form_handler.get_form("test_form") } describe ".next_page" do - let(:previous_page) { "tenant_age" } + let(:previous_page) { "person_1_age" } it "returns the next page given the previous" do - expect(form.next_page(previous_page)).to eq("tenant_gender") + expect(form.next_page(previous_page)).to eq("person_1_gender") end end @@ -20,7 +20,7 @@ RSpec.describe Form, type: :model do describe ".previous_page" do context "given a page in the middle of a subsection" do - let(:current_page) { "tenant_age" } + let(:current_page) { "person_1_age" } it "returns the previous page given the current" do expect(form.previous_page(current_page)).to eq("tenant_code") end diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index 89a5592b4..e6c866c90 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -24,7 +24,7 @@ RSpec.describe CaseLogsController, type: :request do describe "POST #create" do let(:tenant_code) { "T365" } - let(:tenant_age) { 35 } + let(:person_1_age) { 35 } let(:property_number_of_times_relet) { 12 } let(:property_postcode) { "SE11 6TY" } let(:in_progress) { "in_progress" } @@ -33,7 +33,7 @@ RSpec.describe CaseLogsController, type: :request do let(:params) do { "tenant_code": tenant_code, - "tenant_age": tenant_age, + "person_1_age": person_1_age, "property_postcode": property_postcode, "property_number_of_times_relet": property_number_of_times_relet, } @@ -55,18 +55,18 @@ RSpec.describe CaseLogsController, type: :request do it "creates a case log with the values passed" do json_response = JSON.parse(response.body) expect(json_response["tenant_code"]).to eq(tenant_code) - expect(json_response["tenant_age"]).to eq(tenant_age) + expect(json_response["person_1_age"]).to eq(person_1_age) expect(json_response["property_postcode"]).to eq(property_postcode) end context "invalid json params" do - let(:tenant_age) { 2000 } + let(:person_1_age) { 2000 } let(:property_number_of_times_relet) { 21 } it "validates case log parameters" do json_response = JSON.parse(response.body) expect(response).to have_http_status(:unprocessable_entity) - expect(json_response["errors"]).to match_array([["property_number_of_times_relet", ["Must be between 0 and 20"]], ["tenant_age", ["Tenant age must be between 0 and 120"]]]) + expect(json_response["errors"]).to match_array([["property_number_of_times_relet", ["Must be between 0 and 20"]], ["person_1_age", ["Tenant age must be between 0 and 120"]]]) end end @@ -149,7 +149,7 @@ RSpec.describe CaseLogsController, type: :request do end context "invalid case log params" do - let(:params) { { tenant_age: 200 } } + let(:params) { { person_1_age: 200 } } it "returns 422" do expect(response).to have_http_status(:unprocessable_entity) @@ -157,7 +157,7 @@ RSpec.describe CaseLogsController, type: :request do it "returns an error message" do json_response = JSON.parse(response.body) - expect(json_response["errors"]).to eq({ "tenant_age" => ["Tenant age must be between 0 and 120"] }) + expect(json_response["errors"]).to eq({ "person_1_age" => ["Tenant age must be between 0 and 120"] }) end end From 62ce94d8ccda6493fb062feefc093960b47198eb Mon Sep 17 00:00:00 2001 From: magicmilo Date: Tue, 26 Oct 2021 14:31:24 +0100 Subject: [PATCH 08/14] fix bad migration --- db/schema.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 5386babb2..2d4f30da1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -84,6 +84,7 @@ ActiveRecord::Schema.define(version: 2021_10_26_123542) do t.string "property_void_date" t.string "property_major_repairs" t.string "property_major_repairs_date" + t.integer "property_number_of_times_relet" t.string "property_wheelchair_accessible" t.string "net_income" t.string "net_income_frequency" @@ -105,8 +106,6 @@ ActiveRecord::Schema.define(version: 2021_10_26_123542) do t.string "cbl_letting" t.string "chr_letting" t.string "cap_letting" - t.string "conditional_page_option" - t.string "continue_option" t.string "outstanding_rent_or_charges" t.string "other_reason_for_leaving_last_settled_home" t.boolean "accessibility_requirements_fully_wheelchair_accessible_housing" @@ -133,12 +132,6 @@ ActiveRecord::Schema.define(version: 2021_10_26_123542) do t.boolean "reasonable_preference_reason_avoid_hardship" t.boolean "reasonable_preference_reason_do_not_know" t.datetime "discarded_at" - t.integer "property_number_of_times_relet" - t.string "is_postcode_known" - t.string "select_your_la" - t.string "why_is_la_unkown" - t.string "do_you_know_la" - t.string "is_property_a_relet" t.index ["discarded_at"], name: "index_case_logs_on_discarded_at" end From 88043a9386ba83777c1926c8f335063bad1cf1f1 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Tue, 26 Oct 2021 14:39:26 +0100 Subject: [PATCH 09/14] made validator private method --- app/models/case_log.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 2a1e4d7d2..baab481af 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -55,13 +55,6 @@ class CaseLogValidator < ActiveModel::Validator 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? - record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 - 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 @@ -78,6 +71,15 @@ class CaseLogValidator < ActiveModel::Validator validation_methods.each { |meth| public_send(meth, record) } end end + + private + + 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? + record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 + end + end end class CaseLog < ApplicationRecord From c6fb2580df1b856ccf0708b11d26c78f03cc8b6b Mon Sep 17 00:00:00 2001 From: magicmilo Date: Tue, 26 Oct 2021 16:10:25 +0100 Subject: [PATCH 10/14] rubocop --- app/helpers/check_answers_helper.rb | 3 --- app/models/case_log.rb | 5 +++-- spec/features/case_log_spec.rb | 22 +++++++++++----------- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 16f07032d..3e59de811 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -16,9 +16,6 @@ module CheckAnswersHelper while page_name.to_s != "check_answers" && subsection_keys.include?(page_name) questions = form.questions_for_page(page_name) - question_key = questions.keys[0] - question_value = questions.values[0] - applicable_questions = filter_conditional_questions(questions, case_log) total_questions = total_questions.merge(applicable_questions) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index baab481af..16c1915ed 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -72,12 +72,13 @@ class CaseLogValidator < ActiveModel::Validator end end - private +private 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? - record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 + + record["person_#{n}_gender"] == "Female" && record["person_#{n}_age"] >= 16 && record["person_#{n}_age"] <= 50 end end end diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 6bd4d14b0..587fc80f1 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -97,41 +97,41 @@ RSpec.describe "Test Features" do it "Cannot answer yes if no female tenants" do expect { CaseLog.create!(pregnancy: "Yes", - person_1_gender: "Male", - person_1_age: 20) + person_1_gender: "Male", + person_1_age: 20) }.to raise_error(ActiveRecord::RecordInvalid) end it "Cannot answer yes if no female tenants within age range" do expect { CaseLog.create!(pregnancy: "Yes", - person_1_gender: "Female", - person_1_age: 51) + person_1_gender: "Female", + person_1_age: 51) }.to raise_error(ActiveRecord::RecordInvalid) end it "Cannot answer prefer not to say if no valid tenants" do expect { CaseLog.create!(pregnancy: "Prefer not to say", - person_1_gender: "Male", - person_1_age: 20) + person_1_gender: "Male", + person_1_age: 20) }.to raise_error(ActiveRecord::RecordInvalid) end it "Can answer yes if valid tenants" do expect { CaseLog.create!(pregnancy: "Yes", - person_1_gender: "Female", - person_1_age: 20) + person_1_gender: "Female", + person_1_age: 20) }.not_to raise_error end it "Can answer yes if valid second tenant" do expect { CaseLog.create!(pregnancy: "Yes", - person_1_gender: "Male", person_1_age: 99, - person_2_gender: "Female", - person_2_age: 20) + person_1_gender: "Male", person_1_age: 99, + person_2_gender: "Female", + person_2_age: 20) }.not_to raise_error end end From 938c5b9807674749a98299244082e75fd89b2f5c Mon Sep 17 00:00:00 2001 From: magicmilo Date: Tue, 26 Oct 2021 16:23:31 +0100 Subject: [PATCH 11/14] updated age and gender field person_1 in docs --- docs/api/DLUHC-CORE-Data.v1.json | 60 +++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index da88622ae..a62d1f184 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": { - "person_1_age": [ + "tenant_age": [ "Tenant age must be between 0 and 120" ] } @@ -193,7 +193,7 @@ "summary": "Create New Case Log", "operationId": "post-caselog", "responses": { - "200": { + "201": { "description": "Case Log Created", "content": { "application/json": { @@ -219,7 +219,7 @@ "reasonable_preference_reason": [ "If reasonable preference is Yes, a reason must be given" ], - "person_1_age": [ + "tenant_age": [ "Tenant age must be between 0 and 120" ] } @@ -268,8 +268,8 @@ "x-examples": { "example-1": { "tenant_code": "T657", - "person_1_age": 35, - "person_1_gender": "Female", + "tenant_age": 35, + "tenant_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", @@ -781,11 +781,55 @@ }, "reason_for_leaving_last_settled_home": { "type": "string", - "minLength": 1 + "minLength": 1, + "enum": [ + "Permanently decanted from another property owned by this landlord", + "Left home country as a refugee", + "Loss of tied accommodation", + "Domestic abuse", + "(Non violent) relationship breakdown with partner", + "Asked to leave by family or friends", + "Racial harassment", + "Other problems with neighbours", + "Property unsuitable because of overcrowding", + "End of assured shorthold tenancy - no fault", + "End of assured shorthold tenancy - tenant's fault", + "End of fixed term tenancy - no fault", + "End of fixed term tenancy - tenant's fault", + "Repossession", + "Under occupation - offered incentive to downsize", + "Under occupation - no incentive", + "Property unsuitable because of ill health / disability", + "Property unsuitable because of poor condition", + "Couldn't afford fees attached to renewing the tenancy", + "Couldn't afford increase in rent", + "Couldn't afford rent or mortgage - welfare reforms", + "Couldn't afford rent or mortgage - employment", + "Couldn't afford rent or mortgage - other", + "To move nearer to family / friends / school", + "To move nearer to work", + "To move to accomodation with support", + "To move to independent accomodation", + "Hate crime", + "Death of household member in last settled accomodation", + "Discharged from prison", + "Discharged from long stay hospital or similar institution", + "Other", + "Do not know", + "Prefer not to say" + ] }, "benefit_cap_spare_room_subsidy": { "type": "string", - "minLength": 1 + "minLength": 1, + "enum": [ + "Yes - benefit cap", + "Yes - removal of the spare room subsidy", + "Yes - both the benefit cap and the removal of the spare room subsidy", + "No", + "Do not know", + "Prefer not to say" + ] }, "armed_forces_active": { "type": "string", @@ -1158,4 +1202,4 @@ }, "securitySchemes": {} } -} +} \ No newline at end of file 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 12/14] 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 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 13/14] 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 14/14] 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