From b5ecd513f08d21b7815f8e3171eee39f2a320889 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Wed, 6 Oct 2021 21:07:15 +0100 Subject: [PATCH] Refactor and fix specs --- app/controllers/case_logs_controller.rb | 16 ++----- app/helpers/conditional_questions_helper.rb | 2 +- .../conditional_question_controller.js | 2 +- app/models/case_log.rb | 2 +- app/models/form.rb | 10 +++++ app/views/form/page.html.erb | 4 +- config/routes.rb | 2 +- spec/features/case_log_spec.rb | 42 +++++++++---------- 8 files changed, 41 insertions(+), 39 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 56d13ec7e..45bc05ee4 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -20,24 +20,16 @@ class CaseLogsController < ApplicationController render :edit end - def next_page + def submit_form 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) } - begin - @case_log.update!(answers_for_page) - next_page = form.next_page(previous_page) - redirect_path = if next_page == :check_answers - subsection = form.subsection_for_page(previous_page) - "case_log_#{subsection}_check_answers_path" - else - "case_log_#{next_page}_path" - end - + if @case_log.update(answers_for_page) + redirect_path = form.next_page_redirect_path(previous_page) redirect_to(send(redirect_path, @case_log)) - rescue ActiveRecord::RecordInvalid + else page_info = form.all_pages[previous_page] render "form/page", locals: { form: form, page_key: previous_page, page_info: page_info }, status: :unprocessable_entity end diff --git a/app/helpers/conditional_questions_helper.rb b/app/helpers/conditional_questions_helper.rb index 78859c6f1..c77119243 100644 --- a/app/helpers/conditional_questions_helper.rb +++ b/app/helpers/conditional_questions_helper.rb @@ -6,6 +6,6 @@ module ConditionalQuestionsHelper end def display_question_key_div(page_info, question_key) - "style='display:none;'" if conditional_questions_for_page(page_info).include?(question_key) + "style='display:none;'".html_safe if conditional_questions_for_page(page_info).include?(question_key) end end diff --git a/app/javascript/controllers/conditional_question_controller.js b/app/javascript/controllers/conditional_question_controller.js index 1b0073476..861e6918e 100644 --- a/app/javascript/controllers/conditional_question_controller.js +++ b/app/javascript/controllers/conditional_question_controller.js @@ -16,7 +16,7 @@ export default class extends Controller { div.style.display = "block" } else { div.style.display = "none" - let buttons = document.getElementsByName(key) + let buttons = document.getElementsByName("case_log[" + key + "]") Object.entries(buttons).forEach(([idx, button]) => { button.checked = false; }) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 142461597..c05ed887b 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -1,5 +1,5 @@ class CaseLog < ApplicationRecord enum status: { "in progress" => 0, "submitted" => 1 } - validates :tenant_age, presence: true + # validates :tenant_age, presence: true end diff --git a/app/models/form.rb b/app/models/form.rb index 586df20f5..9812d890f 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -53,6 +53,16 @@ class Form pages_for_subsection(subsection).keys[previous_page_idx + 1] || :check_answers end + def next_page_redirect_path(previous_page) + next_page = next_page(previous_page) + if next_page == :check_answers + subsection = subsection_for_page(previous_page) + "case_log_#{subsection}_check_answers_path" + else + "case_log_#{next_page}_path" + end + end + def previous_page(current_page) subsection = subsection_for_page(current_page) current_page_idx = pages_for_subsection(subsection).keys.index(current_page) diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index 025693866..2bca4d982 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -11,10 +11,10 @@ <%= page_info["header"] %> <% end %> - <%= form_with model: @case_log, method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> + <%= form_with model: @case_log, method: "submit_form", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> <%= f.govuk_error_summary %> <% page_info["questions"].map do |question_key, question| %> -
<%= display_question_key_div(page_info, question_key) %> > +
<%= display_question_key_div(page_info, question_key) %> > <%= render partial: "form/#{question["type"]}_question", locals: { question_key: question_key, question: question, f: f } %>
<% end %> diff --git a/config/routes.rb b/config/routes.rb index d261b43dd..609124146 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,7 +3,7 @@ Rails.application.routes.draw do get "about", to: "about#index" get "/", to: "test#index" - post '/case_logs/:id', to: "case_logs#next_page" + post '/case_logs/:id', to: "case_logs#submit_form" form = Form.new(2021, 2022) resources :case_logs do diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 5faab619f..46dde63c7 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -17,12 +17,12 @@ RSpec.describe "Test Features" do def answer_all_questions_in_income_subsection visit("/case_logs/#{empty_case_log.id}/net_income") - fill_in("net_income", with: 18_000) - choose("net-income-frequency-yearly-field") + fill_in("case-log-net-income-field", with: 18_000) + choose("case-log-net-income-frequency-yearly-field") click_button("Save and continue") - choose("net-income-uc-proportion-all-field") + choose("case-log-net-income-uc-proportion-all-field") click_button("Save and continue") - choose("housing-benefit-housing-benefit-but-not-universal-credit-field") + choose("case-log-housing-benefit-housing-benefit-but-not-universal-credit-field") click_button("Save and continue") end @@ -80,19 +80,19 @@ RSpec.describe "Test Features" do it "displays the household questions when you click into that section" do visit("/case_logs/#{id}") click_link("Household characteristics") - expect(page).to have_field("tenant-code-field") + expect(page).to have_field("case-log-tenant-code-field") click_button("Save and continue") - expect(page).to have_field("tenant-age-field") + expect(page).to have_field("case-log-tenant-age-field") click_button("Save and continue") - expect(page).to have_field("tenant-gender-male-field") + expect(page).to have_field("case-log-tenant-gender-male-field") visit page.driver.request.env["HTTP_REFERER"] - expect(page).to have_field("tenant-age-field") + expect(page).to have_field("case-log-tenant-age-field") end describe "form questions" do it "can be accessed by url" do visit("/case_logs/#{id}/tenant_age") - expect(page).to have_field("tenant-age-field") + expect(page).to have_field("case-log-tenant-age-field") end it "updates model attributes correctly for each question" do @@ -103,11 +103,11 @@ RSpec.describe "Test Features" do visit("/case_logs/#{id}/#{question}") case type when "text" - fill_in(question.to_s, with: answer) + fill_in("case-log-#{question.to_s.dasherize}-field", with: answer) when "radio" - choose("#{question.to_s.dasherize}-#{answer.parameterize}-field") + choose("case-log-#{question.to_s.dasherize}-#{answer.parameterize}-field") else - fill_in(question.to_s, with: answer) + fill_in("case-log-#{question.to_s.dasherize}-field", with: answer) end expect { click_button("Save and continue") }.to change { case_log.reload.send(question.to_s) @@ -126,7 +126,7 @@ RSpec.describe "Test Features" do it "go back to tenant code page from tenant age page" do visit("/case_logs/#{id}/tenant_age") click_link(text: "Back") - expect(page).to have_field("tenant-code-field") + expect(page).to have_field("case-log-tenant-code-field") end end end @@ -150,7 +150,7 @@ RSpec.describe "Test Features" do context "when the user needs to check their answers for a subsection" do def fill_in_number_question(case_log_id, question, value) visit("/case_logs/#{case_log_id}/#{question}") - fill_in(question.to_s, with: value) + fill_in("case-log-#{question.to_s.dasherize}-field", with: value) click_button("Save and continue") end @@ -175,7 +175,7 @@ RSpec.describe "Test Features" do 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("tenant-gender-non-binary-field") + choose("case-log-tenant-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") @@ -229,14 +229,14 @@ RSpec.describe "Test Features" do it "shows conditional questions if the required answer is selected and hides it again when a different answer option is selected", js: true do visit("/case_logs/#{id}/armed_forces") # Something about our styling makes the selenium webdriver think the actual radio buttons are not visible so we allow label click here - choose("armed-forces-yes-a-regular-field", allow_label_click: true) + choose("case-log-armed-forces-yes-a-regular-field", allow_label_click: true) expect(page).to have_selector("#armed_forces_injured_div") - choose("armed-forces-injured-no-field", allow_label_click: true) - expect(find_field("armed-forces-injured-no-field", visible: false).checked?).to be_truthy - choose("armed-forces-no-field", allow_label_click: true) + choose("case-log-armed-forces-injured-no-field", allow_label_click: true) + expect(find_field("case-log-armed-forces-injured-no-field", visible: false).checked?).to be_truthy + choose("case-log-armed-forces-no-field", allow_label_click: true) expect(page).not_to have_selector("#armed_forces_injured_div") - choose("armed-forces-yes-a-regular-field", allow_label_click: true) - expect(find_field("armed-forces-injured-no-field", visible: false).checked?).to be_falsey + choose("case-log-armed-forces-yes-a-regular-field", allow_label_click: true) + expect(find_field("case-log-armed-forces-injured-no-field", visible: false).checked?).to be_falsey end end end