Browse Source

CLDC-510: Income range soft validations (#72)

* Init

* Add some tests

* Add failing spec

* Move soft validations into a module

* Update test

* Rubocop

* Scope are auto created by enums

* Rename folder to validations

* No instance variable

* Commit both lines

* Add error indication

* Make partial slightly more generic

* Fix back link

* Write failing test

* All specs currently passing. Can this be real?

* Check page should have an override question

* Fix back button for check answers pages

* Don't really need a wrapper method for the validations

* We're really validating a page here not a question

* Dup variable

* Bit of a nasty hack but maybe better than deriving back link?

* Set a no cache header instead of reloading

* Move a teeny bit of logic out of the controller

* Rubocop

* Extract method
pull/75/head
Daniel Baark 3 years ago committed by GitHub
parent
commit
81ff68bb11
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 27
      app/controllers/case_logs_controller.rb
  2. 22
      app/models/case_log.rb
  3. 9
      app/models/form.rb
  4. 45
      app/validations/soft_validations.rb
  5. 11
      app/views/form/_validation_override_question.html.erb
  6. 10
      app/views/form/page.html.erb
  7. 9
      config/forms/2021_2022.json
  8. 5
      db/migrate/20211028090049_add_net_income_override.rb
  9. 1
      db/schema.rb
  10. 14
      spec/controllers/case_logs_controller_spec.rb
  11. 72
      spec/features/case_log_spec.rb
  12. 9
      spec/fixtures/forms/test_form.json

27
app/controllers/case_logs_controller.rb

@ -56,16 +56,14 @@ class CaseLogsController < ApplicationController
def submit_form def submit_form
form = FormHandler.instance.get_form("2021_2022") form = FormHandler.instance.get_form("2021_2022")
@case_log = CaseLog.find(params[:id]) @case_log = CaseLog.find(params[:id])
previous_page = params[:case_log][:previous_page] @case_log.page = params[:case_log][:page]
questions_for_page = form.questions_for_page(previous_page) responses_for_page = responses_for_page(@case_log.page)
responses_for_page = question_responses(questions_for_page) if @case_log.update(responses_for_page) && @case_log.has_no_unresolved_soft_errors?
@case_log.previous_page = previous_page redirect_path = get_next_page_path(form, @case_log.page, @case_log)
if @case_log.update(responses_for_page)
redirect_path = get_next_page_path(form, previous_page, @case_log)
redirect_to(send(redirect_path, @case_log)) redirect_to(send(redirect_path, @case_log))
else else
page_info = form.all_pages[previous_page] page_info = form.all_pages[@case_log.page]
render "form/page", locals: { form: form, page_key: previous_page, page_info: page_info }, status: :unprocessable_entity render "form/page", locals: { form: form, page_key: @case_log.page, page_info: page_info }, status: :unprocessable_entity
end end
end end
@ -101,9 +99,12 @@ private
API_ACTIONS = %w[create show update destroy].freeze API_ACTIONS = %w[create show update destroy].freeze
def question_responses(questions_for_page) def responses_for_page(page)
questions_for_page.each_with_object({}) do |(question_key, question_info), result| form = FormHandler.instance.get_form("2021_2022")
form.expected_responses_for_page(page).each_with_object({}) do |(question_key, question_info), result|
question_params = params["case_log"][question_key] question_params = params["case_log"][question_key]
next unless question_params
if question_info["type"] == "checkbox" if question_info["type"] == "checkbox"
question_info["answer_options"].keys.reject { |x| x.match(/divider/) }.each do |option| question_info["answer_options"].keys.reject { |x| x.match(/divider/) }.each do |option|
result[option] = question_params.include?(option) result[option] = question_params.include?(option)
@ -129,8 +130,8 @@ private
params.require(:case_log).permit(CaseLog.editable_fields) params.require(:case_log).permit(CaseLog.editable_fields)
end end
def get_next_page_path(form, previous_page, case_log = {}) def get_next_page_path(form, page, case_log = {})
content = form.all_pages[previous_page] content = form.all_pages[page]
if content.key?("conditional_route_to") if content.key?("conditional_route_to")
content["conditional_route_to"].each do |route, conditions| content["conditional_route_to"].each do |route, conditions|
@ -139,6 +140,6 @@ private
end end
end end
end end
form.next_page_redirect_path(previous_page) form.next_page_redirect_path(page)
end end
end end

22
app/models/case_log.rb

@ -10,9 +10,9 @@ class CaseLogValidator < ActiveModel::Validator
# If we've come from the form UI we only want to validate the specific fields # If we've come from the form UI we only want to validate the specific fields
# that have just been submitted. If we're submitting a log via API or Bulk Upload # that have just been submitted. If we're submitting a log via API or Bulk Upload
# we want to validate all data fields. # we want to validate all data fields.
question_to_validate = options[:previous_page] page_to_validate = options[:page]
if question_to_validate if page_to_validate
public_send("validate_#{question_to_validate}", record) if respond_to?("validate_#{question_to_validate}") public_send("validate_#{page_to_validate}", record) if respond_to?("validate_#{page_to_validate}")
else else
validation_methods = public_methods.select { |method| method.starts_with?("validate_") } validation_methods = public_methods.select { |method| method.starts_with?("validate_") }
validation_methods.each { |meth| public_send(meth, record) } validation_methods.each { |meth| public_send(meth, record) }
@ -36,25 +36,19 @@ end
class CaseLog < ApplicationRecord class CaseLog < ApplicationRecord
include Discard::Model include Discard::Model
include SoftValidations
default_scope -> { kept } default_scope -> { kept }
scope :not_started, -> { where(status: "not_started") }
scope :in_progress, -> { where(status: "in_progress") }
scope :not_completed, -> { where.not(status: "completed") } scope :not_completed, -> { where.not(status: "completed") }
scope :completed, -> { where(status: "completed") }
validate :instance_validations validates_with CaseLogValidator, ({ page: @page } || {})
before_save :update_status! before_save :update_status!
attr_writer :previous_page attr_accessor :page
enum status: { "not_started" => 0, "in_progress" => 1, "completed" => 2 } enum status: { "not_started" => 0, "in_progress" => 1, "completed" => 2 }
AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze
def instance_validations
validates_with CaseLogValidator, ({ previous_page: @previous_page } || {})
end
def self.editable_fields def self.editable_fields
attribute_names - AUTOGENERATED_FIELDS attribute_names - AUTOGENERATED_FIELDS
end end
@ -125,6 +119,10 @@ private
dynamically_not_required << "fixed_term_tenancy" dynamically_not_required << "fixed_term_tenancy"
end end
unless net_income_in_soft_max_range? || net_income_in_soft_min_range?
dynamically_not_required << "override_net_income_validation"
end
unless tenancy_type == "Other" unless tenancy_type == "Other"
dynamically_not_required << "other_tenancy_type" dynamically_not_required << "other_tenancy_type"
end end

9
app/models/form.rb

@ -41,6 +41,15 @@ class Form
pages_for_subsection(subsection).map { |title, _value| questions_for_page(title) }.reduce(:merge) pages_for_subsection(subsection).map { |title, _value| questions_for_page(title) }.reduce(:merge)
end end
# Returns a hash with soft validation questions as keys
def soft_validations_for_page(page)
all_pages[page]["soft_validations"]
end
def expected_responses_for_page(page)
questions_for_page(page).merge(soft_validations_for_page(page) || {})
end
def first_page_for_subsection(subsection) def first_page_for_subsection(subsection)
pages_for_subsection(subsection).keys.first pages_for_subsection(subsection).keys.first
end end

45
app/validations/soft_validations.rb

@ -0,0 +1,45 @@
module SoftValidations
def has_no_unresolved_soft_errors?
soft_errors.empty? || soft_errors_overridden?
end
def soft_errors
{}.merge(net_income_validations)
end
def soft_errors_overridden?
public_send(soft_errors.keys.first) if soft_errors.present?
end
private
def net_income_validations
net_income_errors = {}
if net_income_in_soft_min_range?
net_income_errors["override_net_income_validation"] = OpenStruct.new(
message: "Net income is lower than expected based on the main tenant's working situation. Are you sure this is correct?",
hint_text: "This is based on the tenant's work situation: #{person_1_economic_status}",
)
elsif net_income_in_soft_max_range?
net_income_errors["override_net_income_validation"] = OpenStruct.new(
message: "Net income is higher than expected based on the main tenant's working situation. Are you sure this is correct?",
hint_text: "This is based on the tenant's work situation: #{person_1_economic_status}",
)
else
update_column(:override_net_income_validation, nil)
end
net_income_errors
end
def net_income_in_soft_max_range?
return unless weekly_net_income && person_1_economic_status
weekly_net_income.between?(applicable_income_range.soft_max, applicable_income_range.hard_max)
end
def net_income_in_soft_min_range?
return unless weekly_net_income && person_1_economic_status
weekly_net_income.between?(applicable_income_range.soft_min, applicable_income_range.hard_min)
end
end

11
app/views/form/_validation_override_question.html.erb

@ -0,0 +1,11 @@
<div class="govuk-form-group govuk-form-group--error">
<%= f.govuk_check_boxes_fieldset @case_log.soft_errors.keys.first,
legend: { text: @case_log.soft_errors.values.first.message, size: "l" },
hint: { text: @case_log.soft_errors.values.first.hint_text } do %>
<%= f.govuk_check_box @case_log.soft_errors.keys.first, @case_log.soft_errors.keys.first,
label: { text: "Yes" },
checked: f.object.send(@case_log.soft_errors.keys.first)
%>
<% end %>
</div>

10
app/views/form/page.html.erb

@ -1,5 +1,7 @@
<meta http-equiv="Pragma" content="no-cache">
<% content_for :before_content do %> <% content_for :before_content do %>
<%= link_to 'Back', 'javascript:history.back()', class: "govuk-back-link" %> <%= link_to 'Back', :back, class: "govuk-back-link" %>
<% end %> <% end %>
<%= turbo_frame_tag "case_log_form", target: "_top" do %> <%= turbo_frame_tag "case_log_form", target: "_top" do %>
@ -17,7 +19,11 @@
</div> </div>
<% end %> <% end %>
<%= f.hidden_field :previous_page, value: page_key %> <% if @case_log.soft_errors.present? && @case_log.soft_errors.keys.first == page_info["soft_validations"]&.keys&.first %>
<%= render partial: "form/validation_override_question", locals: { f: f } %>
<% end %>
<%= f.hidden_field :page, value: page_key %>
<%= f.govuk_submit "Save and continue" %> <%= f.govuk_submit "Save and continue" %>
<% end %> <% end %>
<% end %> <% end %>

9
config/forms/2021_2022.json

@ -1492,6 +1492,15 @@
"2": "Yearly" "2": "Yearly"
} }
} }
},
"soft_validations": {
"override_net_income_validation": {
"check_answer_label": "Net income confirmed?",
"type": "validation_override",
"answer_options": {
"override_net_income_validation": "Yes"
}
}
} }
}, },
"net_income_uc_proportion": { "net_income_uc_proportion": {

5
db/migrate/20211028090049_add_net_income_override.rb

@ -0,0 +1,5 @@
class AddNetIncomeOverride < ActiveRecord::Migration[6.1]
def change
add_column :case_logs, :override_net_income_validation, :boolean
end
end

1
db/schema.rb

@ -133,6 +133,7 @@ ActiveRecord::Schema.define(version: 2021_10_28_095000) do
t.boolean "reasonable_preference_reason_do_not_know" t.boolean "reasonable_preference_reason_do_not_know"
t.datetime "discarded_at" t.datetime "discarded_at"
t.string "other_tenancy_type" t.string "other_tenancy_type"
t.boolean "override_net_income_validation"
t.string "net_income_known" t.string "net_income_known"
t.index ["discarded_at"], name: "index_case_logs_on_discarded_at" t.index ["discarded_at"], name: "index_case_logs_on_discarded_at"
end end

14
spec/controllers/case_logs_controller_spec.rb

@ -52,13 +52,13 @@ RSpec.describe CaseLogsController, type: :controller do
%w[ accessibility_requirements_fully_wheelchair_accessible_housing %w[ accessibility_requirements_fully_wheelchair_accessible_housing
accessibility_requirements_wheelchair_access_to_essential_rooms accessibility_requirements_wheelchair_access_to_essential_rooms
accessibility_requirements_level_access_housing], accessibility_requirements_level_access_housing],
previous_page: "accessibility_requirements" } page: "accessibility_requirements" }
end end
let(:new_case_log_form_params) do let(:new_case_log_form_params) do
{ {
accessibility_requirements: %w[accessibility_requirements_level_access_housing], accessibility_requirements: %w[accessibility_requirements_level_access_housing],
previous_page: "accessibility_requirements", page: "accessibility_requirements",
} }
end end
@ -88,7 +88,7 @@ RSpec.describe CaseLogsController, type: :controller do
accessibility_requirements_wheelchair_access_to_essential_rooms accessibility_requirements_wheelchair_access_to_essential_rooms
accessibility_requirements_level_access_housing], accessibility_requirements_level_access_housing],
tenant_code: tenant_code, tenant_code: tenant_code,
previous_page: "accessibility_requirements" } page: "accessibility_requirements" }
end end
let(:questions_for_page) do let(:questions_for_page) do
{ "accessibility_requirements" => { "accessibility_requirements" =>
@ -124,17 +124,21 @@ RSpec.describe CaseLogsController, type: :controller do
end end
context "conditional routing" do context "conditional routing" do
before do
allow_any_instance_of(CaseLogValidator).to receive(:validate_household_pregnancy).and_return(true)
end
let(:case_log_form_conditional_question_yes_params) do let(:case_log_form_conditional_question_yes_params) do
{ {
pregnancy: "Yes", pregnancy: "Yes",
previous_page: "conditional_question", page: "conditional_question",
} }
end end
let(:case_log_form_conditional_question_no_params) do let(:case_log_form_conditional_question_no_params) do
{ {
pregnancy: "No", pregnancy: "No",
previous_page: "conditional_question", page: "conditional_question",
} }
end end

72
spec/features/case_log_spec.rb

@ -32,10 +32,6 @@ RSpec.describe "Test Features" do
describe "Create new log" do describe "Create new log" do
it "redirects to the task list for the new log" do it "redirects to the task list for the new log" do
visit("/case_logs") visit("/case_logs")
# Ensure that we've finished creating both case logs before running the
# Capybara click part to ensure we don't get creation race conditions
expect(page).to have_link(nil, href: "/case_logs/#{case_log.id}")
expect(page).to have_link(nil, href: "/case_logs/#{empty_case_log.id}")
click_link("Create new log") click_link("Create new log")
id = CaseLog.order(created_at: :desc).first.id id = CaseLog.order(created_at: :desc).first.id
expect(page).to have_content("Tasklist for log #{id}") expect(page).to have_content("Tasklist for log #{id}")
@ -233,6 +229,15 @@ RSpec.describe "Test Features" do
expect(page).to have_current_path("/case_logs/#{id}/#{pages[index + 1]}") expect(page).to have_current_path("/case_logs/#{id}/#{pages[index + 1]}")
end end
end end
context "when changing an answer from the check answers page" do
it "the back button routes correctly" do
visit("/case_logs/#{id}/household_characteristics/check_answers")
first("a", text: /Answer/).click
click_link("Back")
expect(page).to have_current_path("/case_logs/#{id}/household_characteristics/check_answers")
end
end
end end
end end
@ -377,11 +382,66 @@ RSpec.describe "Test Features" do
end end
end end
describe "Soft Validation" do
context "given a weekly net income that is above the expected amount for the given economic status but below the hard max" do
let!(:case_log) { FactoryBot.create(:case_log, :in_progress, person_1_economic_status: "Full-time - 30 hours or more") }
let(:income_over_soft_limit) { 750 }
let(:income_under_soft_limit) { 700 }
it "prompts the user to confirm the value is correct" do
visit("/case_logs/#{case_log.id}/net_income")
fill_in("case-log-net-income-field", with: income_over_soft_limit)
choose("case-log-net-income-frequency-weekly-field", allow_label_click: true)
click_button("Save and continue")
expect(page).to have_content("Are you sure this is correct?")
check("case-log-override-net-income-validation-override-net-income-validation-field", allow_label_click: true)
click_button("Save and continue")
expect(page).to have_current_path("/case_logs/#{case_log.id}/net_income_uc_proportion")
end
it "does not require confirming the value if the value is amended" do
visit("/case_logs/#{case_log.id}/net_income")
fill_in("case-log-net-income-field", with: income_over_soft_limit)
choose("case-log-net-income-frequency-weekly-field", allow_label_click: true)
click_button("Save and continue")
fill_in("case-log-net-income-field", with: income_under_soft_limit)
click_button("Save and continue")
expect(page).to have_current_path("/case_logs/#{case_log.id}/net_income_uc_proportion")
case_log.reload
expect(case_log.override_net_income_validation).to be_nil
end
it "clears the confirmation question if the amount was amended and the page is returned to using the back button", js: true do
visit("/case_logs/#{case_log.id}/net_income")
fill_in("case-log-net-income-field", with: income_over_soft_limit)
choose("case-log-net-income-frequency-weekly-field", allow_label_click: true)
click_button("Save and continue")
fill_in("case-log-net-income-field", with: income_under_soft_limit)
click_button("Save and continue")
click_link(text: "Back")
expect(page).not_to have_content("Are you sure this is correct?")
end
it "does not clear the confirmation question if the page is returned to using the back button and the amount is still over the soft limit", js: true do
visit("/case_logs/#{case_log.id}/net_income")
fill_in("case-log-net-income-field", with: income_over_soft_limit)
choose("case-log-net-income-frequency-weekly-field", allow_label_click: true)
click_button("Save and continue")
check("case-log-override-net-income-validation-override-net-income-validation-field", allow_label_click: true)
click_button("Save and continue")
click_link(text: "Back")
expect(page).to have_content("Are you sure this is correct?")
end
end
end
describe "conditional page routing", js: true do describe "conditional page routing", js: true do
before do
allow_any_instance_of(CaseLogValidator).to receive(:validate_household_pregnancy).and_return(true)
end
it "can route the user to a different page based on their answer on the current page" do it "can route the user to a different page based on their answer on the current page" do
visit("case_logs/#{id}/conditional_question") visit("case_logs/#{id}/conditional_question")
# using a question name that is already in the db to avoid
# having to add a new column to the db for this test
choose("case-log-pregnancy-yes-field", allow_label_click: true) choose("case-log-pregnancy-yes-field", allow_label_click: true)
click_button("Save and continue") click_button("Save and continue")
expect(page).to have_current_path("/case_logs/#{id}/conditional_question_yes_page") expect(page).to have_current_path("/case_logs/#{id}/conditional_question_yes_page")

9
spec/fixtures/forms/test_form.json vendored

@ -328,6 +328,15 @@
"2": "Yearly" "2": "Yearly"
} }
} }
},
"soft_validations": {
"override_net_income_validation": {
"check_answer_label": "Net income confirmed?",
"type": "validation_override",
"answer_options": {
"override_net_income_validation": "Yes"
}
}
} }
}, },
"net_income_uc_proportion": { "net_income_uc_proportion": {

Loading…
Cancel
Save