From 02e5c2132d33709d7480dbc4af9457bdef4cd74d Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 22 Feb 2022 13:54:03 +0000
Subject: [PATCH] Change how we're reseting answers (#322)
* Reset values for dependant derived questions (only layear)
* do not reset questions that have selected answer options
* Fix test and assign nil to dependent
* rubocop
* update dependencies
---
Gemfile.lock | 16 ++++++------
app/models/case_log.rb | 29 ++++++++++++++++++---
app/models/form.rb | 7 ++++--
spec/factories/case_log.rb | 2 +-
spec/fixtures/exports/case_logs.xml | 2 +-
spec/fixtures/forms/2021_2022.json | 6 ++---
spec/models/case_log_spec.rb | 39 ++++++++++++++++++++++++++++-
spec/models/form_spec.rb | 4 +--
8 files changed, 82 insertions(+), 23 deletions(-)
diff --git a/Gemfile.lock b/Gemfile.lock
index 81ffe1343..61538bf99 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -23,7 +23,7 @@ GIT
GIT
remote: https://github.com/tagliala/activeadmin.git
- revision: a8ce826de172e3604b152d53c41aa68e2015ab80
+ revision: 000dd9425860fbadc72d4e08b893be098f5dff9b
branch: feature/railties-7
specs:
activeadmin (2.10.0)
@@ -111,8 +111,8 @@ GEM
ruby2_keywords (>= 0.0.2, < 1.0)
ast (2.4.2)
aws-eventstream (1.2.0)
- aws-partitions (1.554.0)
- aws-sdk-core (3.126.1)
+ aws-partitions (1.556.0)
+ aws-sdk-core (3.126.2)
aws-eventstream (~> 1, >= 1.0.2)
aws-partitions (~> 1, >= 1.525.0)
aws-sigv4 (~> 1.1)
@@ -254,11 +254,11 @@ GEM
net-protocol
timeout
nio4r (2.5.8)
- nokogiri (1.13.1-arm64-darwin)
+ nokogiri (1.13.3-arm64-darwin)
racc (~> 1.4)
- nokogiri (1.13.1-x86_64-darwin)
+ nokogiri (1.13.3-x86_64-darwin)
racc (~> 1.4)
- nokogiri (1.13.1-x86_64-linux)
+ nokogiri (1.13.3-x86_64-linux)
racc (~> 1.4)
notifications-ruby-client (5.3.0)
jwt (>= 1.5, < 3)
@@ -274,7 +274,7 @@ GEM
globalid
paper_trail (>= 3.0.0)
parallel (1.21.0)
- parser (3.1.0.0)
+ parser (3.1.1.0)
ast (~> 2.4.1)
pg (1.3.2)
postcodes_io (0.4.0)
@@ -419,7 +419,7 @@ GEM
simplecov_json_formatter (~> 0.1)
simplecov-html (0.12.3)
simplecov_json_formatter (0.1.4)
- stimulus-rails (1.0.2)
+ stimulus-rails (1.0.4)
railties (>= 6.0.0)
strscan (3.0.1)
thor (1.2.1)
diff --git a/app/models/case_log.rb b/app/models/case_log.rb
index 4c2ae50d7..21d60a872 100644
--- a/app/models/case_log.rb
+++ b/app/models/case_log.rb
@@ -213,12 +213,33 @@ private
end
end
+ def reset_not_routed_questions
+ form.invalidated_page_questions(self).each do |question|
+ enabled = form.enabled_page_questions(self)
+ contains_selected_answer_option = enabled.map(&:id).include?(question.id) && enabled.find { |q| q.id == question.id }.answer_options.values.map { |x| x["value"] }.include?(public_send(question.id))
+ if !contains_selected_answer_option && respond_to?(question.id.to_s)
+ public_send("#{question.id}=", nil)
+ end
+ end
+ end
+
+ def reset_derived_questions
+ dependent_questions = { layear: [{ key: :renewal, value: "No" }], homeless: [{ key: :renewal, value: "No" }] }
+
+ dependent_questions.each do |dependent, conditions|
+ condition_key = conditions.first[:key]
+ condition_value = conditions.first[:value]
+ if public_send("#{condition_key}_changed?") && condition_value == public_send(condition_key) && !public_send("#{dependent}_changed?")
+ self[dependent] = nil
+ end
+ end
+ end
+
def reset_invalidated_dependent_fields!
return unless form
- form.invalidated_page_questions(self).each do |question|
- public_send("#{question.id}=", nil) if respond_to?(question.id.to_s)
- end
+ reset_not_routed_questions
+ reset_derived_questions
end
def dynamically_not_required
@@ -256,11 +277,11 @@ private
end
self.has_benefits = get_has_benefits
self.nocharge = household_charge == "Yes" ? "No" : "Yes"
- self.layear = "Less than 1 year" if renewal == "Yes"
self.underoccupation_benefitcap = "No" if renewal == "Yes" && year == 2021
if renewal == "Yes"
self.homeless = "No"
self.referral = "Internal transfer"
+ self.layear = "Less than 1 year"
end
if needstype == "General needs"
self.prevten = "Fixed-term private registered provider (PRP) general needs tenancy" if managing_organisation.provider_type == "PRP"
diff --git a/app/models/form.rb b/app/models/form.rb
index c30771c74..b1b00082b 100644
--- a/app/models/form.rb
+++ b/app/models/form.rb
@@ -116,9 +116,12 @@ class Form
def invalidated_page_questions(case_log)
# we're already treating address fields as a special case and reset their values upon saving a case_log
address_questions = %w[postcode_known la previous_postcode_known prevloc property_postcode previous_postcode]
+ invalidated_pages(case_log).flat_map(&:questions).reject { |q| address_questions.include?(q.id) } || []
+ end
+
+ def enabled_page_questions(case_log)
pages_that_are_routed_to = pages.select { |p| p.routed_to?(case_log) }
- enabled_question_ids = pages_that_are_routed_to.flat_map(&:questions).map(&:id) || []
- invalidated_pages(case_log).flat_map(&:questions).reject { |q| enabled_question_ids.include?(q.id) || address_questions.include?(q.id) } || []
+ pages_that_are_routed_to.flat_map(&:questions) || []
end
def invalidated_conditional_questions(case_log)
diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb
index aea1e11d1..2a481dec1 100644
--- a/spec/factories/case_log.rb
+++ b/spec/factories/case_log.rb
@@ -110,7 +110,7 @@ FactoryBot.define do
net_income_known { "Yes" }
property_owner_organisation { "Test" }
property_manager_organisation { "Test" }
- renewal { 1 }
+ renewal { "No" }
rent_type { 1 }
intermediate_rent_product_name { 2 }
needstype { 1 }
diff --git a/spec/fixtures/exports/case_logs.xml b/spec/fixtures/exports/case_logs.xml
index ac2c9a2d5..70b68f4f0 100644
--- a/spec/fixtures/exports/case_logs.xml
+++ b/spec/fixtures/exports/case_logs.xml
@@ -97,7 +97,7 @@
Test
Test
- 1
+ No
1
2
798794
diff --git a/spec/fixtures/forms/2021_2022.json b/spec/fixtures/forms/2021_2022.json
index 6c7f5e546..94f76f963 100644
--- a/spec/fixtures/forms/2021_2022.json
+++ b/spec/fixtures/forms/2021_2022.json
@@ -396,9 +396,6 @@
"answer_options": {
"0": {
"value": "Yes"
- },
- "1": {
- "value": "No"
}
}
}
@@ -725,7 +722,8 @@
}
}
}
- }
+ },
+ "depends_on": [{ "renewal": "No" }]
},
"time_on_la_waiting_list": {
"questions": {
diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb
index 0f96f1563..89875c2b4 100644
--- a/spec/models/case_log_spec.rb
+++ b/spec/models/case_log_spec.rb
@@ -532,9 +532,17 @@ RSpec.describe CaseLog do
context "with two pages having the same question key, only one's dependency is met" do
let(:case_log) { FactoryBot.create(:case_log, :in_progress, cbl: "Yes", preg_occ: "No") }
- it "does not clear the answer" do
+ it "does not clear the value for answers that apply to both pages" do
expect(case_log.cbl).to eq("Yes")
end
+
+ it "does clear the value for answers that do not apply for invalidated page" do
+ case_log.update!({ wchair: "Yes", sex2: "Female", age2: 33 })
+ case_log.update!({ cbl: "No" })
+ case_log.update!({ preg_occ: "Yes" })
+
+ expect(case_log.cbl).to eq(nil)
+ end
end
context "when the case log does not have a valid form set yet" do
@@ -544,6 +552,35 @@ RSpec.describe CaseLog do
expect { case_log.update(startdate: Time.zone.local(2015, 1, 1)) }.not_to raise_error
end
end
+
+ context "when it changes from a renewal to not a renewal" do
+ let(:case_log) { FactoryBot.create(:case_log) }
+
+ it "resets inferred layear value" do
+ case_log.update!({ renewal: "Yes" })
+
+ record_from_db = ActiveRecord::Base.connection.execute("select layear from case_logs where id=#{case_log.id}").to_a[0]
+ expect(record_from_db["layear"]).to eq(2)
+ expect(case_log["layear"]).to eq("Less than 1 year")
+
+ case_log.update!({ renewal: "No" })
+ record_from_db = ActiveRecord::Base.connection.execute("select layear from case_logs where id=#{case_log.id}").to_a[0]
+ expect(record_from_db["layear"]).to eq(nil)
+ expect(case_log["layear"]).to eq(nil)
+ end
+ end
+
+ context "when it is not a renewal" do
+ let(:case_log) { FactoryBot.create(:case_log) }
+
+ it "saves layear value" do
+ case_log.update!({ renewal: "No", layear: "1 year but under 2 years" })
+
+ record_from_db = ActiveRecord::Base.connection.execute("select layear from case_logs where id=#{case_log.id}").to_a[0]
+ expect(record_from_db["layear"]).to eq(7)
+ expect(case_log["layear"]).to eq("1 year but under 2 years")
+ end
+ end
end
describe "paper trail" do
diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb
index 685a20212..131b97bcc 100644
--- a/spec/models/form_spec.rb
+++ b/spec/models/form_spec.rb
@@ -131,7 +131,7 @@ RSpec.describe Form, type: :model do
describe "invalidated_page_questions" do
context "when dependencies are not met" do
- let(:expected_invalid) { %w[la_known cbl conditional_question_no_second_question dependent_question declaration] }
+ let(:expected_invalid) { %w[la_known cbl conditional_question_no_second_question dependent_question layear declaration] }
it "returns an array of question keys whose pages conditions are not met" do
expect(form.invalidated_page_questions(case_log).map(&:id).uniq).to eq(expected_invalid)
@@ -139,7 +139,7 @@ RSpec.describe Form, type: :model do
end
context "with two pages having the same question and only one has dependencies met" do
- let(:expected_invalid) { %w[la_known conditional_question_no_second_question dependent_question declaration] }
+ let(:expected_invalid) { %w[la_known cbl conditional_question_no_second_question dependent_question layear declaration] }
it "returns an array of question keys whose pages conditions are not met" do
case_log["preg_occ"] = "No"