From 73bef20430bc76b33834b125aa222aaf31790fcf Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 11 Oct 2021 15:30:18 +0100 Subject: [PATCH] CLDC-342: Save checkbox answers (#40) * Add checked answers values as true if they are passed back as case log params from the form * Allow changing/unticking saved checkbox values * rubocop * Save keys for checkboxes instead of labels * Add conditional effects keys and table fields * Add reasonable preference reason keys and table fields * Redo the migrations * Move method call around * Update case_log params before checking them with page_params * Reload case log rather than multiple lookups * Refactor checkbox question selection * Refactor out the need to get list of checkbox questions * Spec merging of answers across different question types * Update readme to include checkbox question data model assumptions * Fix postcode duplication Co-authored-by: baarkerlounger --- README.md | 2 +- app/controllers/case_logs_controller.rb | 21 +++-- app/views/case_logs/_log_list.html.erb | 2 +- app/views/form/_checkbox_question.html.erb | 2 +- config/forms/2021_2022.json | 46 +++++------ ...20211008130122_add_accessibility_fields.rb | 13 +++ ...1008130149_add_condition_effects_fields.rb | 17 ++++ ...add_reasonable_preference_reason_fields.rb | 11 +++ ...011115946_rename_economic_status_fields.rb | 14 ++++ db/schema.rb | 42 +++++++--- spec/controllers/case_logs_controller_spec.rb | 82 +++++++++++++++++++ spec/factories/case_log.rb | 4 +- spec/helpers/tasklist_helper_spec.rb | 2 +- spec/views/case_log_index_view_spec.rb | 4 +- 14 files changed, 216 insertions(+), 46 deletions(-) create mode 100644 db/migrate/20211008130122_add_accessibility_fields.rb create mode 100644 db/migrate/20211008130149_add_condition_effects_fields.rb create mode 100644 db/migrate/20211008130208_add_reasonable_preference_reason_fields.rb create mode 100644 db/migrate/20211011115946_rename_economic_status_fields.rb diff --git a/README.md b/README.md index 7a7a2efc5..20046146f 100644 --- a/README.md +++ b/README.md @@ -133,7 +133,7 @@ Assumptions made by the format: - All sections have at least 1 subsection - All subsections have at least 1 page - All pages have at least 1 question -- The ActiveRecord case log model has a field for each question name (must match) +- The ActiveRecord case log model has a field for each question name (must match). In the case of checkbox questions it must have one field for every answer option (again names must match). - Text not required by a page/question such as a header or hint text should be passed as an empty string - For conditionally shown questions conditions that have been implemented and can be used are: - Radio question answer option selected matches one of conditional e.g. ["answer-options-1-string", "answer-option-3-string"] diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 9dffdc533..82ea9ec1e 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -24,9 +24,10 @@ class CaseLogsController < ApplicationController form = Form.new(2021, 2022) @case_log = CaseLog.find(params[:id]) previous_page = params[:case_log][:previous_page] - questions_for_page = form.questions_for_page(previous_page).keys - answers_for_page = page_params(questions_for_page).select { |k, _v| questions_for_page.include?(k) } - if @case_log.update(answers_for_page) + questions_for_page = form.questions_for_page(previous_page) + responses_for_page = question_responses(questions_for_page) + + if @case_log.update(responses_for_page) redirect_path = form.next_page_redirect_path(previous_page) redirect_to(send(redirect_path, @case_log)) else @@ -52,7 +53,17 @@ class CaseLogsController < ApplicationController private - def page_params(questions_for_page) - params.require(:case_log).permit(questions_for_page) + def question_responses(questions_for_page) + questions_for_page.each_with_object({}) do |(question_key, question_info), result| + question_params = params["case_log"][question_key] + if question_info["type"] == "checkbox" + question_info["answer_options"].keys.reject { |x| x.match(/divider/) }.each do |option| + result[option] = question_params.include?(option) + end + else + result[question_key] = question_params + end + result + end end end diff --git a/app/views/case_logs/_log_list.html.erb b/app/views/case_logs/_log_list.html.erb index 37d0dca14..3edfabe06 100644 --- a/app/views/case_logs/_log_list.html.erb +++ b/app/views/case_logs/_log_list.html.erb @@ -15,7 +15,7 @@ <%= link_to log.id, case_log_path(log) %> - <%= log.postcode %> + <%= log.property_postcode %> <%= log.tenant_code %> diff --git a/app/views/form/_checkbox_question.html.erb b/app/views/form/_checkbox_question.html.erb index 99bd3fd49..ccd50d8a3 100644 --- a/app/views/form/_checkbox_question.html.erb +++ b/app/views/form/_checkbox_question.html.erb @@ -6,7 +6,7 @@ <% if key.starts_with?("divider") %> <%= f.govuk_check_box_divider %> <% else %> - <%= f.govuk_check_box question_key, val, label: { text: val }, **stimulus_html_attributes(question) %> + <%= f.govuk_check_box question_key, key, label: { text: val }, **stimulus_html_attributes(question) %> <% end %> <% end %> <% end %> diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index eb91f81e3..8bed823cf 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -783,15 +783,15 @@ "type": "checkbox", "check_answer_label": "Disability requirements", "answer_options": { - "0": "Fully wheelchair accessible housing", - "1": "Wheelchair access to essential rooms", - "2": "Level access housing", - "3": "Other disability requirements", - "4": "No disability requirements", + "accessibility_requirements_fully_wheelchair_accessible_housing": "Fully wheelchair accessible housing", + "accessibility_requirements_wheelchair_access_to_essential_rooms": "Wheelchair access to essential rooms", + "accessibility_requirements_level_access_housing": "Level access housing", + "accessibility_requirements_other_disability_requirements": "Other disability requirements", + "accessibility_requirements_no_disability_requirements": "No disability requirements", "divider_a": true, - "5": "Do not know", + "accessibility_requirements_do_not_know": "Do not know", "divider_b": true, - "6": "Prefer not to say" + "accessibility_requirements_prefer_not_to_say": "Prefer not to say" } } } @@ -806,18 +806,18 @@ "type": "checkbox", "check_answer_label": "Conditions or illnesses", "answer_options": { - "0": "Vision - such as blindness or partial sight", - "1": "Hearing - such as deafness or partial hearing", - "2": "Mobility - such as walking short distances or climbing stairs", - "3": "Dexterity - such as lifting and carrying objects or using a keyboard", - "4": "Stamina or breathing or fatigue", - "5": "Learning or understanding or concentrating", - "6": "Memory", - "7": "Mental health - such as depression, anxiety, schizophrenia or bipolar", - "8": "Socially or behaviourally - such as those associated with autism spectral disorder (ASD) including Aspergers’ or attention deficit hyperactivity disorder (ADHD))", - "9": "Other", + "condition_effects_vision": "Vision - such as blindness or partial sight", + "condition_effects_hearing": "Hearing - such as deafness or partial hearing", + "condition_effects_mobility": "Mobility - such as walking short distances or climbing stairs", + "condition_effects_dexterity": "Dexterity - such as lifting and carrying objects or using a keyboard", + "condition_effects_stamina": "Stamina or breathing or fatigue", + "condition_effects_learning": "Learning or understanding or concentrating", + "condition_effects_memory": "Memory", + "condition_effects_mental_health": "Mental health - such as depression, anxiety, schizophrenia or bipolar", + "condition_effects_social_or_behavioral": "Socially or behaviourally - such as those associated with autism spectral disorder (ASD) including Aspergers’ or attention deficit hyperactivity disorder (ADHD))", + "condition_effects_other": "Other", "divider": true, - "10": "Prefer not to say" + "condition_effects_prefer_not_to_say": "Prefer not to say" } } } @@ -2025,12 +2025,12 @@ "hint_text": "Select all that apply", "type": "checkbox", "answer_options": { - "0": "Homeless or about to lose their home (within 56 days)", - "1": "Living in insanitary or overcrowded or unsatisfactory housing", - "2": "A need to move on medical and welfare grounds (including a disability)", - "3": "A need to move to avoid hardship to themselves or others", + "reasonable_preference_reason_homeless": "Homeless or about to lose their home (within 56 days)", + "reasonable_preference_reason_unsatisfactory_housing": "Living in insanitary or overcrowded or unsatisfactory housing", + "reasonable_preference_reason_medical_grounds": "A need to move on medical and welfare grounds (including a disability)", + "reasonable_preference_reason_avoid_hardship": "A need to move to avoid hardship to themselves or others", "divider": true, - "4": "Do not know" + "reasonable_preference_reason_do_not_know": "Do not know" } } } diff --git a/db/migrate/20211008130122_add_accessibility_fields.rb b/db/migrate/20211008130122_add_accessibility_fields.rb new file mode 100644 index 000000000..500d6c027 --- /dev/null +++ b/db/migrate/20211008130122_add_accessibility_fields.rb @@ -0,0 +1,13 @@ +class AddAccessibilityFields < ActiveRecord::Migration[6.1] + def change + change_table :case_logs, bulk: true do |t| + t.column :accessibility_requirements_fully_wheelchair_accessible_housing, :boolean + t.column :accessibility_requirements_wheelchair_access_to_essential_rooms, :boolean + t.column :accessibility_requirements_level_access_housing, :boolean + t.column :accessibility_requirements_other_disability_requirements, :boolean + t.column :accessibility_requirements_no_disability_requirements, :boolean + t.column :accessibility_requirements_do_not_know, :boolean + t.column :accessibility_requirements_prefer_not_to_say, :boolean + end + end +end diff --git a/db/migrate/20211008130149_add_condition_effects_fields.rb b/db/migrate/20211008130149_add_condition_effects_fields.rb new file mode 100644 index 000000000..4044d91c5 --- /dev/null +++ b/db/migrate/20211008130149_add_condition_effects_fields.rb @@ -0,0 +1,17 @@ +class AddConditionEffectsFields < ActiveRecord::Migration[6.1] + def change + change_table :case_logs, bulk: true do |t| + t.column :condition_effects_vision, :boolean + t.column :condition_effects_hearing, :boolean + t.column :condition_effects_mobility, :boolean + t.column :condition_effects_dexterity, :boolean + t.column :condition_effects_stamina, :boolean + t.column :condition_effects_learning, :boolean + t.column :condition_effects_memory, :boolean + t.column :condition_effects_mental_health, :boolean + t.column :condition_effects_social_or_behavioral, :boolean + t.column :condition_effects_other, :boolean + t.column :condition_effects_prefer_not_to_say, :boolean + end + end +end diff --git a/db/migrate/20211008130208_add_reasonable_preference_reason_fields.rb b/db/migrate/20211008130208_add_reasonable_preference_reason_fields.rb new file mode 100644 index 000000000..cb4e81837 --- /dev/null +++ b/db/migrate/20211008130208_add_reasonable_preference_reason_fields.rb @@ -0,0 +1,11 @@ +class AddReasonablePreferenceReasonFields < ActiveRecord::Migration[6.1] + def change + change_table :case_logs, bulk: true do |t| + t.column :reasonable_preference_reason_homeless, :boolean + t.column :reasonable_preference_reason_unsatisfactory_housing, :boolean + t.column :reasonable_preference_reason_medical_grounds, :boolean + t.column :reasonable_preference_reason_avoid_hardship, :boolean + t.column :reasonable_preference_reason_do_not_know, :boolean + end + end +end diff --git a/db/migrate/20211011115946_rename_economic_status_fields.rb b/db/migrate/20211011115946_rename_economic_status_fields.rb new file mode 100644 index 000000000..b7c6a94ab --- /dev/null +++ b/db/migrate/20211011115946_rename_economic_status_fields.rb @@ -0,0 +1,14 @@ +class RenameEconomicStatusFields < ActiveRecord::Migration[6.1] + def change + change_table :case_logs, bulk: true do |t| + t.rename :person_2_economic, :person_2_economic_status + t.rename :person_3_economic, :person_3_economic_status + t.rename :person_4_economic, :person_4_economic_status + t.rename :person_5_economic, :person_5_economic_status + t.rename :person_6_economic, :person_6_economic_status + t.rename :person_7_economic, :person_7_economic_status + t.rename :person_8_economic, :person_8_economic_status + t.rename :postcode, :property_postcode + end + end +end diff --git a/db/schema.rb b/db/schema.rb index fb810f511..3f5910828 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_05_115813) do +ActiveRecord::Schema.define(version: 2021_10_11_115946) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -27,37 +27,36 @@ ActiveRecord::Schema.define(version: 2021_10_05_115813) do t.string "previous_housing_situation" t.integer "prior_homelessness" t.string "armed_forces" - t.string "postcode" t.string "tenant_economic_status" t.integer "household_number_of_other_members" t.string "person_2_relationship" t.integer "person_2_age" t.string "person_2_gender" - t.string "person_2_economic" + t.string "person_2_economic_status" t.string "person_3_relationship" t.integer "person_3_age" t.string "person_3_gender" - t.string "person_3_economic" + t.string "person_3_economic_status" t.string "person_4_relationship" t.integer "person_4_age" t.string "person_4_gender" - t.string "person_4_economic" + t.string "person_4_economic_status" t.string "person_5_relationship" t.integer "person_5_age" t.string "person_5_gender" - t.string "person_5_economic" + t.string "person_5_economic_status" t.string "person_6_relationship" t.integer "person_6_age" t.string "person_6_gender" - t.string "person_6_economic" + t.string "person_6_economic_status" t.string "person_7_relationship" t.integer "person_7_age" t.string "person_7_gender" - t.string "person_7_economic" + t.string "person_7_economic_status" t.string "person_8_relationship" t.integer "person_8_age" t.string "person_8_gender" - t.string "person_8_economic" + t.string "person_8_economic_status" t.string "homelessness" t.string "reason_for_leaving_last_settled_home" t.string "benefit_cap_spare_room_subsidy" @@ -102,13 +101,36 @@ ActiveRecord::Schema.define(version: 2021_10_05_115813) do t.string "time_lived_in_la" t.string "time_on_la_waiting_list" t.string "previous_la" - t.string "property_postcode" t.string "reasonable_preference" t.string "reasonable_preference_reason" t.string "cbl_letting" t.string "chr_letting" t.string "cap_letting" t.string "outstanding_rent_or_charges" + t.boolean "accessibility_requirements_fully_wheelchair_accessible_housing" + t.boolean "accessibility_requirements_wheelchair_access_to_essential_rooms" + t.boolean "accessibility_requirements_level_access_housing" + t.boolean "accessibility_requirements_other_disability_requirements" + t.boolean "accessibility_requirements_no_disability_requirements" + t.boolean "accessibility_requirements_do_not_know" + t.boolean "accessibility_requirements_prefer_not_to_say" + t.boolean "condition_effects_vision" + t.boolean "condition_effects_hearing" + t.boolean "condition_effects_mobility" + t.boolean "condition_effects_dexterity" + t.boolean "condition_effects_stamina" + t.boolean "condition_effects_learning" + t.boolean "condition_effects_memory" + t.boolean "condition_effects_mental_health" + t.boolean "condition_effects_social_or_behavioral" + t.boolean "condition_effects_other" + t.boolean "condition_effects_prefer_not_to_say" + t.boolean "reasonable_preference_reason_homeless" + t.boolean "reasonable_preference_reason_unsatisfactory_housing" + t.boolean "reasonable_preference_reason_medical_grounds" + t.boolean "reasonable_preference_reason_avoid_hardship" + t.boolean "reasonable_preference_reason_do_not_know" + t.string "property_postcode" end end diff --git a/spec/controllers/case_logs_controller_spec.rb b/spec/controllers/case_logs_controller_spec.rb index 5526f8cb3..6d73a5212 100644 --- a/spec/controllers/case_logs_controller_spec.rb +++ b/spec/controllers/case_logs_controller_spec.rb @@ -43,4 +43,86 @@ RSpec.describe CaseLogsController, type: :controller do end end end + + describe "submit_form" do + let!(:case_log) { FactoryBot.create(:case_log) } + let(:id) { case_log.id } + let(:case_log_form_params) do + { accessibility_requirements: + %w[ accessibility_requirements_fully_wheelchair_accessible_housing + accessibility_requirements_wheelchair_access_to_essential_rooms + accessibility_requirements_level_access_housing], + previous_page: "accessibility_requirements" } + end + + let(:new_case_log_form_params) do + { + accessibility_requirements: %w[accessibility_requirements_level_access_housing], + previous_page: "accessibility_requirements", + } + end + + it "sets checked items to true" do + post :submit_form, params: { id: id, case_log: case_log_form_params } + case_log.reload + + expect(case_log.accessibility_requirements_fully_wheelchair_accessible_housing).to eq(true) + expect(case_log.accessibility_requirements_wheelchair_access_to_essential_rooms).to eq(true) + expect(case_log.accessibility_requirements_level_access_housing).to eq(true) + end + + it "sets previously submitted items to false when resubmitted with new values" do + post :submit_form, params: { id: id, case_log: new_case_log_form_params } + case_log.reload + + expect(case_log.accessibility_requirements_fully_wheelchair_accessible_housing).to eq(false) + expect(case_log.accessibility_requirements_wheelchair_access_to_essential_rooms).to eq(false) + expect(case_log.accessibility_requirements_level_access_housing).to eq(true) + end + + context "given a page with checkbox and non-checkbox questions" do + let(:tenant_code) { "BZ355" } + let(:case_log_form_params) do + { accessibility_requirements: + %w[ accessibility_requirements_fully_wheelchair_accessible_housing + accessibility_requirements_wheelchair_access_to_essential_rooms + accessibility_requirements_level_access_housing], + tenant_code: tenant_code, + previous_page: "accessibility_requirements" } + end + let(:questions_for_page) do + {"accessibility_requirements"=> + { + "type"=>"checkbox", + "answer_options"=> + {"accessibility_requirements_fully_wheelchair_accessible_housing"=>"Fully wheelchair accessible housing", + "accessibility_requirements_wheelchair_access_to_essential_rooms"=>"Wheelchair access to essential rooms", + "accessibility_requirements_level_access_housing"=>"Level access housing", + "accessibility_requirements_other_disability_requirements"=>"Other disability requirements", + "accessibility_requirements_no_disability_requirements"=>"No disability requirements", + "divider_a"=>true, + "accessibility_requirements_do_not_know"=>"Do not know", + "divider_b"=>true, + "accessibility_requirements_prefer_not_to_say"=>"Prefer not to say" + } + }, + "tenant_code"=> + { + "type"=>"text" + } + } + end + + it "updates both question fields" do + allow_any_instance_of(Form).to receive(:questions_for_page).and_return(questions_for_page) + post :submit_form, params: { id: id, case_log: case_log_form_params } + case_log.reload + + expect(case_log.accessibility_requirements_fully_wheelchair_accessible_housing).to eq(true) + expect(case_log.accessibility_requirements_wheelchair_access_to_essential_rooms).to eq(true) + expect(case_log.accessibility_requirements_level_access_housing).to eq(true) + expect(case_log.tenant_code).to eq(tenant_code) + end + end + end end diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index d55408242..a9aa4ab56 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -4,14 +4,14 @@ FactoryBot.define do trait :in_progress do status { 0 } tenant_code { "TH356" } - postcode { "SW2 6HI" } + property_postcode { "SW2 6HI" } previous_postcode { "P0 5ST" } tenant_age { "12" } end trait :submitted do status { 1 } tenant_code { "BZ737" } - postcode { "NW1 7TY" } + property_postcode { "NW1 7TY" } end created_at { Time.zone.now } updated_at { Time.zone.now } diff --git a/spec/helpers/tasklist_helper_spec.rb b/spec/helpers/tasklist_helper_spec.rb index ec07092f6..f7b2a6534 100644 --- a/spec/helpers/tasklist_helper_spec.rb +++ b/spec/helpers/tasklist_helper_spec.rb @@ -73,7 +73,7 @@ RSpec.describe TasklistHelper do it "returns the number of sections in progress" do @form = Form.new(2021, 2022) - expect(get_sections_count(@form, case_log, :in_progress)).to eq(2) + expect(get_sections_count(@form, case_log, :in_progress)).to eq(3) end it "returns 0 for invalid state" do diff --git a/spec/views/case_log_index_view_spec.rb b/spec/views/case_log_index_view_spec.rb index f99c82419..b97347cb6 100644 --- a/spec/views/case_log_index_view_spec.rb +++ b/spec/views/case_log_index_view_spec.rb @@ -12,7 +12,7 @@ RSpec.describe "case_logs/index" do expect(rendered).to match(/Logs you need to complete/) expect(rendered).not_to match(/Logs you've submitted/) expect(rendered).to match(in_progress_log.tenant_code) - expect(rendered).to match(in_progress_log.postcode) + expect(rendered).to match(in_progress_log.property_postcode) end end @@ -25,7 +25,7 @@ RSpec.describe "case_logs/index" do expect(rendered).to match(/Logs you've submitted/) expect(rendered).not_to match(/Logs you need to complete/) expect(rendered).to match(submitted_log.tenant_code) - expect(rendered).to match(submitted_log.postcode) + expect(rendered).to match(submitted_log.property_postcode) end end