From 4e924afbf7d96c9fce75da62249e107b900aa998 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 24 Sep 2021 10:41:02 +0100 Subject: [PATCH 01/26] add new trait to case log factory --- spec/factories/case_log.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index e2f132c53..41c92a3b3 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -11,6 +11,16 @@ FactoryBot.define do tenant_code { "BZ737" } postcode { "NW1 7TY" } end + trait :near_check_answers_household_characteristics do + status { 0 } + tenant_code { "AB123" } + postcode { "LE11 2DW" } + tenant_age { 25 } + tenant_gender { "Male" } + tenant_ethnic_group { "White: English/Scottish/Welsh/Northern Irish/British" } + tenant_nationality { "UK national resident in UK" } + tenant_economic_status { "Part-time - Less than 30 hours" } + end created_at { Time.zone.now } updated_at { Time.zone.now } end From a04a31105c4bbd304c7846e4e2f5b42a6e842906 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 24 Sep 2021 16:54:33 +0100 Subject: [PATCH 02/26] add check answers page with passing test --- app/controllers/case_logs_controller.rb | 5 +++++ app/views/form/check_answers.html.erb | 6 ++++++ config/routes.rb | 6 +++++- spec/factories/case_log.rb | 4 ++-- spec/features/case_log_spec.rb | 19 +++++++++++++++++++ 5 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 app/views/form/check_answers.html.erb diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index aa6982ac2..6545b9a9c 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -31,6 +31,11 @@ class CaseLogsController < ApplicationController redirect_to(send("case_log_#{next_page}_path", @case_log)) end + def check_answers + @case_log = CaseLog.find(params[:case_log_id]) + render "form/check_answers", locals: { case_log_id: @case_log.id } + end + form = Form.new(2021, 2022) form.all_pages.map do |page_key, page_info| define_method(page_key) do diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb new file mode 100644 index 000000000..5f1310211 --- /dev/null +++ b/app/views/form/check_answers.html.erb @@ -0,0 +1,6 @@ +<%= turbo_frame_tag "case_log_form", target: "_top" do %> +

Check the answers you gave for household characteristics

+ <%= form_with action: '/case_logs', method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> + <%= f.govuk_submit "Save and continue" %> + <% end %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index 6443b9d35..99d2c48ab 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,10 +3,14 @@ Rails.application.routes.draw do get "about", to: "about#index" get "/", to: "test#index" + form = Form.new(2021, 2022) resources :case_logs do - Form.new(2021, 2022).all_pages.keys.map do |page| + form.all_pages.keys.map do |page| get page.to_s, to: "case_logs##{page}" post page.to_s, to: "case_logs#next_page" + form.all_subsections.keys.map do |subsection| + get "#{subsection}/check_answers", to: "case_logs#check_answers" + end end end end diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index 41c92a3b3..48c6c33a5 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -15,12 +15,12 @@ FactoryBot.define do status { 0 } tenant_code { "AB123" } postcode { "LE11 2DW" } - tenant_age { 25 } + tenant_age { 25 } tenant_gender { "Male" } tenant_ethnic_group { "White: English/Scottish/Welsh/Northern Irish/British" } tenant_nationality { "UK national resident in UK" } tenant_economic_status { "Part-time - Less than 30 hours" } - end + end created_at { Time.zone.now } updated_at { Time.zone.now } end diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 4efc07f8b..3c7ccd498 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -1,6 +1,7 @@ require "rails_helper" RSpec.describe "Test Features" do let!(:case_log) { FactoryBot.create(:case_log, :in_progress) } + let!(:check_answers_case_log) { FactoryBot.create(:case_log, :near_check_answers_household_characteristics) } let(:id) { case_log.id } let(:status) { case_log.status } @@ -96,4 +97,22 @@ RSpec.describe "Test Features" do end end end + + describe "check answers page" do + let(:subsection) { "household_characteristics" } + + context "only one questions remains to be answered for the household characteristics section" do + # it "redirects to the check answers page when answering the last question and clicking save and continue" do + # visit("/case_logs/#{check_answers_case_log.id}/household_number_of_other_members") + # fill_in("household_number_of_other_members", with: 0) + # click_button("Save and continue") + # expect(page).to have_current_path("/case_logs/#{check_answers_case_log.id}/check-answers") + # end + + it "can be visited by URL" do + visit("case_logs/#{case_log.id}/#{subsection}/check_answers") + expect(page).to have_content("Check the answers you gave for #{subsection.tr('_', ' ')}") + end + end + end end From ed9ce2e137e392edb4ca0647909600c4b24bc092 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Tue, 28 Sep 2021 09:39:50 +0100 Subject: [PATCH 03/26] Get a second passing test in place Co-authored-by: Daniel Baark --- app/controllers/case_logs_controller.rb | 9 ++++++++- app/models/form.rb | 2 +- spec/factories/case_log.rb | 10 ---------- spec/features/case_log_spec.rb | 20 ++++++++++---------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 6545b9a9c..e1e2d8890 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -28,7 +28,14 @@ class CaseLogsController < ApplicationController answers_for_page = page_params(questions_for_page).select { |k, _v| questions_for_page.include?(k) } @case_log.update!(answers_for_page) next_page = form.next_page(previous_page) - redirect_to(send("case_log_#{next_page}_path", @case_log)) + 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 + + redirect_to(send(redirect_path, @case_log)) end def check_answers diff --git a/app/models/form.rb b/app/models/form.rb index 1be383ea0..00c3503af 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -50,7 +50,7 @@ class Form def next_page(previous_page) subsection = subsection_for_page(previous_page) previous_page_idx = pages_for_subsection(subsection).keys.index(previous_page) - pages_for_subsection(subsection).keys[previous_page_idx + 1] || previous_page # Placeholder until we have a check answers page + pages_for_subsection(subsection).keys[previous_page_idx + 1] || :check_answers end def previous_page(current_page) diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index 48c6c33a5..e2f132c53 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -11,16 +11,6 @@ FactoryBot.define do tenant_code { "BZ737" } postcode { "NW1 7TY" } end - trait :near_check_answers_household_characteristics do - status { 0 } - tenant_code { "AB123" } - postcode { "LE11 2DW" } - tenant_age { 25 } - tenant_gender { "Male" } - tenant_ethnic_group { "White: English/Scottish/Welsh/Northern Irish/British" } - tenant_nationality { "UK national resident in UK" } - tenant_economic_status { "Part-time - Less than 30 hours" } - end created_at { Time.zone.now } updated_at { Time.zone.now } end diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 3c7ccd498..bc76a9231 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -1,7 +1,6 @@ require "rails_helper" RSpec.describe "Test Features" do let!(:case_log) { FactoryBot.create(:case_log, :in_progress) } - let!(:check_answers_case_log) { FactoryBot.create(:case_log, :near_check_answers_household_characteristics) } let(:id) { case_log.id } let(:status) { case_log.status } @@ -101,18 +100,19 @@ RSpec.describe "Test Features" do describe "check answers page" do let(:subsection) { "household_characteristics" } - context "only one questions remains to be answered for the household characteristics section" do - # it "redirects to the check answers page when answering the last question and clicking save and continue" do - # visit("/case_logs/#{check_answers_case_log.id}/household_number_of_other_members") - # fill_in("household_number_of_other_members", with: 0) - # click_button("Save and continue") - # expect(page).to have_current_path("/case_logs/#{check_answers_case_log.id}/check-answers") - # end - + context "when the user needs to check their answers for a subsection" do it "can be visited by URL" do - visit("case_logs/#{case_log.id}/#{subsection}/check_answers") + visit("case_logs/#{id}/#{subsection}/check_answers") expect(page).to have_content("Check the answers you gave for #{subsection.tr('_', ' ')}") end + + let(:last_question_for_subsection) { "household_number_of_other_members" } + it "redirects to the check answers page when answering the last question and clicking save and continue" do + visit("/case_logs/#{id}/#{last_question_for_subsection}") + fill_in(last_question_for_subsection, with: 0) + click_button("Save and continue") + expect(page).to have_current_path("/case_logs/#{id}/#{subsection}/check_answers") + end end end end From c73074bc0e62ba25ab15ccbaaf98f12348cc7d5a Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Wed, 29 Sep 2021 12:20:00 +0100 Subject: [PATCH 04/26] Add test for check answers questions labels --- app/controllers/case_logs_controller.rb | 6 +++++- app/views/form/check_answers.html.erb | 9 +++++++++ config/forms/2021_2022.json | 7 +++++++ spec/features/case_log_spec.rb | 11 +++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index e1e2d8890..54afdeffe 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -40,7 +40,11 @@ class CaseLogsController < ApplicationController def check_answers @case_log = CaseLog.find(params[:case_log_id]) - render "form/check_answers", locals: { case_log_id: @case_log.id } + form = Form.new(2021, 2022) + current_url = request.env['PATH_INFO'] + subsection = current_url.split('/')[-2] + subsection_pages = form.pages_for_subsection(subsection) + render "form/check_answers", locals: { case_log_id: @case_log.id, subsection_pages: subsection_pages } end form = Form.new(2021, 2022) diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 5f1310211..f9122a0a2 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -1,5 +1,14 @@ <%= turbo_frame_tag "case_log_form", target: "_top" do %>

Check the answers you gave for household characteristics

