baarkerlounger
/
submit-social-housing-lettings-and-sales-data
mirror of https://github.com/communitiesuk/submit-social-housing-lettings-and-sales-data.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Re-render page with errors (#1523)

pull/1526/head
kosiakkatrina 2 years ago committed by GitHub
parent
9bbc2c2e17
commit
97e1463fda
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 40 additions and 48 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 29
      app/controllers/form_controller.rb
  2. 2
      app/views/form/page.html.erb
  3. 2
      config/routes.rb
  4. 4
      spec/features/form/page_routing_spec.rb
  5. 51
      spec/requests/form_controller_spec.rb

29
app/controllers/form_controller.rb
Unescape View File

@ -1,7 +1,7 @@
class FormController < ApplicationController class FormController < ApplicationController
before_action :authenticate_user! before_action :authenticate_user!
before_action :find_resource, only: %i[submit_form review] before_action :find_resource, only: %i[review]
before_action :find_resource_by_named_id, except: %i[submit_form review] before_action :find_resource_by_named_id, except: %i[review]
before_action :check_collection_period, only: %i[submit_form show_page] before_action :check_collection_period, only: %i[submit_form show_page]
def submit_form def submit_form
@ -11,16 +11,15 @@ class FormController < ApplicationController
mandatory_questions_with_no_response = mandatory_questions_with_no_response(responses_for_page) mandatory_questions_with_no_response = mandatory_questions_with_no_response(responses_for_page)
if mandatory_questions_with_no_response.empty? && @log.update(responses_for_page.merge(updated_by: current_user)) if mandatory_questions_with_no_response.empty? && @log.update(responses_for_page.merge(updated_by: current_user))
session[:errors] = session[:fields] = nil
redirect_to(successful_redirect_path) redirect_to(successful_redirect_path)
else else
redirect_path = "#{@log.model_name.param_key}_#{@page.id}_path"
mandatory_questions_with_no_response.map do |question| mandatory_questions_with_no_response.map do |question|
@log.errors.add question.id.to_sym, question.unanswered_error_message @log.errors.add question.id.to_sym, question.unanswered_error_message
end end
session[:errors] = @log.errors.to_json
Rails.logger.info "User triggered validation(s) on: #{@log.errors.map(&:attribute).join(', ')}" Rails.logger.info "User triggered validation(s) on: #{@log.errors.map(&:attribute).join(', ')}"
redirect_to(send(redirect_path, @log)) @subsection = form.subsection_for_page(@page)
restore_error_field_values(@page&.questions)
render "form/page"
end end
else else
render_not_found render_not_found
@ -47,7 +46,6 @@ class FormController < ApplicationController
def show_page def show_page
if @log if @log
restore_error_field_values
page_id = request.path.split("/")[-1].underscore page_id = request.path.split("/")[-1].underscore
@page = form.get_page(page_id) @page = form.get_page(page_id)
@subsection = form.subsection_for_page(@page) @subsection = form.subsection_for_page(@page)
@ -63,17 +61,12 @@ class FormController < ApplicationController
private private
def restore_error_field_values def restore_error_field_values(questions)
if session["errors"] return unless questions
JSON(session["errors"]).each do |field, messages|
messages.each { |message| @log.errors.add field.to_sym, message } questions.each do |question|
end if question&.type == "date" && @log.attributes.key?(question.id)
end @log[question.id] = @log.send("#{question.id}_was")
if session["fields"]
session["fields"].each do |field, value|
if form.get_question(field, @log)&.type != "date" && @log.attributes.key?(field)
@log[field] = value
end
end end
end end
end end

2
app/views/form/page.html.erb
Unescape View File

@ -5,7 +5,7 @@
<% end %> <% end %>
<div data-controller="govukfrontend"></div> <div data-controller="govukfrontend"></div>
<%= form_with model: @log, url: form_lettings_log_path(@log), method: "post", local: true do |f| %> <%= form_with model: @log, url: request.original_url, method: "post", local: true do |f| %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-<%= @page.questions[0].type == "interruption_screen" ? "full-from-desktop" : "two-thirds-from-desktop" %>"> <div class="govuk-grid-column-<%= @page.questions[0].type == "interruption_screen" ? "full-from-desktop" : "two-thirds-from-desktop" %>">
<% remove_other_page_errors(@log, @page) %> <% remove_other_page_errors(@log, @page) %>

2
config/routes.rb
Unescape View File

@ -164,6 +164,7 @@ Rails.application.routes.draw do
FormHandler.instance.lettings_forms.each do |_key, form| FormHandler.instance.lettings_forms.each do |_key, form|
form.pages.map do |page| form.pages.map do |page|
get page.id.to_s.dasherize, to: "form#show_page" get page.id.to_s.dasherize, to: "form#show_page"
post page.id.to_s.dasherize, to: "form#submit_form"
end end
form.subsections.map do |subsection| form.subsections.map do |subsection|
@ -190,6 +191,7 @@ Rails.application.routes.draw do
FormHandler.instance.sales_forms.each do |_key, form| FormHandler.instance.sales_forms.each do |_key, form|
form.pages.map do |page| form.pages.map do |page|
get page.id.to_s.dasherize, to: "form#show_page" get page.id.to_s.dasherize, to: "form#show_page"
post page.id.to_s.dasherize, to: "form#submit_form"
end end
form.subsections.map do |subsection| form.subsections.map do |subsection|

