From 5aff4d9989d09b9ab89bfe2bc2d2cd847ff9b608 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:24:36 +0000 Subject: [PATCH 1/3] CLDC-1180 Link to radio/checkbox errors from summary (#2835) * try linking errors * Change question ids to symbols and update checkboxes --- app/views/form/_checkbox_question.html.erb | 5 +++-- app/views/form/_radio_question.html.erb | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/views/form/_checkbox_question.html.erb b/app/views/form/_checkbox_question.html.erb index 2e8585e15..b4feb12bd 100644 --- a/app/views/form/_checkbox_question.html.erb +++ b/app/views/form/_checkbox_question.html.erb @@ -6,16 +6,17 @@ hint: { text: question.hint_text&.html_safe } do %> <% after_divider = false %> - <% question.displayed_answer_options(@log).map do |key, option| %> + <% question.displayed_answer_options(@log).each_with_index do |(key, option), index| %> <% if key.starts_with?("divider") %> <% after_divider = true %> <%= f.govuk_check_box_divider %> <% else %> - <%= f.govuk_check_box question.id, key, + <%= f.govuk_check_box question.id.to_sym, key, label: { text: option["value"] }, hint: { text: option["hint"] }, checked: @log[key] == 1, exclusive: after_divider, + link_errors: index.zero? ? true : nil, **stimulus_html_attributes(question) %> <% end %> <% end %> diff --git a/app/views/form/_radio_question.html.erb b/app/views/form/_radio_question.html.erb index 41e98a1cc..bf6abb0d0 100644 --- a/app/views/form/_radio_question.html.erb +++ b/app/views/form/_radio_question.html.erb @@ -18,22 +18,24 @@ legend: legend(question, page_header, conditional), hint: { text: question.hint_text&.html_safe } do %> - <% question.displayed_answer_options(@log, current_user).map do |key, options| %> + <% question.displayed_answer_options(@log, current_user).each_with_index do |(key, options), index| %> <% if key.starts_with?("divider") %> <%= f.govuk_radio_divider %> <% else %> <% conditional_question = find_conditional_question(@page, question, key) %> <% if conditional_question.nil? %> - <%= f.govuk_radio_button question.id, + <%= f.govuk_radio_button question.id.to_sym, key, label: { text: options["value"] }, hint: { text: options["hint"] }, + link_errors: index.zero? ? true : nil, **stimulus_html_attributes(question) %> <% else %> - <%= f.govuk_radio_button question.id, + <%= f.govuk_radio_button question.id.to_sym, key, label: { text: options["value"] }, hint: { text: options["hint"] }, + link_errors: index.zero? ? true : nil, **stimulus_html_attributes(question) do %> <%= render partial: "#{conditional_question.type}_question", locals: { question: conditional_question, From 838ad04e631419b0dbf3b459c47e9a53d5df63f0 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:08:54 +0000 Subject: [PATCH 2/3] CLDC-2141 Update form submit flow (#2838) * Redirect after update * Fix world * Only check type if question is found * Remove empty line * Clear cache --- Dockerfile | 2 +- app/controllers/form_controller.rb | 36 +++++++++++++++++++++------ spec/requests/form_controller_spec.rb | 11 ++++++-- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/Dockerfile b/Dockerfile index 74faebfd8..814582011 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,7 +10,7 @@ RUN apk add --update --no-cache tzdata && \ # build-base: compilation tools for bundle # yarn: node package manager # postgresql-dev: postgres driver and libraries -RUN apk add --no-cache build-base=0.5-r3 busybox=1.36.1-r7 nodejs-current=20.8.1-r0 yarn=1.22.19-r0 postgresql13-dev=13.17-r0 git=2.40.3-r0 bash=5.2.15-r5 +RUN apk add --no-cache build-base=0.5-r3 busybox=1.36.1-r7 nodejs-current=20.8.1-r0 yarn=1.22.19-r0 postgresql13-dev=13.18-r0 git=2.40.3-r0 bash=5.2.15-r5 # Bundler version should be the same version as what the Gemfile.lock was bundled with RUN gem install bundler:2.3.14 --no-document diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 70b6f892b..7ce63e609 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -5,6 +5,7 @@ class FormController < ApplicationController before_action :find_resource, only: %i[review] before_action :find_resource_by_named_id, except: %i[review] before_action :check_collection_period, only: %i[submit_form show_page] + before_action :set_cache_headers, only: [:show_page] def submit_form if @log @@ -37,8 +38,9 @@ class FormController < ApplicationController error_attributes = @log.errors.map(&:attribute) Rails.logger.info "User triggered validation(s) on: #{error_attributes.join(', ')}" @subsection = form.subsection_for_page(@page) - restore_error_field_values(@page&.questions) - render "form/page" + flash[:errors] = @log.errors + flash[:log_data] = responses_for_page + redirect_to send("#{@log.class.name.underscore}_#{@page.id}_path", @log, { referrer: request.params["referrer"], original_page_id: request.params["original_page_id"], related_question_ids: request.params["related_question_ids"] }) end else render_not_found @@ -84,6 +86,10 @@ class FormController < ApplicationController @questions = request.params["related_question_ids"].map { |id| @log.form.get_question(id, @log) } render "form/check_errors" else + if flash[:errors].present? + restore_previous_errors(flash[:errors]) + restore_error_field_values(flash[:log_data]) + end render "form/page" end else @@ -96,13 +102,21 @@ class FormController < ApplicationController private - def restore_error_field_values(questions) - return unless questions + def restore_error_field_values(previous_responses) + return unless previous_responses - questions.each do |question| - if question&.type == "date" && @log.attributes.key?(question.id) - @log[question.id] = @log.send("#{question.id}_was") - end + previous_responses_to_reset = previous_responses.reject do |key, _| + @log.form.get_question(key, @log)&.type == "date" + end + + @log.assign_attributes(previous_responses_to_reset) + end + + def restore_previous_errors(previous_errors) + return unless previous_errors + + previous_errors.each do |attribute, message| + @log.errors.add attribute, message.first end end @@ -431,4 +445,10 @@ private def updated_answer_from_check_errors_page? params["check_errors"] end + + def set_cache_headers + response.headers["Cache-Control"] = "no-store, no-cache, must-revalidate, max-age=0" + response.headers["Pragma"] = "no-cache" + response.headers["Expires"] = "Mon, 01 Jan 1990 00:00:00 GMT" + end end diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 13d711c20..e727d8acc 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -582,8 +582,9 @@ RSpec.describe FormController, type: :request do allow(Rails.logger).to receive(:info) end - it "re-renders the same page with errors if validation fails" do + it "redirects to the same page with errors if validation fails" do post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params + follow_redirect! expect(page).to have_content("There is a problem") expect(page).to have_content("Error: What is the tenant’s age?") end @@ -591,6 +592,8 @@ RSpec.describe FormController, type: :request do it "resets errors when fixed" do post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: valid_params + follow_redirect! + expect(page).not_to have_content("There is a problem") get "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}" expect(page).not_to have_content("There is a problem") end @@ -616,6 +619,7 @@ RSpec.describe FormController, type: :request do it "validates the date correctly" do post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params + follow_redirect! expect(page).to have_content("There is a problem") end end @@ -693,6 +697,7 @@ RSpec.describe FormController, type: :request do it "validates the date correctly" do post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params + follow_redirect! expect(page).to have_content("There is a problem") end end @@ -713,6 +718,7 @@ RSpec.describe FormController, type: :request do it "validates the date correctly" do post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params: params + follow_redirect! expect(page).to have_content("There is a problem") end end @@ -748,8 +754,9 @@ RSpec.describe FormController, type: :request do Timecop.unfreeze end - it "re-renders the same page with errors if validation fails" do + it "redirects the same page with errors if validation fails" do post "/lettings-logs/#{lettings_log.id}/managing-organisation", params: params + follow_redirect! expect(page).to have_content("There is a problem") expect(page).to have_content("Error: Which organisation manages this letting?") end From 4659b609ce4772f2d2362dfe393fece7889a258b Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:13:13 +0000 Subject: [PATCH 3/3] Fix world (#2844)