+ <% subsection_pages.each do |page, page_info| %> +
+
+
+ <%= page_info["check_answer_label"].to_s %> +
+
+
+ <% end %> <%= form_with action: '/case_logs', method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> <%= f.govuk_submit "Save and continue" %> <% end %> diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 83d80674a..9e84b9e55 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -10,6 +10,7 @@ "label": "Household characteristics", "pages": { "tenant_code":{ + "check_answer_label": "Tenant code", "header": "", "description": "", "questions":{ @@ -21,6 +22,7 @@ } }, "tenant_age":{ + "check_answer_label": "Tenant's age", "header": "", "description": "", "questions":{ @@ -35,6 +37,7 @@ } }, "tenant_gender":{ + "check_answer_label": "Tenant's gender", "header": "", "description": "", "questions":{ @@ -52,6 +55,7 @@ } }, "tenant_ethnic_group":{ + "check_answer_label": "Ethnicity", "header": "", "description": "", "questions":{ @@ -84,6 +88,7 @@ } }, "tenant_nationality":{ + "check_answer_label": "Nationality", "header": "", "description": "", "questions":{ @@ -113,6 +118,7 @@ } }, "tenant_economic_status":{ + "check_answer_label": "Work", "header": "", "description": "", "questions":{ @@ -137,6 +143,7 @@ } }, "household_number_of_other_members":{ + "check_answer_label": "Number of Other Household Members", "header": "", "description": "", "questions":{ diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index bc76a9231..2501ed1a4 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -113,6 +113,17 @@ RSpec.describe "Test Features" do click_button("Save and continue") expect(page).to have_current_path("/case_logs/#{id}/#{subsection}/check_answers") end + + it "has question headings based on the subsection" do + visit("case_logs/#{id}/#{subsection}/check_answers") + expect(page).to have_content("Tenant code") + expect(page).to have_content("Tenant's age") + expect(page).to have_content("Tenant's gender") + expect(page).to have_content("Ethnicity") + expect(page).to have_content("Nationality") + expect(page).to have_content("Work") + expect(page).to have_content("Number of Other Household Members") + end end end end From fe43a1e99910b23af8286dc5dbc3776f9d2687e4 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Wed, 29 Sep 2021 15:25:58 +0100 Subject: [PATCH 05/26] Display the answers in the answer check page Save radio button and checkbox string values instead of the key/number in the db. ADR will be added to explain why we are saving values directly to the db instead of using enums with active record to convert the key/numbers to the appropriate strings. --- app/controllers/case_logs_controller.rb | 2 +- app/views/form/_checkbox_question.html.erb | 2 +- app/views/form/_radio_question.html.erb | 4 ++-- app/views/form/check_answers.html.erb | 3 +++ spec/features/case_log_spec.rb | 23 ++++++++++++++++------ 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 54afdeffe..b34ab0740 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -44,7 +44,7 @@ class CaseLogsController < ApplicationController current_url = request.env['PATH_INFO'] subsection = current_url.split('/')[-2] subsection_pages = form.pages_for_subsection(subsection) - render "form/check_answers", locals: { case_log_id: @case_log.id, subsection_pages: subsection_pages } + render "form/check_answers", locals: { case_log_id: @case_log.id, case_log: @case_log, subsection_pages: subsection_pages } end form = Form.new(2021, 2022) diff --git a/app/views/form/_checkbox_question.html.erb b/app/views/form/_checkbox_question.html.erb index e41ab5029..0a958fb0e 100644 --- a/app/views/form/_checkbox_question.html.erb +++ b/app/views/form/_checkbox_question.html.erb @@ -6,7 +6,7 @@ <% if key.starts_with?("divider") %> <%= f.govuk_check_box_divider %> <% else %> - <%= f.govuk_check_box question_key, key, label: { text: val } %> + <%= f.govuk_check_box question_key, val, label: { text: val } %> <% end %> <% end %> <% end %> diff --git a/app/views/form/_radio_question.html.erb b/app/views/form/_radio_question.html.erb index 3681bb21d..e525cf159 100644 --- a/app/views/form/_radio_question.html.erb +++ b/app/views/form/_radio_question.html.erb @@ -6,8 +6,8 @@ <% question["answer_options"].map do |key, val| %> <% if key.starts_with?("divider") %> <%= f.govuk_radio_divider %> - <% else %> - <%= f.govuk_radio_button question_key, key, label: { text: val } %> + <% else %> + <%= f.govuk_radio_button question_key, val, label: { text: val } %> <% end %> <% end %> <% end %> diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index f9122a0a2..7efc276b6 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -6,6 +6,9 @@
<%= page_info["check_answer_label"].to_s %>
+
+ <%= case_log[page] %> +
<% end %> diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 2501ed1a4..8aa7aa153 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -7,10 +7,10 @@ RSpec.describe "Test Features" do question_answers = { tenant_code: { type: "text", answer: "BZ737" }, tenant_age: { type: "numeric", answer: 25 }, - tenant_gender: { type: "radio", answer: "1" }, - tenant_ethnic_group: { type: "radio", answer: "2" }, - tenant_nationality: { type: "radio", answer: "0" }, - tenant_economic_status: { type: "radio", answer: "4" }, + tenant_gender: { type: "radio", answer: "Female" }, + tenant_ethnic_group: { type: "radio", answer: "Prefer not to say" }, + tenant_nationality: { type: "radio", answer: "Lithuania" }, + tenant_economic_status: { type: "radio", answer: "Jobseeker" }, household_number_of_other_members: { type: "numeric", answer: 2 }, } @@ -37,7 +37,7 @@ RSpec.describe "Test Features" do click_button("Save and continue") expect(page).to have_field("tenant-age-field") click_button("Save and continue") - expect(page).to have_field("tenant-gender-0-field") + expect(page).to have_field("tenant-gender-male-field") visit page.driver.request.env["HTTP_REFERER"] expect(page).to have_field("tenant-age-field") end @@ -58,7 +58,7 @@ RSpec.describe "Test Features" do when "text" fill_in(question.to_s, with: answer) when "radio" - choose("#{question.to_s.tr('_', '-')}-#{answer}-field") + choose("#{question.to_s.tr('_', '-')}-#{answer.parameterize}-field") else fill_in(question.to_s, with: answer) end @@ -123,6 +123,17 @@ RSpec.describe "Test Features" do expect(page).to have_content("Nationality") expect(page).to have_content("Work") expect(page).to have_content("Number of Other Household Members") + end + + it "should display answers given by the user for the question in the subsection" do + visit("/case_logs/#{id}/tenant_age") + fill_in("tenant_age", with: 28) + click_button("Save and continue") + choose("tenant-gender-non-binary-field") + click_button("Save and continue") + visit("/case_logs/#{id}/#{subsection}/check_answers") + expect(page).to have_content("28") + expect(page).to have_content("Non-binary") end end end From fc132c452c2617026573264a4176b0ad63b7ed99 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Wed, 29 Sep 2021 16:21:40 +0100 Subject: [PATCH 06/26] Add ADR about saving values to database --- doc/adr/adr-005.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 doc/adr/adr-005.md diff --git a/doc/adr/adr-005.md b/doc/adr/adr-005.md new file mode 100644 index 000000000..7e5b370b8 --- /dev/null +++ b/doc/adr/adr-005.md @@ -0,0 +1,23 @@ +### ADR - 005: Saving values to the database + +We have opted to save values to the database directly instead of saving keys/numbers that need to be converted with enums in models using active record. + +### Saving values to the database + +There are a few reasons we have opted to save the values directly, they are as follows + +- The data will be easier to consume and analyse for anyone associated with the project who needs to do so but does not necessarily have the technical skills to access it through Rails i.e. A person could get the data directly from the database and it would require no additional work to be usable for reporting purposes. + +- Currently there is no need to abstract the data as the data should be safe from being accessed by anyone external to the project. + +- It doesn't require additional dev work to map keys/numbers to values, we can just pull the values out directly and use them in the code, for example on the check answers page. + + + +### Drawbacks + +- changing the wording/casing of the answers could result in discrepancies in the database + +- There is a small risk that if the database is accessed by someone unauthorised they would have access to personally identifiable information. + +This decision is not too difficult to change and can be revisited in the future if there is sufficient reason to switch to storing keys/numbers and using enums and active record to convert those to the appropriate values. \ No newline at end of file From 82e87483b959d6f567220b93e865a4cd87b1cb47 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Wed, 29 Sep 2021 16:58:08 +0100 Subject: [PATCH 07/26] add specs for change/answer links on check answers --- app/views/form/check_answers.html.erb | 7 +++++++ spec/features/case_log_spec.rb | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 7efc276b6..1c9de803a 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -9,6 +9,13 @@
<%= case_log[page] %>
+
+ <% if case_log[page].blank? %> + <%= link_to "Answer", "/case_logs/#{case_log_id}/#{page}", class: "govuk-link" %> + <% else %> + <%= link_to "Change", "/case_logs/#{case_log_id}/#{page}", class: "govuk-link" %> + <% end %> +
<% end %> diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 8aa7aa153..95ac523eb 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -1,6 +1,7 @@ require "rails_helper" RSpec.describe "Test Features" do let!(:case_log) { FactoryBot.create(:case_log, :in_progress) } + let!(:empty_case_log) { FactoryBot.create(:case_log) } let(:id) { case_log.id } let(:status) { case_log.status } @@ -135,6 +136,23 @@ RSpec.describe "Test Features" do expect(page).to have_content("28") expect(page).to have_content("Non-binary") end + + it "should have an answer link for questions missing an answer" do + visit("case_logs/#{empty_case_log.id}/#{subsection}/check_answers") + assert_selector "a", text: "Answer", count: 7 + assert_selector "a", text: "Change", count: 0 + expect(page).to have_link('Answer', href: "/case_logs/#{empty_case_log.id}/tenant_age") + end + + it "should have a change link for answered questions" do + visit("/case_logs/#{empty_case_log.id}/tenant_age") + fill_in("tenant_age", with: 28) + click_button("Save and continue") + visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") + assert_selector "a", text: "Answer", count: 6 + assert_selector "a", text: "Change", count: 1 + expect(page).to have_link('Change', href: "/case_logs/#{empty_case_log.id}/tenant_age") + end end end end From 78df07b29bb6dd04566a9157bcfcaaa38c718df4 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Thu, 30 Sep 2021 09:31:16 +0100 Subject: [PATCH 08/26] Improve test code with minor refactors Co-authored-by: Katrina Kosiak --- spec/features/case_log_spec.rb | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 95ac523eb..861d9d49e 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -102,6 +102,12 @@ RSpec.describe "Test Features" do let(:subsection) { "household_characteristics" } 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}", with: value) + click_button("Save and continue") + end + it "can be visited by URL" do visit("case_logs/#{id}/#{subsection}/check_answers") expect(page).to have_content("Check the answers you gave for #{subsection.tr('_', ' ')}") @@ -109,30 +115,23 @@ RSpec.describe "Test Features" do let(:last_question_for_subsection) { "household_number_of_other_members" } it "redirects to the check answers page when answering the last question and clicking save and continue" do - visit("/case_logs/#{id}/#{last_question_for_subsection}") - fill_in(last_question_for_subsection, with: 0) - click_button("Save and continue") + fill_in_number_question(id, last_question_for_subsection, 0) expect(page).to have_current_path("/case_logs/#{id}/#{subsection}/check_answers") end it "has question headings based on the subsection" do visit("case_logs/#{id}/#{subsection}/check_answers") - expect(page).to have_content("Tenant code") - expect(page).to have_content("Tenant's age") - expect(page).to have_content("Tenant's gender") - expect(page).to have_content("Ethnicity") - expect(page).to have_content("Nationality") - expect(page).to have_content("Work") - expect(page).to have_content("Number of Other Household Members") + question_labels = ["Tenant code", "Tenant's age", "Tenant's gender", "Ethnicity", "Nationality", "Work", "Number of Other Household Members"] + question_labels.each do |label| + expect(page).to have_content(label) + end end it "should display answers given by the user for the question in the subsection" do - visit("/case_logs/#{id}/tenant_age") - fill_in("tenant_age", with: 28) - click_button("Save and continue") + fill_in_number_question(empty_case_log.id, "tenant_age", 28) choose("tenant-gender-non-binary-field") click_button("Save and continue") - visit("/case_logs/#{id}/#{subsection}/check_answers") + visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") expect(page).to have_content("28") expect(page).to have_content("Non-binary") end @@ -145,9 +144,7 @@ RSpec.describe "Test Features" do end it "should have a change link for answered questions" do - visit("/case_logs/#{empty_case_log.id}/tenant_age") - fill_in("tenant_age", with: 28) - click_button("Save and continue") + fill_in_number_question(empty_case_log.id, "tenant_age", 28) visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") assert_selector "a", text: "Answer", count: 6 assert_selector "a", text: "Change", count: 1 From c07b3f34fef35699f262e6418eca4686605e64c0 Mon Sep 17 00:00:00 2001 From: Kat Date: Thu, 30 Sep 2021 11:03:57 +0100 Subject: [PATCH 09/26] Add check_answer_label for the rest of the pages --- config/forms/2021_2022.json | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 9e84b9e55..0966a9bdc 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -421,6 +421,7 @@ "tenancy_code":{ "header": "", "description": "", + "check_answer_label": "What is the tenancy code?", "questions":{ "tenancy_code": { "header": "What is the tenancy code?", @@ -432,6 +433,7 @@ "tenancy_start_date": { "header": "", "description": "", + "check_answer_label": "When is the tenancy start date?", "questions": { "tenancy_start_date": { "header": "What is the tenancy start date?", @@ -443,6 +445,7 @@ "starter_tenancy": { "header": "", "description": "", + "check_answer_label": "Is this a starter or introductory tenancy?", "questions": { "starter_tenancy": { "header": "Is this a starter tenancy?", @@ -458,6 +461,7 @@ "fixed_term_tenancy":{ "header": "", "description": "", + "check_answer_label": "If the main tenancy is a fixed term tenancy, please provide the length of the fixed term (to the nearest year) excluding any starter/introductory period", "questions":{ "fixed_term_tenancy": { "header": "If fixed-term, what is the length of the fixed-term tenancy after any starter period?", @@ -472,6 +476,7 @@ "tenancy_type": { "header": "", "description": "", + "check_answer_label": "Type of main tenancy (after any starter/introductory period)", "questions": { "tenancy_type": { "header": "What is the type of tenancy after the starter period has ended?", @@ -491,6 +496,7 @@ "letting_type": { "header": "", "description": "", + "check_answer_label": "Type of letting", "questions": { "letting_type": { "header": "Which type of letting is this?", @@ -510,6 +516,7 @@ "letting_provider": { "header": "", "description": "", + "check_answer_label": "Provider", "questions": { "letting_provider": { "header": "Who is the letting provider?", @@ -530,6 +537,7 @@ "property_location":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "property_location": { "header": "Property location", @@ -1020,6 +1028,7 @@ "net_income":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "net_income": { "header": "What is the tenant’s /and partner’s combined income after tax?", @@ -1043,6 +1052,7 @@ "net_income_uc_proportion":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "net_income_uc_proportion": { "header": "How much of the tenant’s income is from Universal Credit, state pensions or benefits?", @@ -1060,6 +1070,7 @@ "housing_benefit":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "housing_benefit": { "header": "Is the tenant likely to be in receipt of any of these housing-related benefits?", @@ -1085,6 +1096,7 @@ "rent":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "rent_frequency": { "header": "Which period are rent and other charges due?", @@ -1162,6 +1174,7 @@ "time_lived_in_la":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "time_lived_in_la": { "header": "How long has the household continuously lived in the local authority area where the new letting is located?", @@ -1183,6 +1196,7 @@ "time_on_la_waiting_list":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "time_on_la_waiting_list": { "header": "How long has the household been on the local authority waiting list where the new letting is located?", @@ -1204,6 +1218,7 @@ "previous_la":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "previous_la": { "header": "Which local authority area did the household live in immediately before this letting?", @@ -1530,6 +1545,7 @@ "previous_postcode": { "header": "", "description": "", + "check_answer_label": "", "questions": { "previous_postcode": { "header": "Postcode for the previous accommodation", @@ -1541,6 +1557,7 @@ "reasonable_preference": { "header": "", "description": "", + "check_answer_label": "", "questions": { "reasonable_preference": { "header": "Was the household given reasonable preference by the local authority?", @@ -1569,6 +1586,7 @@ "lettings_policy": { "header": "", "description": "", + "check_answer_label": "", "questions": { "cbl_letting": { "header": "Was the letting made under choice-based lettings (CBL)? ", @@ -1612,6 +1630,7 @@ "declaration":{ "header": "", "description": "", + "check_answer_label": "", "questions":{ "declaration": { "header": "What is the tenant code?", From a436a6cae21af66f34259eacb0ecaee0953a2b9e Mon Sep 17 00:00:00 2001 From: Kat Date: Thu, 30 Sep 2021 14:44:31 +0100 Subject: [PATCH 10/26] Add check answers helper, display number of questions answered and a link to the next unanswered question --- app/helpers/check_answers_helper.rb | 23 ++++++++++++++++++++ app/views/form/check_answers.html.erb | 9 ++++---- spec/features/case_log_spec.rb | 30 ++++++++++++++++++++------- 3 files changed, 49 insertions(+), 13 deletions(-) create mode 100644 app/helpers/check_answers_helper.rb diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb new file mode 100644 index 000000000..883105de9 --- /dev/null +++ b/app/helpers/check_answers_helper.rb @@ -0,0 +1,23 @@ +module CheckAnswersHelper + def get_answered_questions_total(subsection_pages, case_log) + questions = [] + subsection_pages.keys.each do |page| + questions << page + end + return questions.count {|question| !(case_log[question].blank?)} + end + + + def create_update_answer_link(case_log_answer, case_log_id, page) + link_name = case_log_answer.blank? ? "Answer" : "Change" + link_to(link_name, "/case_logs/#{case_log_id}/#{page}", class: "govuk-link").html_safe + end + + def create_next_missing_question_link(case_log_id, subsections, case_log) + empty_question = subsections.keys.find{|x| case_log[x].blank? } + + url = "/case_logs/#{case_log_id}/#{empty_question}" + link_to('Answer the missing questions', url, class: "govuk-link").html_safe + end + +end diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 1c9de803a..8df10dcc8 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -1,5 +1,7 @@ <%= turbo_frame_tag "case_log_form", target: "_top" do %>

Check the answers you gave for household characteristics

+

You answered <%= get_answered_questions_total(subsection_pages, case_log) %> of <%= subsection_pages.count %> questions

+ <%= create_next_missing_question_link(case_log_id, subsection_pages, case_log) %> <% subsection_pages.each do |page, page_info| %>
@@ -10,11 +12,7 @@ <%= case_log[page] %>
- <% if case_log[page].blank? %> - <%= link_to "Answer", "/case_logs/#{case_log_id}/#{page}", class: "govuk-link" %> - <% else %> - <%= link_to "Change", "/case_logs/#{case_log_id}/#{page}", class: "govuk-link" %> - <% end %> + <%= create_update_answer_link(case_log[page], case_log_id, page)%>
@@ -23,3 +21,4 @@ <%= f.govuk_submit "Save and continue" %> <% end %> <% end %> + diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 861d9d49e..9f0e5e643 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -124,9 +124,9 @@ RSpec.describe "Test Features" do question_labels = ["Tenant code", "Tenant's age", "Tenant's gender", "Ethnicity", "Nationality", "Work", "Number of Other Household Members"] question_labels.each do |label| expect(page).to have_content(label) - end + end end - + 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") @@ -134,22 +134,36 @@ RSpec.describe "Test Features" do visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") expect(page).to have_content("28") expect(page).to have_content("Non-binary") - end - + end + it "should have an answer link for questions missing an answer" do visit("case_logs/#{empty_case_log.id}/#{subsection}/check_answers") - assert_selector "a", text: "Answer", count: 7 + assert_selector "a", text: /Answer\z/, count: 7 assert_selector "a", text: "Change", count: 0 expect(page).to have_link('Answer', href: "/case_logs/#{empty_case_log.id}/tenant_age") end - it "should have a change link for answered questions" do + it "should have a change link for answered questions" do fill_in_number_question(empty_case_log.id, "tenant_age", 28) visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") - assert_selector "a", text: "Answer", count: 6 + assert_selector "a", text: /Answer\z/, count: 6 assert_selector "a", text: "Change", count: 1 expect(page).to have_link('Change', href: "/case_logs/#{empty_case_log.id}/tenant_age") - end + end + + it "should have a link pointing to the first question if no questions are answered" do + visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") + expect(page).to have_content('You answered 0 of 7 questions') + expect(page).to have_link('Answer the missing questions', href: "/case_logs/#{empty_case_log.id}/tenant_code") + end + + it "should have a link pointing to the next empty question if some questions are answered" do + fill_in_number_question(empty_case_log.id, "tenant_code", 0) + + visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") + expect(page).to have_content('You answered 1 of 7 questions') + expect(page).to have_link('Answer the missing questions', href: "/case_logs/#{empty_case_log.id}/tenant_age") + end end end end From e086f1c13177b8a790445c72504cea1263ac9721 Mon Sep 17 00:00:00 2001 From: Kat Date: Thu, 30 Sep 2021 16:07:25 +0100 Subject: [PATCH 11/26] Extract partial answers table, rename variable in helper method --- app/controllers/case_logs_controller.rb | 8 ++++---- app/helpers/check_answers_helper.rb | 4 ++-- app/views/form/_check_answers_table.html.erb | 13 +++++++++++++ app/views/form/check_answers.html.erb | 17 ++--------------- spec/features/case_log_spec.rb | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-) create mode 100644 app/views/form/_check_answers_table.html.erb diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index b34ab0740..f162f1f64 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -28,13 +28,13 @@ class CaseLogsController < ApplicationController answers_for_page = page_params(questions_for_page).select { |k, _v| questions_for_page.include?(k) } @case_log.update!(answers_for_page) next_page = form.next_page(previous_page) - redirect_path = if next_page == :check_answers + redirect_path = if next_page == :check_answers subsection = form.subsection_for_page(previous_page) "case_log_#{subsection}_check_answers_path" - else + else "case_log_#{next_page}_path" end - + redirect_to(send(redirect_path, @case_log)) end @@ -44,7 +44,7 @@ class CaseLogsController < ApplicationController current_url = request.env['PATH_INFO'] subsection = current_url.split('/')[-2] subsection_pages = form.pages_for_subsection(subsection) - render "form/check_answers", locals: { case_log_id: @case_log.id, case_log: @case_log, subsection_pages: subsection_pages } + render "form/check_answers", locals: { case_log: @case_log, subsection_pages: subsection_pages } end form = Form.new(2021, 2022) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 883105de9..464f42fab 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -13,8 +13,8 @@ module CheckAnswersHelper link_to(link_name, "/case_logs/#{case_log_id}/#{page}", class: "govuk-link").html_safe end - def create_next_missing_question_link(case_log_id, subsections, case_log) - empty_question = subsections.keys.find{|x| case_log[x].blank? } + def create_next_missing_question_link(case_log_id, subsection_pages, case_log) + empty_question = subsection_pages.keys.find{|x| case_log[x].blank? } url = "/case_logs/#{case_log_id}/#{empty_question}" link_to('Answer the missing questions', url, class: "govuk-link").html_safe diff --git a/app/views/form/_check_answers_table.html.erb b/app/views/form/_check_answers_table.html.erb new file mode 100644 index 000000000..b4551f622 --- /dev/null +++ b/app/views/form/_check_answers_table.html.erb @@ -0,0 +1,13 @@ +
+
+
+ <%= page_info["check_answer_label"].to_s %> +
+
+ <%= case_log[page] %> +
+
+ <%= create_update_answer_link(case_log[page], case_log["id"], page)%> +
+
+
diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 8df10dcc8..872f7ae92 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -1,24 +1,11 @@ <%= turbo_frame_tag "case_log_form", target: "_top" do %>

Check the answers you gave for household characteristics

You answered <%= get_answered_questions_total(subsection_pages, case_log) %> of <%= subsection_pages.count %> questions

- <%= create_next_missing_question_link(case_log_id, subsection_pages, case_log) %> + <%= create_next_missing_question_link(case_log["id"], subsection_pages, case_log) %> <% subsection_pages.each do |page, page_info| %> -
-
-
- <%= page_info["check_answer_label"].to_s %> -
-
- <%= case_log[page] %> -
-
- <%= create_update_answer_link(case_log[page], case_log_id, page)%> -
-
-
+ <%= render partial: 'form/check_answers_table', locals: { page: page, page_info: page_info, case_log: case_log } %> <% end %> <%= form_with action: '/case_logs', method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> <%= f.govuk_submit "Save and continue" %> <% end %> <% end %> - diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 9f0e5e643..2af4c5433 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -59,7 +59,7 @@ RSpec.describe "Test Features" do when "text" fill_in(question.to_s, with: answer) when "radio" - choose("#{question.to_s.tr('_', '-')}-#{answer.parameterize}-field") + choose("#{question.to_s.dasherize}-#{answer.parameterize}-field") else fill_in(question.to_s, with: answer) end From 62a8b12d33081bd08181e779250e1c7b4b48c6d4 Mon Sep 17 00:00:00 2001 From: Kat Date: Thu, 30 Sep 2021 16:15:04 +0100 Subject: [PATCH 12/26] Update the ADR --- doc/adr/adr-005.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/adr/adr-005.md b/doc/adr/adr-005.md index 7e5b370b8..0e13194e1 100644 --- a/doc/adr/adr-005.md +++ b/doc/adr/adr-005.md @@ -6,18 +6,18 @@ We have opted to save values to the database directly instead of saving keys/num There are a few reasons we have opted to save the values directly, they are as follows -- The data will be easier to consume and analyse for anyone associated with the project who needs to do so but does not necessarily have the technical skills to access it through Rails i.e. A person could get the data directly from the database and it would require no additional work to be usable for reporting purposes. +- The data will be easier to consume and analyse for anyone associated with the project who needs to do so but does not necessarily have the technical skills to access it through Rails i.e. A person could get the data directly from the database and it would require no additional work to be usable for reporting purposes -- Currently there is no need to abstract the data as the data should be safe from being accessed by anyone external to the project. +- Currently there is no need to abstract the data as the data should be safe from being accessed by anyone external to the project -- It doesn't require additional dev work to map keys/numbers to values, we can just pull the values out directly and use them in the code, for example on the check answers page. +- It doesn't require additional dev work to map keys/numbers to values, we can just pull the values out directly and use them in the code, for example on the check answers page ### Drawbacks -- changing the wording/casing of the answers could result in discrepancies in the database +- Changing the wording/casing of the answers could result in discrepancies in the database -- There is a small risk that if the database is accessed by someone unauthorised they would have access to personally identifiable information. +- There is a small risk that if the database is accessed by someone unauthorised they would have access to personally identifiable information if we were to collect Any. We will be mitigating this risk by encrypting the production database -This decision is not too difficult to change and can be revisited in the future if there is sufficient reason to switch to storing keys/numbers and using enums and active record to convert those to the appropriate values. \ No newline at end of file +This decision is not too difficult to change and can be revisited in the future if there is sufficient reason to switch to storing keys/numbers and using enums and active record to convert those to the appropriate values. From 336aaea1e01821b6243434eddb8edd5c9a76dbf9 Mon Sep 17 00:00:00 2001 From: Kat Date: Fri, 1 Oct 2021 09:39:43 +0100 Subject: [PATCH 13/26] check questions instead of page names for get answered questions total --- app/helpers/check_answers_helper.rb | 7 +++---- spec/helpers/check_answers_helper_spec.rb | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 spec/helpers/check_answers_helper_spec.rb diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 464f42fab..605d6c9e7 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -1,10 +1,9 @@ module CheckAnswersHelper def get_answered_questions_total(subsection_pages, case_log) - questions = [] - subsection_pages.keys.each do |page| - questions << page + questions = subsection_pages.values.map do |page| + page["questions"].keys end - return questions.count {|question| !(case_log[question].blank?)} + return questions.count {|questions_for_page| questions_for_page.none? { |question| case_log[question].blank?}} end diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb new file mode 100644 index 000000000..78cc23b25 --- /dev/null +++ b/spec/helpers/check_answers_helper_spec.rb @@ -0,0 +1,18 @@ +require "rails_helper" + +RSpec.describe CheckAnswersHelper do + describe "Get answered questions total" do + let!(:case_log) { FactoryBot.create(:case_log) } + @form = Form.new(2021, 2022) + subsection_pages = @form.pages_for_subsection("income_and_benefits") + + it "returns 0 if no questions are answered" do + expect(get_answered_questions_total(subsection_pages, case_log)).to equal(0) + end + + it "returns 0 if only 1 question on a page gets answered" do + case_log["net_income"] = "123" + expect(get_answered_questions_total(subsection_pages, case_log)).to equal(0) + end + end +end From 26cca734fac70cc5d532156589113b38a76226d2 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 12:07:31 +0100 Subject: [PATCH 14/26] Use questions instead of pages in check answers --- app/helpers/check_answers_helper.rb | 6 +++--- app/views/form/_check_answers_table.html.erb | 6 +++--- app/views/form/check_answers.html.erb | 4 +++- config/forms/2021_2022.json | 14 +++++++------- spec/helpers/check_answers_helper_spec.rb | 4 ++-- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 605d6c9e7..5941800aa 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -1,9 +1,9 @@ module CheckAnswersHelper def get_answered_questions_total(subsection_pages, case_log) - questions = subsection_pages.values.map do |page| - page["questions"].keys + questions = subsection_pages.values.flat_map do |page| + page["questions"].keys end - return questions.count {|questions_for_page| questions_for_page.none? { |question| case_log[question].blank?}} + questions.count { |question| case_log[question].present? } end diff --git a/app/views/form/_check_answers_table.html.erb b/app/views/form/_check_answers_table.html.erb index b4551f622..fb3d47d0f 100644 --- a/app/views/form/_check_answers_table.html.erb +++ b/app/views/form/_check_answers_table.html.erb @@ -1,13 +1,13 @@
- <%= page_info["check_answer_label"].to_s %> + <%= question_info["check_answer_label"].to_s %>
- <%= case_log[page] %> + <%= case_log[question_title] %>
- <%= create_update_answer_link(case_log[page], case_log["id"], page)%> + <%= create_update_answer_link(case_log[question_title], case_log["id"], question_title)%>
diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 872f7ae92..a4404472f 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -3,7 +3,9 @@

You answered <%= get_answered_questions_total(subsection_pages, case_log) %> of <%= subsection_pages.count %> questions

<%= create_next_missing_question_link(case_log["id"], subsection_pages, case_log) %> <% subsection_pages.each do |page, page_info| %> - <%= render partial: 'form/check_answers_table', locals: { page: page, page_info: page_info, case_log: case_log } %> + <% page_info["questions"].each do |question_title, question_info| %> + <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: case_log } %> + <%end %> <% end %> <%= form_with action: '/case_logs', method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> <%= f.govuk_submit "Save and continue" %> diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 0966a9bdc..51da24d30 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -10,11 +10,11 @@ "label": "Household characteristics", "pages": { "tenant_code":{ - "check_answer_label": "Tenant code", "header": "", "description": "", "questions":{ "tenant_code": { + "check_answer_label": "Tenant code", "header": "What is the tenant code?", "hint_text": "", "type": "text" @@ -22,11 +22,11 @@ } }, "tenant_age":{ - "check_answer_label": "Tenant's age", "header": "", "description": "", "questions":{ "tenant_age": { + "check_answer_label": "Tenant's age", "header": "What is the tenant's age?", "hint_text": "", "type": "numeric", @@ -37,11 +37,11 @@ } }, "tenant_gender":{ - "check_answer_label": "Tenant's gender", "header": "", "description": "", "questions":{ "tenant_gender": { + "check_answer_label": "Tenant's gender", "header": "Which of these best describes the tenant's gender identity?", "hint_text": "", "type": "radio", @@ -55,11 +55,11 @@ } }, "tenant_ethnic_group":{ - "check_answer_label": "Ethnicity", "header": "", "description": "", "questions":{ "tenant_ethnic_group": { + "check_answer_label": "Ethnicity", "header": "What is the tenant's ethnic group?", "hint_text": "", "type": "radio", @@ -88,11 +88,11 @@ } }, "tenant_nationality":{ - "check_answer_label": "Nationality", "header": "", "description": "", "questions":{ "tenant_nationality": { + "check_answer_label": "Nationality", "header": "What is the tenant's nationality?", "hint_text": "", "type": "radio", @@ -118,11 +118,11 @@ } }, "tenant_economic_status":{ - "check_answer_label": "Work", "header": "", "description": "", "questions":{ "tenant_economic_status": { + "check_answer_label": "Work", "header": "Which of these best describes the tenant's working situation?", "hint_text": "", "type": "radio", @@ -143,11 +143,11 @@ } }, "household_number_of_other_members":{ - "check_answer_label": "Number of Other Household Members", "header": "", "description": "", "questions":{ "household_number_of_other_members": { + "check_answer_label": "Number of Other Household Members", "header": "How many other people are there in the household?", "hint_text": "The maximum number of others is 7", "type": "numeric", diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb index 78cc23b25..598fc9564 100644 --- a/spec/helpers/check_answers_helper_spec.rb +++ b/spec/helpers/check_answers_helper_spec.rb @@ -10,9 +10,9 @@ RSpec.describe CheckAnswersHelper do expect(get_answered_questions_total(subsection_pages, case_log)).to equal(0) end - it "returns 0 if only 1 question on a page gets answered" do + it "returns 1 if 1 question gets answered" do case_log["net_income"] = "123" - expect(get_answered_questions_total(subsection_pages, case_log)).to equal(0) + expect(get_answered_questions_total(subsection_pages, case_log)).to equal(1) end end end From 9ca0063ac0a1974cee1425900a831b1817ac5206 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 12:39:43 +0100 Subject: [PATCH 15/26] spec uses subsection with 2 questions on page --- app/helpers/check_answers_helper.rb | 17 +++++++++++++---- app/views/form/check_answers.html.erb | 2 +- spec/features/case_log_spec.rb | 8 ++++---- spec/helpers/check_answers_helper_spec.rb | 10 ++++++++++ 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 5941800aa..ec123db72 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -13,10 +13,19 @@ module CheckAnswersHelper end def create_next_missing_question_link(case_log_id, subsection_pages, case_log) - empty_question = subsection_pages.keys.find{|x| case_log[x].blank? } - - url = "/case_logs/#{case_log_id}/#{empty_question}" - link_to('Answer the missing questions', url, class: "govuk-link").html_safe + pages_to_fill_in = [] + subsection_pages.each do |page_title, page_info| + page_info["questions"].any? { |q| case_log["q"].blank?} + pages_to_fill_in << page_title + end + url = "/case_logs/#{case_log_id}/#{pages_to_fill_in.first}" + link_to('Answer the missing questions', url, class: "govuk-link").html_safe end + def get_total_number_of_questions(subsection_pages) + questions = subsection_pages.values.flat_map do |page| + page["questions"].keys + end + questions.count + end end diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index a4404472f..503a6c700 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -1,6 +1,6 @@ <%= turbo_frame_tag "case_log_form", target: "_top" do %>

Check the answers you gave for household characteristics

-

You answered <%= get_answered_questions_total(subsection_pages, case_log) %> of <%= subsection_pages.count %> questions

+

You answered <%= get_answered_questions_total(subsection_pages, case_log) %> of <%= get_total_number_of_questions(subsection_pages) %> questions

<%= create_next_missing_question_link(case_log["id"], subsection_pages, case_log) %> <% subsection_pages.each do |page, page_info| %> <% page_info["questions"].each do |question_title, question_info| %> diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 2af4c5433..ca520ce65 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -158,11 +158,11 @@ RSpec.describe "Test Features" do end it "should have a link pointing to the next empty question if some questions are answered" do - fill_in_number_question(empty_case_log.id, "tenant_code", 0) + fill_in_number_question(empty_case_log.id, "net_income", 18000) - visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") - expect(page).to have_content('You answered 1 of 7 questions') - expect(page).to have_link('Answer the missing questions', href: "/case_logs/#{empty_case_log.id}/tenant_age") + visit("/case_logs/#{empty_case_log.id}/income_and_benefits/check_answers") + expect(page).to have_content('You answered 1 of 4 questions') + expect(page).to have_link('Answer the missing questions', href: "/case_logs/#{empty_case_log.id}/net_income") end end end diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb index 598fc9564..b85640bad 100644 --- a/spec/helpers/check_answers_helper_spec.rb +++ b/spec/helpers/check_answers_helper_spec.rb @@ -15,4 +15,14 @@ RSpec.describe CheckAnswersHelper do expect(get_answered_questions_total(subsection_pages, case_log)).to equal(1) end end + + describe "Get total number of questions" do + let!(:case_log) { FactoryBot.create(:case_log) } + @form = Form.new(2021, 2022) + subsection_pages = @form.pages_for_subsection("income_and_benefits") + + it "returns the total number of questions for a subsection" do + expect(get_total_number_of_questions(subsection_pages)).to eq(4) + end + end end From 0333a1c1caf106a571ac9883036602f12f8aa91a Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 14:20:23 +0100 Subject: [PATCH 16/26] add spec for all questions answered text --- app/helpers/check_answers_helper.rb | 19 ++++++++++++++----- app/views/form/check_answers.html.erb | 3 +-- spec/features/case_log_spec.rb | 17 +++++++++++++++++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index ec123db72..7bc4cca79 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -6,6 +6,12 @@ module CheckAnswersHelper questions.count { |question| case_log[question].present? } end + def get_total_number_of_questions(subsection_pages) + questions = subsection_pages.values.flat_map do |page| + page["questions"].keys + end + questions.count + end def create_update_answer_link(case_log_answer, case_log_id, page) link_name = case_log_answer.blank? ? "Answer" : "Change" @@ -22,10 +28,13 @@ module CheckAnswersHelper link_to('Answer the missing questions', url, class: "govuk-link").html_safe end - def get_total_number_of_questions(subsection_pages) - questions = subsection_pages.values.flat_map do |page| - page["questions"].keys + def display_answered_questions_summary(subsection_pages, case_log) + if get_answered_questions_total(subsection_pages, case_log) == get_total_number_of_questions(subsection_pages) + "

You answered all the questions

".html_safe + else + "

You answered #{get_answered_questions_total(subsection_pages, case_log)} of #{get_total_number_of_questions(subsection_pages)} questions

+ #{create_next_missing_question_link(case_log["id"], subsection_pages, case_log)}".html_safe + end - questions.count - end + end end diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 503a6c700..4ef25a0bf 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -1,7 +1,6 @@ <%= turbo_frame_tag "case_log_form", target: "_top" do %>

Check the answers you gave for household characteristics

-

You answered <%= get_answered_questions_total(subsection_pages, case_log) %> of <%= get_total_number_of_questions(subsection_pages) %> questions

- <%= create_next_missing_question_link(case_log["id"], subsection_pages, case_log) %> + <%= display_answered_questions_summary(subsection_pages, case_log) %> <% subsection_pages.each do |page, page_info| %> <% page_info["questions"].each do |question_title, question_info| %> <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: case_log } %> diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index ca520ce65..82d161ee8 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -108,6 +108,17 @@ RSpec.describe "Test Features" do click_button("Save and continue") end + def answer_all_questions_in_income_subsection + visit("/case_logs/#{empty_case_log.id}/net_income") + fill_in("net_income", with: 18000) + choose("net-income-frequency-yearly-field") + click_button("Save and continue") + choose("net-income-uc-proportion-all-field") + click_button("Save and continue") + choose("housing-benefit-housing-benefit-but-not-universal-credit-field") + click_button("Save and continue") + end + it "can be visited by URL" do visit("case_logs/#{id}/#{subsection}/check_answers") expect(page).to have_content("Check the answers you gave for #{subsection.tr('_', ' ')}") @@ -164,6 +175,12 @@ RSpec.describe "Test Features" do expect(page).to have_content('You answered 1 of 4 questions') expect(page).to have_link('Answer the missing questions', href: "/case_logs/#{empty_case_log.id}/net_income") end + + it "should not display the missing answer questions link if all questions are answered" do + answer_all_questions_in_income_subsection + expect(page).to have_content('You answered all the questions') + assert_selector "a", text: "Answer the missing questions", count: 0 + end end end end From 8f2d9ddf816f6d4fd3f2760e3bc7824425ccd146 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 14:22:06 +0100 Subject: [PATCH 17/26] linting fixes --- app/controllers/case_logs_controller.rb | 14 ++++++------ app/helpers/check_answers_helper.rb | 26 +++++++++++------------ spec/features/case_log_spec.rb | 22 +++++++++---------- spec/helpers/check_answers_helper_spec.rb | 2 +- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index f162f1f64..39a0dac55 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -29,11 +29,11 @@ class CaseLogsController < ApplicationController @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 + subsection = form.subsection_for_page(previous_page) + "case_log_#{subsection}_check_answers_path" + else + "case_log_#{next_page}_path" + end redirect_to(send(redirect_path, @case_log)) end @@ -41,8 +41,8 @@ class CaseLogsController < ApplicationController def check_answers @case_log = CaseLog.find(params[:case_log_id]) form = Form.new(2021, 2022) - current_url = request.env['PATH_INFO'] - subsection = current_url.split('/')[-2] + current_url = request.env["PATH_INFO"] + subsection = current_url.split("/")[-2] subsection_pages = form.pages_for_subsection(subsection) render "form/check_answers", locals: { case_log: @case_log, subsection_pages: subsection_pages } end diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 7bc4cca79..6aceadca2 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -1,14 +1,14 @@ module CheckAnswersHelper def get_answered_questions_total(subsection_pages, case_log) - questions = subsection_pages.values.flat_map do |page| - page["questions"].keys + questions = subsection_pages.values.flat_map do |page| + page["questions"].keys end questions.count { |question| case_log[question].present? } end def get_total_number_of_questions(subsection_pages) - questions = subsection_pages.values.flat_map do |page| - page["questions"].keys + questions = subsection_pages.values.flat_map do |page| + page["questions"].keys end questions.count end @@ -19,13 +19,13 @@ module CheckAnswersHelper end def create_next_missing_question_link(case_log_id, subsection_pages, case_log) - pages_to_fill_in = [] - subsection_pages.each do |page_title, page_info| - page_info["questions"].any? { |q| case_log["q"].blank?} - pages_to_fill_in << page_title - end - url = "/case_logs/#{case_log_id}/#{pages_to_fill_in.first}" - link_to('Answer the missing questions', url, class: "govuk-link").html_safe + pages_to_fill_in = [] + subsection_pages.each do |page_title, page_info| + page_info["questions"].any? { |_q| case_log["q"].blank? } + pages_to_fill_in << page_title + end + url = "/case_logs/#{case_log_id}/#{pages_to_fill_in.first}" + link_to("Answer the missing questions", url, class: "govuk-link").html_safe end def display_answered_questions_summary(subsection_pages, case_log) @@ -33,8 +33,8 @@ module CheckAnswersHelper "

You answered all the questions

".html_safe else "

You answered #{get_answered_questions_total(subsection_pages, case_log)} of #{get_total_number_of_questions(subsection_pages)} questions

- #{create_next_missing_question_link(case_log["id"], subsection_pages, case_log)}".html_safe + #{create_next_missing_question_link(case_log['id'], subsection_pages, case_log)}".html_safe end - end + end end diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 82d161ee8..fa3cfbbf1 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -104,13 +104,13 @@ 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}", with: value) + fill_in(question.to_s, with: value) click_button("Save and continue") end def answer_all_questions_in_income_subsection visit("/case_logs/#{empty_case_log.id}/net_income") - fill_in("net_income", with: 18000) + fill_in("net_income", with: 18_000) choose("net-income-frequency-yearly-field") click_button("Save and continue") choose("net-income-uc-proportion-all-field") @@ -151,7 +151,7 @@ RSpec.describe "Test Features" do visit("case_logs/#{empty_case_log.id}/#{subsection}/check_answers") assert_selector "a", text: /Answer\z/, count: 7 assert_selector "a", text: "Change", count: 0 - expect(page).to have_link('Answer', href: "/case_logs/#{empty_case_log.id}/tenant_age") + expect(page).to have_link("Answer", href: "/case_logs/#{empty_case_log.id}/tenant_age") end it "should have a change link for answered questions" do @@ -159,28 +159,28 @@ RSpec.describe "Test Features" do visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") assert_selector "a", text: /Answer\z/, count: 6 assert_selector "a", text: "Change", count: 1 - expect(page).to have_link('Change', href: "/case_logs/#{empty_case_log.id}/tenant_age") + expect(page).to have_link("Change", href: "/case_logs/#{empty_case_log.id}/tenant_age") end it "should have a link pointing to the first question if no questions are answered" do visit("/case_logs/#{empty_case_log.id}/#{subsection}/check_answers") - expect(page).to have_content('You answered 0 of 7 questions') - expect(page).to have_link('Answer the missing questions', href: "/case_logs/#{empty_case_log.id}/tenant_code") + expect(page).to have_content("You answered 0 of 7 questions") + expect(page).to have_link("Answer the missing questions", href: "/case_logs/#{empty_case_log.id}/tenant_code") end it "should have a link pointing to the next empty question if some questions are answered" do - fill_in_number_question(empty_case_log.id, "net_income", 18000) + fill_in_number_question(empty_case_log.id, "net_income", 18_000) visit("/case_logs/#{empty_case_log.id}/income_and_benefits/check_answers") - expect(page).to have_content('You answered 1 of 4 questions') - expect(page).to have_link('Answer the missing questions', href: "/case_logs/#{empty_case_log.id}/net_income") + expect(page).to have_content("You answered 1 of 4 questions") + expect(page).to have_link("Answer the missing questions", href: "/case_logs/#{empty_case_log.id}/net_income") end it "should not display the missing answer questions link if all questions are answered" do answer_all_questions_in_income_subsection - expect(page).to have_content('You answered all the questions') + expect(page).to have_content("You answered all the questions") assert_selector "a", text: "Answer the missing questions", count: 0 - end + end end end end diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb index b85640bad..348159566 100644 --- a/spec/helpers/check_answers_helper_spec.rb +++ b/spec/helpers/check_answers_helper_spec.rb @@ -22,7 +22,7 @@ RSpec.describe CheckAnswersHelper do subsection_pages = @form.pages_for_subsection("income_and_benefits") it "returns the total number of questions for a subsection" do - expect(get_total_number_of_questions(subsection_pages)).to eq(4) + expect(get_total_number_of_questions(subsection_pages)).to eq(4) end end end From b05c66cff256375f8dafb70995c420b5d8a5a569 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 14:22:42 +0100 Subject: [PATCH 18/26] delete empty line --- app/helpers/check_answers_helper.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 6aceadca2..f51dc638a 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -34,7 +34,6 @@ module CheckAnswersHelper else "

You answered #{get_answered_questions_total(subsection_pages, case_log)} of #{get_total_number_of_questions(subsection_pages)} questions

#{create_next_missing_question_link(case_log['id'], subsection_pages, case_log)}".html_safe - end end end From 199d4b9540f5ae250524b2d059da3e07b657023f Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 14:25:59 +0100 Subject: [PATCH 19/26] styling changes --- app/helpers/check_answers_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index f51dc638a..a61dd31c2 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -30,9 +30,9 @@ module CheckAnswersHelper def display_answered_questions_summary(subsection_pages, case_log) if get_answered_questions_total(subsection_pages, case_log) == get_total_number_of_questions(subsection_pages) - "

You answered all the questions

".html_safe + '

You answered all the questions

'.html_safe else - "

You answered #{get_answered_questions_total(subsection_pages, case_log)} of #{get_total_number_of_questions(subsection_pages)} questions

+ "

You answered #{get_answered_questions_total(subsection_pages, case_log)} of #{get_total_number_of_questions(subsection_pages)} questions

#{create_next_missing_question_link(case_log['id'], subsection_pages, case_log)}".html_safe end end From c02249ae6a62a93349409ff0b177db024e8ba0db Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 14:30:46 +0100 Subject: [PATCH 20/26] layout changes --- app/controllers/case_logs_controller.rb | 2 +- app/views/form/check_answers.html.erb | 24 ++++++++++++++---------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 39a0dac55..1a7282bb1 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -44,7 +44,7 @@ class CaseLogsController < ApplicationController current_url = request.env["PATH_INFO"] subsection = current_url.split("/")[-2] subsection_pages = form.pages_for_subsection(subsection) - render "form/check_answers", locals: { case_log: @case_log, subsection_pages: subsection_pages } + render "form/check_answers", locals: { case_log: @case_log, subsection_pages: subsection_pages, subsection: subsection } end form = Form.new(2021, 2022) diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 4ef25a0bf..69b39e33f 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -1,12 +1,16 @@ <%= turbo_frame_tag "case_log_form", target: "_top" do %> -

Check the answers you gave for household characteristics

- <%= display_answered_questions_summary(subsection_pages, case_log) %> - <% subsection_pages.each do |page, page_info| %> - <% page_info["questions"].each do |question_title, question_info| %> - <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: case_log } %> - <%end %> - <% end %> - <%= form_with action: '/case_logs', method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> - <%= f.govuk_submit "Save and continue" %> - <% end %> +
+
+

Check the answers you gave for <%= subsection %>

+ <%= display_answered_questions_summary(subsection_pages, case_log) %> + <% subsection_pages.each do |page, page_info| %> + <% page_info["questions"].each do |question_title, question_info| %> + <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: case_log } %> + <%end %> + <% end %> + <%= form_with action: '/case_logs', method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> + <%= f.govuk_submit "Save and continue" %> + <% end %> +
+
<% end %> From 023f1f66e341310a84ff2bbca6f30237d38f0afd Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 14:34:46 +0100 Subject: [PATCH 21/26] make check answer header dynamic --- app/controllers/case_logs_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 1a7282bb1..d84e6187e 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -44,7 +44,7 @@ class CaseLogsController < ApplicationController current_url = request.env["PATH_INFO"] subsection = current_url.split("/")[-2] subsection_pages = form.pages_for_subsection(subsection) - render "form/check_answers", locals: { case_log: @case_log, subsection_pages: subsection_pages, subsection: subsection } + render "form/check_answers", locals: { case_log: @case_log, subsection_pages: subsection_pages, subsection: subsection.humanize(capitalize: false) } end form = Form.new(2021, 2022) From f38c65ea487fb09a2da3958bf73db6b52682ab65 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 14:37:47 +0100 Subject: [PATCH 22/26] fix update answer links --- app/views/form/_check_answers_table.html.erb | 2 +- app/views/form/check_answers.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/form/_check_answers_table.html.erb b/app/views/form/_check_answers_table.html.erb index fb3d47d0f..88567bc9c 100644 --- a/app/views/form/_check_answers_table.html.erb +++ b/app/views/form/_check_answers_table.html.erb @@ -7,7 +7,7 @@ <%= case_log[question_title] %>
- <%= create_update_answer_link(case_log[question_title], case_log["id"], question_title)%> + <%= create_update_answer_link(case_log[question_title], case_log["id"], page)%>
diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 69b39e33f..a2102146d 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -5,7 +5,7 @@ <%= display_answered_questions_summary(subsection_pages, case_log) %> <% subsection_pages.each do |page, page_info| %> <% page_info["questions"].each do |question_title, question_info| %> - <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: case_log } %> + <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: case_log, page: page } %> <%end %> <% end %> <%= form_with action: '/case_logs', method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> From 0db55a78c7c0c319a1e9ae56185c848939557d87 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 15:02:15 +0100 Subject: [PATCH 23/26] add check answer labels to json --- config/forms/2021_2022.json | 54 ++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 51da24d30..28133d58d 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -421,9 +421,9 @@ "tenancy_code":{ "header": "", "description": "", - "check_answer_label": "What is the tenancy code?", "questions":{ "tenancy_code": { + "check_answer_label": "What is the tenancy code?", "header": "What is the tenancy code?", "hint_text": "", "type": "text" @@ -433,9 +433,9 @@ "tenancy_start_date": { "header": "", "description": "", - "check_answer_label": "When is the tenancy start date?", "questions": { "tenancy_start_date": { + "check_answer_label": "When is the tenancy start date?", "header": "What is the tenancy start date?", "hint_text": "For example, 27 3 2007", "type": "date" @@ -445,9 +445,9 @@ "starter_tenancy": { "header": "", "description": "", - "check_answer_label": "Is this a starter or introductory tenancy?", "questions": { "starter_tenancy": { + "check_answer_label": "Is this a starter or introductory tenancy?", "header": "Is this a starter tenancy?", "hint_text": "", "type": "radio", @@ -461,9 +461,9 @@ "fixed_term_tenancy":{ "header": "", "description": "", - "check_answer_label": "If the main tenancy is a fixed term tenancy, please provide the length of the fixed term (to the nearest year) excluding any starter/introductory period", "questions":{ "fixed_term_tenancy": { + "check_answer_label": "If the main tenancy is a fixed term tenancy, please provide the length of the fixed term (to the nearest year) excluding any starter/introductory period", "header": "If fixed-term, what is the length of the fixed-term tenancy after any starter period?", "hint_text": "To the nearest year", "type": "numeric", @@ -476,9 +476,9 @@ "tenancy_type": { "header": "", "description": "", - "check_answer_label": "Type of main tenancy (after any starter/introductory period)", "questions": { "tenancy_type": { + "check_answer_label": "Type of main tenancy (after any starter/introductory period)", "header": "What is the type of tenancy after the starter period has ended?", "hint_text": "", "type": "radio", @@ -496,9 +496,9 @@ "letting_type": { "header": "", "description": "", - "check_answer_label": "Type of letting", "questions": { "letting_type": { + "check_answer_label": "Type of letting", "header": "Which type of letting is this?", "hint_text": "", "type": "radio", @@ -516,9 +516,9 @@ "letting_provider": { "header": "", "description": "", - "check_answer_label": "Provider", "questions": { "letting_provider": { + "check_answer_label": "Provider", "header": "Who is the letting provider?", "hint_text": "", "type": "radio", @@ -537,9 +537,9 @@ "property_location":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "property_location": { + "check_answer_label": "Property Location", "header": "Property location", "hint_text": "", "type": "radio", @@ -866,6 +866,7 @@ "description": "", "questions":{ "property_postcode": { + "check_answer_label": "What was the previous postcode?", "header": "What is the property's postcode?", "hint_text": "", "type": "text" @@ -877,6 +878,7 @@ "description": "", "questions": { "property_relet": { + "check_answer_label": "Which type was the property most recently let as?", "header": "Is this property a relet?", "hint_text": "", "type": "radio", @@ -892,6 +894,7 @@ "description": "", "questions": { "property_vacancy_reason": { + "check_answer_label": "What is the reason for the property vacancy?", "header": "What is the reason for the property vacancy?", "hint_text": "", "type": "radio", @@ -919,6 +922,7 @@ "description": "", "questions":{ "property_reference": { + "check_answer_label": "What’s the property reference?", "header": "What's the property reference?", "hint_text": "", "type": "text" @@ -930,6 +934,7 @@ "description": "", "questions": { "property_unit_type": { + "check_answer_label": "Which type of unit is the property?", "header": "Which type of unit is the property?", "hint_text": "", "type": "radio", @@ -951,6 +956,7 @@ "description": "", "questions":{ "property_number_of_bedrooms": { + "check_answer_label": "How many bedrooms are there in the property?", "header": "How many bedrooms are there in the property?", "hint_text": "If shared accommodation, enter number of bedrooms occupied by this household; a bed-sit has 1 bedroom", "type": "numeric", @@ -965,6 +971,7 @@ "description": "", "questions": { "property_major_repairs": { + "check_answer_label": "Were major repairs carried out during the void period?", "header": "Were any major repairs completed during the void period?", "hint_text": "", "type": "radio", @@ -980,6 +987,7 @@ "description": "", "questions": { "property_major_repairs": { + "check_answer_label": "What was the major repairs completion date?", "header": "What was the major repairs completion date?", "hint_text": "For example, 27 3 2007", "type": "date" @@ -991,6 +999,7 @@ "description": "", "questions":{ "property_number_of_times_relet": { + "check_answer_label": "How many times has this unit been previously offered since becoming available for relet since the last tenancy ended or as a first let?", "header": "How many times has this unit been previously offered since becoming available for relet since the last tenancy ended or as a first let? ", "hint_text": "For an Affordable Rent or Intermediate Rent Letting, only include number of offers as that type. For a property let at the first attempt enter '0' ", "type": "numeric", @@ -1005,6 +1014,7 @@ "description": "", "questions": { "property_wheelchair_accessible": { + "check_answer_label": "Is property built or adapted to wheelchair user standards?", "header": "Is property built or adapted to wheelchair user standards?", "hint_text": "", "type": "radio", @@ -1028,9 +1038,9 @@ "net_income":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "net_income": { + "check_answer_label": "", "header": "What is the tenant’s /and partner’s combined income after tax?", "hint_text": "", "type": "numeric", @@ -1052,9 +1062,9 @@ "net_income_uc_proportion":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "net_income_uc_proportion": { + "check_answer_label": "", "header": "How much of the tenant’s income is from Universal Credit, state pensions or benefits?", "hint_text": "", "type": "radio", @@ -1070,9 +1080,9 @@ "housing_benefit":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "housing_benefit": { + "check_answer_label": "", "header": "Is the tenant likely to be in receipt of any of these housing-related benefits?", "hint_text": "", "type": "radio", @@ -1096,9 +1106,9 @@ "rent":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "rent_frequency": { + "check_answer_label": "Rent Period", "header": "Which period are rent and other charges due?", "hint_text": "", "type": "radio", @@ -1116,6 +1126,7 @@ } }, "basic_rent": { + "check_answer_label": "Basic Rent", "header": "What is the basic rent?", "hint_text": "Eligible for housing benefit or Universal Credit", "type": "numeric", @@ -1123,6 +1134,7 @@ "step": 1 }, "service_charge": { + "check_answer_label": "Service Charge", "header": "What is the service charge?", "hint_text": "Eligible for housing benefit or Universal Credit", "type": "numeric", @@ -1130,6 +1142,7 @@ "step": 1 }, "personal_service_charge": { + "check_answer_label": "Personal Service Charge", "header": "What is the personal service charge?", "hint_text": "Not eligible for housing benefit or Universal Credit. For example, hot water excluding water rates.", "type": "numeric", @@ -1137,6 +1150,7 @@ "step": 1 }, "support_charge": { + "check_answer_label": "Support Charge", "header": "What is the support charge?", "hint_text": "This is to fund housing-related support services included in the tenancy agreement", "type": "numeric", @@ -1144,6 +1158,7 @@ "step": 1 }, "total_charge": { + "check_answer_label": "Total Charge", "header": "Total charge?", "hint_text": "This is the total of rent and all charges", "type": "numeric", @@ -1151,6 +1166,7 @@ "step": 1 }, "outstanding_amount": { + "check_answer_label": "After housing benefit and/or housing element of UC payment is received, will there be an outstanding amount for basic rent and/or benefit eligible charges?", "header": "After housing benefit and/or housing element of UC payment is received, will there be an outstanding amount for basic rent and/or benefit eligible charges?", "hint_text": "", "type": "radio", @@ -1174,9 +1190,9 @@ "time_lived_in_la":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "time_lived_in_la": { + "check_answer_label": "", "header": "How long has the household continuously lived in the local authority area where the new letting is located?", "hint_text": "", "type": "radio", @@ -1196,9 +1212,9 @@ "time_on_la_waiting_list":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "time_on_la_waiting_list": { + "check_answer_label": "", "header": "How long has the household been on the local authority waiting list where the new letting is located?", "hint_text": "", "type": "radio", @@ -1218,9 +1234,9 @@ "previous_la":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "previous_la": { + "check_answer_label": "", "header": "Which local authority area did the household live in immediately before this letting?", "hint_text": "Includes temporary accommodation", "type": "radio", @@ -1545,8 +1561,8 @@ "previous_postcode": { "header": "", "description": "", - "check_answer_label": "", "questions": { + "check_answer_label": "", "previous_postcode": { "header": "Postcode for the previous accommodation", "hint_text": "If the household has moved from settled accommodation immediately prior to being re-housed", @@ -1557,9 +1573,9 @@ "reasonable_preference": { "header": "", "description": "", - "check_answer_label": "", "questions": { "reasonable_preference": { + "check_answer_label": "", "header": "Was the household given reasonable preference by the local authority?", "hint_text": "", "type": "radio", @@ -1586,9 +1602,9 @@ "lettings_policy": { "header": "", "description": "", - "check_answer_label": "", "questions": { "cbl_letting": { + "check_answer_label": "", "header": "Was the letting made under choice-based lettings (CBL)? ", "hint_text": "", "type": "radio", @@ -1630,9 +1646,9 @@ "declaration":{ "header": "", "description": "", - "check_answer_label": "", "questions":{ "declaration": { + "check_answer_label": "", "header": "What is the tenant code?", "hint_text": "", "type": "text" From acb5c8f4f87865435920f5275db1230d3c4a19b5 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 15:16:55 +0100 Subject: [PATCH 24/26] improve clarity --- app/helpers/check_answers_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index a61dd31c2..4fd46ce9d 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -21,7 +21,7 @@ module CheckAnswersHelper def create_next_missing_question_link(case_log_id, subsection_pages, case_log) pages_to_fill_in = [] subsection_pages.each do |page_title, page_info| - page_info["questions"].any? { |_q| case_log["q"].blank? } + page_info["questions"].any? { |question| case_log[question].blank? } pages_to_fill_in << page_title end url = "/case_logs/#{case_log_id}/#{pages_to_fill_in.first}" From 667649834a41095cbc0847f087cfbbeeaba30ee1 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Fri, 1 Oct 2021 15:23:18 +0100 Subject: [PATCH 25/26] rename adr --- doc/adr/{adr-005.md => adr-006-saving-values.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename doc/adr/{adr-005.md => adr-006-saving-values.md} (97%) diff --git a/doc/adr/adr-005.md b/doc/adr/adr-006-saving-values.md similarity index 97% rename from doc/adr/adr-005.md rename to doc/adr/adr-006-saving-values.md index 0e13194e1..68b3e9622 100644 --- a/doc/adr/adr-005.md +++ b/doc/adr/adr-006-saving-values.md @@ -1,4 +1,4 @@ -### ADR - 005: Saving values to the database +### ADR - 006: Saving values to the database We have opted to save values to the database directly instead of saving keys/numbers that need to be converted with enums in models using active record. From 2f4a953937e92165ff41263e6cf362042e4c3d85 Mon Sep 17 00:00:00 2001 From: Kat Date: Fri, 1 Oct 2021 15:10:58 +0100 Subject: [PATCH 26/26] update answer label values --- config/forms/2021_2022.json | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 28133d58d..d0bf139f5 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -1040,7 +1040,7 @@ "description": "", "questions":{ "net_income": { - "check_answer_label": "", + "check_answer_label": "Income", "header": "What is the tenant’s /and partner’s combined income after tax?", "hint_text": "", "type": "numeric", @@ -1048,6 +1048,7 @@ "step": "1" }, "net_income_frequency": { + "check_answer_label": "Income Frequency", "header": "How often do they receive this income?", "hint_text": "", "type": "radio", @@ -1064,7 +1065,7 @@ "description": "", "questions":{ "net_income_uc_proportion": { - "check_answer_label": "", + "check_answer_label": "Benefits as a proportion of income", "header": "How much of the tenant’s income is from Universal Credit, state pensions or benefits?", "hint_text": "", "type": "radio", @@ -1082,7 +1083,7 @@ "description": "", "questions":{ "housing_benefit": { - "check_answer_label": "", + "check_answer_label": "Universal Credit & Housing Benefit\t", "header": "Is the tenant likely to be in receipt of any of these housing-related benefits?", "hint_text": "", "type": "radio", @@ -1192,7 +1193,7 @@ "description": "", "questions":{ "time_lived_in_la": { - "check_answer_label": "", + "check_answer_label": "How long has the household continuously lived in the local authority area where the new letting is located?", "header": "How long has the household continuously lived in the local authority area where the new letting is located?", "hint_text": "", "type": "radio", @@ -1214,7 +1215,7 @@ "description": "", "questions":{ "time_on_la_waiting_list": { - "check_answer_label": "", + "check_answer_label": "How long has the household been on the local authority waiting list where the new letting is located?", "header": "How long has the household been on the local authority waiting list where the new letting is located?", "hint_text": "", "type": "radio", @@ -1236,7 +1237,7 @@ "description": "", "questions":{ "previous_la": { - "check_answer_label": "", + "check_answer_label": "The LA in which household lived immediately before this letting\t", "header": "Which local authority area did the household live in immediately before this letting?", "hint_text": "Includes temporary accommodation", "type": "radio", @@ -1562,8 +1563,8 @@ "header": "", "description": "", "questions": { - "check_answer_label": "", "previous_postcode": { + "check_answer_label": "Postcode of previous accomodation if the household has moved from settled accommodation", "header": "Postcode for the previous accommodation", "hint_text": "If the household has moved from settled accommodation immediately prior to being re-housed", "type": "text" @@ -1575,7 +1576,7 @@ "description": "", "questions": { "reasonable_preference": { - "check_answer_label": "", + "check_answer_label": "Was the household given Reasonable Preference (i.e. priority) for housing by the Local Authority?", "header": "Was the household given reasonable preference by the local authority?", "hint_text": "", "type": "radio", @@ -1585,6 +1586,7 @@ } }, "reasonable_preference_reason": { + "check_answer_label": "Reason for reasonable preference", "header": "Why were they given reasonable preference?", "hint_text": "Select all that apply", "type": "checkbox", @@ -1604,7 +1606,7 @@ "description": "", "questions": { "cbl_letting": { - "check_answer_label": "", + "check_answer_label": "Choice-based letting?", "header": "Was the letting made under choice-based lettings (CBL)? ", "hint_text": "", "type": "radio", @@ -1614,6 +1616,7 @@ } }, "chr_letting": { + "check_answer_label": "Common housing register letting?", "header": "Was the letting made under common housing register (CHR)? ", "hint_text": "", "type": "radio", @@ -1623,6 +1626,7 @@ } }, "cap_letting": { + "check_answer_label": "Common allocation policy letting?", "header": "Was the letting made under common allocation policy (CAP)? ", "hint_text": "", "type": "radio",