Refactor and fix specs

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

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

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;
           })

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

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)

app/views/form/page.html.erb

@@ -11,7 +11,7 @@
       <%= page_info["header"] %>
     </h1>
   <% 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| %>
       <div id=<%= question_key + "_div " %><%= display_question_key_div(page_info, question_key) %> >

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

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