4
spec/features/form/page_routing_spec.rb
Unescape View File

@ -85,11 +85,11 @@ RSpec.describe "Form Page Routing" do
context "when answer is invalid" do context "when answer is invalid" do
it "shows error with invalid value in the field" do it "shows error with invalid value in the field" do
visit("/lettings-logs/#{id}/property-postcode") visit("/lettings-logs/#{id}/property-postcode")
fill_in("lettings-log-postcode-full-field", with: "fake_postcode") fill_in("lettings-log-postcode-full-field", with: "FAKE_POSTCODE")
click_button("Save and continue") click_button("Save and continue")
expect(page).to have_current_path("/lettings-logs/#{id}/property-postcode") expect(page).to have_current_path("/lettings-logs/#{id}/property-postcode")
expect(find("#lettings-log-postcode-full-field-error").value).to eq("fake_postcode") expect(find("#lettings-log-postcode-full-field-error").value).to eq("FAKE_POSTCODE")
end end
it "does not reset the displayed date" do it "does not reset the displayed date" do

51
spec/requests/form_controller_spec.rb
Unescape View File

@ -57,7 +57,7 @@ RSpec.describe FormController, type: :request do
describe "POST" do describe "POST" do
it "does not let you post form answers to lettings logs you don't have access to" do it "does not let you post form answers to lettings logs you don't have access to" do
post "/lettings-logs/#{lettings_log.id}/form", params: {} post "/lettings-logs/#{lettings_log.id}/net-income", params: {}
expect(response).to redirect_to("/account/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
end end
@ -102,7 +102,7 @@ RSpec.describe FormController, type: :request do
end end
it "resets created by and renders the next page" do it "resets created by and renders the next page" do
post "/lettings-logs/#{lettings_log.id}/form", params: params post "/lettings-logs/#{lettings_log.id}/net-income", params: params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/created-by") expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/created-by")
follow_redirect! follow_redirect!
lettings_log.reload lettings_log.reload
@ -127,7 +127,7 @@ RSpec.describe FormController, type: :request do
end end
it "does not reset created by" do it "does not reset created by" do
post "/lettings-logs/#{lettings_log.id}/form", params: params post "/lettings-logs/#{lettings_log.id}/net-income", params: params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/created-by") expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/created-by")
follow_redirect! follow_redirect!
lettings_log.reload lettings_log.reload
@ -152,7 +152,7 @@ RSpec.describe FormController, type: :request do
end end
it "does not reset created by" do it "does not reset created by" do
post "/lettings-logs/#{lettings_log.id}/form", params: params post "/lettings-logs/#{lettings_log.id}/stock-owner", params: params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation") expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
follow_redirect! follow_redirect!
lettings_log.reload lettings_log.reload
@ -177,7 +177,7 @@ RSpec.describe FormController, type: :request do
end end
it "does not reset created by" do it "does not reset created by" do
post "/lettings-logs/#{lettings_log.id}/form", params: params post "/lettings-logs/#{lettings_log.id}/stock-owner", params: params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation") expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
follow_redirect! follow_redirect!
lettings_log.reload lettings_log.reload
@ -400,22 +400,21 @@ RSpec.describe FormController, type: :request do
end end
it "re-renders the same page with errors if validation fails" do it "re-renders the same page with errors if validation fails" do
post "/lettings-logs/#{lettings_log.id}/form", params: params post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}")
follow_redirect!
expect(page).to have_content("There is a problem") expect(page).to have_content("There is a problem")
expect(page).to have_content("Error: What is the tenant’s age?")
end end
it "resets errors when fixed" do it "resets errors when fixed" do
post "/lettings-logs/#{lettings_log.id}/form", params: params post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params
post "/lettings-logs/#{lettings_log.id}/form", params: valid_params post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: valid_params
get "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}" get "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}"
expect(page).not_to have_content("There is a problem") expect(page).not_to have_content("There is a problem")
end end
it "logs that validation was triggered" do it "logs that validation was triggered" do
expect(Rails.logger).to receive(:info).with("User triggered validation(s) on: age1").once expect(Rails.logger).to receive(:info).with("User triggered validation(s) on: age1").once
post "/lettings-logs/#{lettings_log.id}/form", params: post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
end end
context "when the number of days is too high for the month" do context "when the number of days is too high for the month" do
@ -433,8 +432,7 @@ RSpec.describe FormController, type: :request do
end end
it "validates the date correctly" do it "validates the date correctly" do
post "/lettings-logs/#{lettings_log.id}/form", params: params 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("There is a problem")
end end
end end
@ -465,10 +463,9 @@ RSpec.describe FormController, type: :request do
end end
it "re-renders the same page with errors if validation fails" do it "re-renders the same page with errors if validation fails" do
post "/lettings-logs/#{lettings_log.id}/form", params: params post "/lettings-logs/#{lettings_log.id}/managing-organisation", params: params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
follow_redirect!
expect(page).to have_content("There is a problem") expect(page).to have_content("There is a problem")
expect(page).to have_content("Error: Which organisation manages this letting?")
end end
end end
@ -486,7 +483,7 @@ RSpec.describe FormController, type: :request do
end end
before do before do
post "/lettings-logs/#{lettings_log.id}/form", params: post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
end end
it "re-renders the same page with errors if validation fails" do it "re-renders the same page with errors if validation fails" do
@ -524,7 +521,7 @@ RSpec.describe FormController, type: :request do
before do before do
lettings_log.update!(postcode_known: 1, postcode_full: "NW1 8RR") lettings_log.update!(postcode_known: 1, postcode_full: "NW1 8RR")
post "/lettings-logs/#{lettings_log.id}/form", params: valid_params post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: valid_params
end end
it "does not require you to answer that question" do it "does not require you to answer that question" do
@ -558,14 +555,14 @@ RSpec.describe FormController, type: :request do
end end
it "sets checked items to true" do it "sets checked items to true" do
post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_params post "/lettings-logs/#{lettings_log.id}/accessibility-requirements", params: lettings_log_form_params
lettings_log.reload lettings_log.reload
expect(lettings_log.housingneeds_b).to eq(1) expect(lettings_log.housingneeds_b).to eq(1)
end end
it "sets previously submitted items to false when resubmitted with new values" do it "sets previously submitted items to false when resubmitted with new values" do
post "/lettings-logs/#{lettings_log.id}/form", params: new_lettings_log_form_params post "/lettings-logs/#{lettings_log.id}/accessibility-requirements", params: new_lettings_log_form_params
lettings_log.reload lettings_log.reload
expect(lettings_log.housingneeds_b).to eq(0) expect(lettings_log.housingneeds_b).to eq(0)
@ -609,7 +606,7 @@ RSpec.describe FormController, type: :request do
it "updates both question fields" do it "updates both question fields" do
allow(page).to receive(:questions).and_return(questions_for_page) allow(page).to receive(:questions).and_return(questions_for_page)
post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_params post "/lettings-logs/#{lettings_log.id}/#{page.id.dasherize}", params: lettings_log_form_params
lettings_log.reload lettings_log.reload
expect(lettings_log.housingneeds_a).to eq(1) expect(lettings_log.housingneeds_a).to eq(1)
@ -650,16 +647,16 @@ RSpec.describe FormController, type: :request do
end end
it "routes to the appropriate conditional page based on the question answer of the current page" do it "routes to the appropriate conditional page based on the question answer of the current page" do
post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_conditional_question_yes_params post "/lettings-logs/#{lettings_log.id}/property-wheelchair-accessible", params: lettings_log_form_conditional_question_yes_params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-yes-page") expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-yes-page")
post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_conditional_question_no_params post "/lettings-logs/#{lettings_log.id}/property-wheelchair-accessible", params: lettings_log_form_conditional_question_no_params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-no-page") expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-no-page")
end end
it "routes to the page if at least one of the condition sets is met" do it "routes to the page if at least one of the condition sets is met" do
post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_conditional_question_wchair_yes_params post "/lettings-logs/#{lettings_log.id}/property-wheelchair-accessible", params: lettings_log_form_conditional_question_wchair_yes_params
post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_conditional_question_no_params post "/lettings-logs/#{lettings_log.id}/property-wheelchair-accessible", params: lettings_log_form_conditional_question_no_params
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-yes-page") expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-yes-page")
end end
end end
@ -690,7 +687,7 @@ RSpec.describe FormController, type: :request do
completed_lettings_log.update!(ecstat1: 1, earnings: 130, hhmemb: 1) # we're not routing to that page, so it gets cleared? completed_lettings_log.update!(ecstat1: 1, earnings: 130, hhmemb: 1) # we're not routing to that page, so it gets cleared?
allow(completed_lettings_log).to receive(:net_income_soft_validation_triggered?).and_return(true) allow(completed_lettings_log).to receive(:net_income_soft_validation_triggered?).and_return(true)
allow(completed_lettings_log.form).to receive(:end_date).and_return(Time.zone.today + 1.day) allow(completed_lettings_log.form).to receive(:end_date).and_return(Time.zone.today + 1.day)
post "/lettings-logs/#{completed_lettings_log.id}/form", params: interrupt_params, headers: headers.merge({ "HTTP_REFERER" => referrer }) post "/lettings-logs/#{completed_lettings_log.id}/net-income-value-check", params: interrupt_params, headers: headers.merge({ "HTTP_REFERER" => referrer })
end end
context "when yes is answered" do context "when yes is answered" do
@ -722,7 +719,7 @@ RSpec.describe FormController, type: :request do
end end
before do before do
post "/lettings-logs/#{unauthorized_lettings_log.id}/form", params: {} post "/lettings-logs/#{unauthorized_lettings_log.id}/net-income", params: {}
end end
it "does not let you post form answers to lettings logs you don't have access to" do it "does not let you post form answers to lettings logs you don't have access to" do

Write Preview
Loading…
Cancel
Save