From 92ffab0c3d45f13de8b12e2dfb3596cf633934d5 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 12 Nov 2021 16:39:29 +0000 Subject: [PATCH 01/16] rubocop --- Gemfile | 2 +- db/migrate/20211112105348_add_incref_field.rb | 1 - lib/tasks/form_definition.rake | 81 +++++++++---------- 3 files changed, 41 insertions(+), 43 deletions(-) diff --git a/Gemfile b/Gemfile index ed2af8973..599c5d369 100644 --- a/Gemfile +++ b/Gemfile @@ -29,7 +29,7 @@ gem "discard" gem "activeadmin" # Admin charts gem "chartkick" -#Json Schema +# Json Schema gem "json-schema" gem "uk_postcode" diff --git a/db/migrate/20211112105348_add_incref_field.rb b/db/migrate/20211112105348_add_incref_field.rb index 65df53aca..ece9a458b 100644 --- a/db/migrate/20211112105348_add_incref_field.rb +++ b/db/migrate/20211112105348_add_incref_field.rb @@ -5,4 +5,3 @@ class AddIncrefField < ActiveRecord::Migration[6.1] end end end - diff --git a/lib/tasks/form_definition.rake b/lib/tasks/form_definition.rake index 565728064..c2b3c6103 100644 --- a/lib/tasks/form_definition.rake +++ b/lib/tasks/form_definition.rake @@ -2,49 +2,48 @@ require "json" require "json-schema" def get_all_form_paths(directories) - form_paths = [] - directories.each do |directory| - Dir.glob("#{directory}/*.json").each do |form_path| - form_paths.push(form_path) - end + form_paths = [] + directories.each do |directory| + Dir.glob("#{directory}/*.json").each do |form_path| + form_paths.push(form_path) end - form_paths + end + form_paths end namespace :form_definition do - desc "Validate JSON against Generic Form Schema" - task :validate do - puts "#{Rails.root}" - path = "config/forms/schema/generic.json" - - file = File.open(path) - schema = JSON.parse(file.read) - metaschema = JSON::Validator.validator_for_name("draft4").metaschema - - puts path - - if JSON::Validator.validate(metaschema, schema) - puts "schema valid" - else - puts "schema not valid" - return - end - - directories = ["config/forms", "spec/fixtures/forms"] - # directories = ["config/forms"] - - get_all_form_paths(directories).each do |path| - puts path - file = File.open(path) - data = JSON.parse(file.read) - - puts JSON::Validator.fully_validate(schema, data, :strict => true) - - begin - JSON::Validator.validate!(schema, data) - rescue JSON::Schema::ValidationError => e - e.message - end - end + desc "Validate JSON against Generic Form Schema" + task validate: :environment do + puts Rails.root.to_s + path = "config/forms/schema/generic.json" + + file = File.open(path) + schema = JSON.parse(file.read) + metaschema = JSON::Validator.validator_for_name("draft4").metaschema + + puts path + + if JSON::Validator.validate(metaschema, schema) + puts "schema valid" + else + puts "schema not valid" + return end - end \ No newline at end of file + + directories = ["config/forms", "spec/fixtures/forms"] + + get_all_form_paths(directories).each do |path| + puts path + file = File.open(path) + data = JSON.parse(file.read) + + puts JSON::Validator.fully_validate(schema, data, strict: true) + + begin + JSON::Validator.validate!(schema, data) + rescue JSON::Schema::ValidationError => e + e.message + end + end + end +end From 0e88b5e7036696e9de6a59c03eba3706075f7c64 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 12 Nov 2021 17:37:03 +0000 Subject: [PATCH 02/16] Refactor --- README.md | 9 ++-- lib/tasks/form_definition.rake | 43 ++++++++----------- .../tasks/form_definition_validator_spec.rb | 32 ++++++++++++++ 3 files changed, 56 insertions(+), 28 deletions(-) create mode 100644 spec/lib/tasks/form_definition_validator_spec.rb diff --git a/README.md b/README.md index b637e606e..ab098aa84 100644 --- a/README.md +++ b/README.md @@ -163,12 +163,13 @@ Assumptions made by the format: ## JSON Form Validation against Schema To validate the form JSON against the schema you can run: -`rake form_definition:validate` +`rake form_definition:validate["config/forms/2021_22.json"]` -This will validate all forms in: -directories = ["config/forms", "spec/fixtures/forms"] +This will validate the given form definition against the schema in `config/forms/schema/generic.json`. -against the schema in (config/forms/schema/generic.json) +You can also run: +`rake form_definition:validate_all` +This will validate all forms in directories = ["config/forms", "spec/fixtures/forms"] ## Useful documentation (external dependencies) diff --git a/lib/tasks/form_definition.rake b/lib/tasks/form_definition.rake index c2b3c6103..7061acc56 100644 --- a/lib/tasks/form_definition.rake +++ b/lib/tasks/form_definition.rake @@ -13,37 +13,32 @@ end namespace :form_definition do desc "Validate JSON against Generic Form Schema" - task validate: :environment do - puts Rails.root.to_s - path = "config/forms/schema/generic.json" - file = File.open(path) - schema = JSON.parse(file.read) - metaschema = JSON::Validator.validator_for_name("draft4").metaschema + task validate_all: :environment do + puts Rails.root.to_s - puts path + directories = ["config/forms", "spec/fixtures/forms"] + paths = get_all_form_paths(directories) + ["config/forms/schema/generic.json"] - if JSON::Validator.validate(metaschema, schema) - puts "schema valid" - else - puts "schema not valid" - return + paths.each do |path| + Rake::Task["form_definition:validate"].reenable + Rake::Task["form_definition:validate"].invoke(path) end + end - directories = ["config/forms", "spec/fixtures/forms"] - - get_all_form_paths(directories).each do |path| - puts path - file = File.open(path) - data = JSON.parse(file.read) + task :validate, %i[path] => :environment do |_task, args| + path = Rails.root.join(args.path) + file = File.open(path) + form_definition = JSON.parse(file.read) + schema = JSON::Validator.validator_for_name("draft4").metaschema - puts JSON::Validator.fully_validate(schema, data, strict: true) + puts path + puts JSON::Validator.fully_validate(schema, form_definition, strict: true) - begin - JSON::Validator.validate!(schema, data) - rescue JSON::Schema::ValidationError => e - e.message - end + begin + JSON::Validator.validate!(schema, form_definition) + rescue JSON::Schema::ValidationError => e + e.message end end end diff --git a/spec/lib/tasks/form_definition_validator_spec.rb b/spec/lib/tasks/form_definition_validator_spec.rb new file mode 100644 index 000000000..55479fe7b --- /dev/null +++ b/spec/lib/tasks/form_definition_validator_spec.rb @@ -0,0 +1,32 @@ +require "rails_helper" +require "rake" + +describe "rake form_definition:validate_all", type: :task do + subject(:task) { Rake::Task["form_definition:validate_all"] } + + before do + Rake.application.rake_require("tasks/form_definition") + Rake::Task.define_task(:environment) + task.reenable + end + + it "runs the validate task for each form definition in the project" do + expect(Rake::Task["form_definition:validate"]).to receive(:invoke).exactly(5).times + task.invoke + end +end + +describe "rake form_definition:validate", type: :task do + subject(:task) { Rake::Task["form_definition:validate"] } + + before do + Rake.application.rake_require("tasks/form_definition") + Rake::Task.define_task(:environment) + task.reenable + end + + it "runs the validate task for the given form definition" do + expect(JSON::Validator).to receive(:validate!).at_least(1).time + task.invoke("config/forms/2021_2022.json") + end +end From ff5c1ddd264d2708b5ba67ffd92e747d0eedd550 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 15 Nov 2021 10:33:22 +0000 Subject: [PATCH 03/16] Fix major repairs date --- app/controllers/case_logs_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 8eb60af6a..1f6936520 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -107,6 +107,8 @@ private day = params["case_log"]["#{question_key}(3i)"] month = params["case_log"]["#{question_key}(2i)"] year = params["case_log"]["#{question_key}(1i)"] + next unless day.present? && month.present? && year.present? + result[question_key] = Date.new(year.to_i, month.to_i, day.to_i) end next unless question_params From 16086503f9dc612edbaa90b7481db36815393049 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 15 Nov 2021 10:38:02 +0000 Subject: [PATCH 04/16] Fix merge conflict --- app/helpers/check_answers_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 1cc1bc65e..6282f9d06 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -60,6 +60,8 @@ module CheckAnswersHelper case_log[question_key].blank? || !condition.include?(case_log[question_key]) when "radio" case_log[question_key].blank? || !condition.include?(case_log[question_key]) + when "select" + case_log[question_key].blank? || !condition.include?(case_log[question_key]) else raise "Not implemented yet" end From 4c479d4fd3f799a5941005f1bc63c1f11cf47085 Mon Sep 17 00:00:00 2001 From: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> Date: Mon, 15 Nov 2021 12:25:00 +0000 Subject: [PATCH 05/16] Bug fixes (#89) * Check answers to tasklist * Fix status display * Fix case log status * Update footer * Update spec * Fix spec * Make form pages 2/3 width * Make spec failures less likely --- .../soft_validations_controller.js | 2 +- app/views/case_logs/edit.html.erb | 2 +- app/views/form/check_answers.html.erb | 2 +- app/views/form/page.html.erb | 41 ++++++++-------- app/views/layouts/_footer.html.erb | 48 +++++++++++++++++++ app/views/layouts/application.html.erb | 29 +---------- spec/features/case_log_spec.rb | 2 +- 7 files changed, 75 insertions(+), 51 deletions(-) create mode 100644 app/views/layouts/_footer.html.erb diff --git a/app/javascript/controllers/soft_validations_controller.js b/app/javascript/controllers/soft_validations_controller.js index a408e4460..2c223ce1c 100644 --- a/app/javascript/controllers/soft_validations_controller.js +++ b/app/javascript/controllers/soft_validations_controller.js @@ -5,7 +5,7 @@ export default class extends Controller { initialize() { let url = window.location.href + "/soft_validations" - this.fetch_retry(url, { headers: { accept: "application/json" } }, 2) + this.fetch_retry(url, { headers: { accept: "application/json" } }, 5) } fetch_retry(url, options, n) { diff --git a/app/views/case_logs/edit.html.erb b/app/views/case_logs/edit.html.erb index 551ce0cb5..83d442cb9 100644 --- a/app/views/case_logs/edit.html.erb +++ b/app/views/case_logs/edit.html.erb @@ -5,7 +5,7 @@ <%= @case_log.id %>

This submission is - <%= @case_log.status %>

+ <%= @case_log.status.to_s.humanize %>

You've completed <%= get_subsections_count(@form, @case_log, :completed) %> of <%= get_subsections_count(@form, @case_log, :all) %> sections.

<% next_incomplete_section=get_next_incomplete_section(@form, @case_log) %> diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 49ebeda0f..26b153f8f 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -10,7 +10,7 @@ <%end %> <%end %> <% end %> - <%= form_with action: '/case_logs', method: "next_page", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> + <%= form_with model: @case_log, method: "get", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> <%= f.govuk_submit "Save and continue" %> <% end %> diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index 188b16cda..d47e89e7b 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -3,25 +3,28 @@ <% end %> <%= turbo_frame_tag "case_log_form", target: "_top" do %> +

+
+ <% if page_info["header"].present? %> +

+ <%= page_info["header"] %> +

+ <% end %> + <%= form_with model: @case_log, method: "submit_form", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> + <%= f.govuk_error_summary %> + <% page_info["questions"].map do |question_key, question| %> +
<%= display_question_key_div(page_info, question_key) %> > + <%= render partial: "form/#{question["type"]}_question", locals: { question_key: question_key.to_sym, question: question, f: f } %> +
+ <% end %> - <% if page_info["header"].present? %> -

- <%= page_info["header"] %> -

- <% end %> - <%= form_with model: @case_log, method: "submit_form", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> - <%= f.govuk_error_summary %> - <% page_info["questions"].map do |question_key, question| %> -
<%= display_question_key_div(page_info, question_key) %> > - <%= render partial: "form/#{question["type"]}_question", locals: { question_key: question_key.to_sym, question: question, f: f } %> -
- <% end %> + <% if page_info["soft_validations"]&.keys&.first %> + <%= render partial: "form/validation_override_question", locals: { f: f, page_key: page_key, page_info: page_info } %> + <% end %> - <% if page_info["soft_validations"]&.keys&.first %> - <%= render partial: "form/validation_override_question", locals: { f: f, page_key: page_key, page_info: page_info } %> - <% end %> - - <%= f.hidden_field :page, value: page_key %> - <%= f.govuk_submit "Save and continue" %> - <% end %> + <%= f.hidden_field :page, value: page_key %> + <%= f.govuk_submit "Save and continue" %> + <% end %> +
+
<% end %> diff --git a/app/views/layouts/_footer.html.erb b/app/views/layouts/_footer.html.erb new file mode 100644 index 000000000..ffba80da2 --- /dev/null +++ b/app/views/layouts/_footer.html.erb @@ -0,0 +1,48 @@ +
+ +
+
+ +

Support links

+ +
+ +

Get help with this service

+

Online helpdesk

+

+ CORE helpdesk + (opens in a new tab)

+ +

Telephone

+
    +
  • Telephone: 0333 202 5084
    • +
  • Monday to Friday, 9am to 5:30pm (except public holidays)
    • +
+ +

Email

+ + +

Footer links

+ +
+ + + + All content is available under the + Open Government Licence v3.0, except where otherwise stated + +
+
+ +
+
+
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 343307b55..7ea45b6e4 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -72,34 +72,7 @@
-
-
-
-

Support links

- -
-

Support and guidance

- -

- If you have a question, or you've had a problem using this service, please contact us at <%= mail_to "test@mhclg.gov.uk", "test@mhclg.gov.uk", class: "govuk-footer__link" %> -

- -
- - - - - All content is available under the - Open Government Licence v3.0, except where otherwise stated - -
-
- -
-
-
+ <%= render partial: "layouts/footer" %>
diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index b48572d62..5552b0c85 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -43,7 +43,7 @@ RSpec.describe "Test Features" do it "displays a tasklist header" do visit("/case_logs/#{id}") expect(page).to have_content("Tasklist for log #{id}") - expect(page).to have_content("This submission is #{status}") + expect(page).to have_content("This submission is #{status.humanize}") end it "displays a section status" do From 622cb906dcbc62eee61cc955d87758d3fd5ac426 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Mon, 15 Nov 2021 12:30:32 +0000 Subject: [PATCH 06/16] reinclude metaschema validation --- README.md | 5 ++++- lib/tasks/form_definition.rake | 21 ++++++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index ab098aa84..c1afaf7f5 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,10 @@ Assumptions made by the format: ## JSON Form Validation against Schema To validate the form JSON against the schema you can run: -`rake form_definition:validate["config/forms/2021_22.json"]` +`rake form_definition:validate["config/forms/2021_2022.json"]` + +n.b. You may have to escape square brackets in zsh +`rake form_definition:validate\["config/forms/2021_2022.json"\]` This will validate the given form definition against the schema in `config/forms/schema/generic.json`. diff --git a/lib/tasks/form_definition.rake b/lib/tasks/form_definition.rake index 7061acc56..4da4a0ff4 100644 --- a/lib/tasks/form_definition.rake +++ b/lib/tasks/form_definition.rake @@ -15,10 +15,9 @@ namespace :form_definition do desc "Validate JSON against Generic Form Schema" task validate_all: :environment do - puts Rails.root.to_s directories = ["config/forms", "spec/fixtures/forms"] - paths = get_all_form_paths(directories) + ["config/forms/schema/generic.json"] + paths = get_all_form_paths(directories) paths.each do |path| Rake::Task["form_definition:validate"].reenable @@ -27,10 +26,26 @@ namespace :form_definition do end task :validate, %i[path] => :environment do |_task, args| + + puts args + + path = "config/forms/schema/generic.json" + file = File.open(path) + schema = JSON.parse(file.read) + meta_schema = JSON::Validator.validator_for_name("draft4").metaschema + + puts path + + if JSON::Validator.validate(meta_schema, schema) + puts "schema valid" + else + puts "schema not valid" + return + end + path = Rails.root.join(args.path) file = File.open(path) form_definition = JSON.parse(file.read) - schema = JSON::Validator.validator_for_name("draft4").metaschema puts path puts JSON::Validator.fully_validate(schema, form_definition, strict: true) From ef32e2ba67bc980c004e49a77cf6b1f88e034572 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Mon, 15 Nov 2021 13:08:32 +0000 Subject: [PATCH 07/16] fix one validation test --- lib/tasks/form_definition.rake | 6 +++--- spec/lib/tasks/form_definition_validator_spec.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/tasks/form_definition.rake b/lib/tasks/form_definition.rake index 4da4a0ff4..50b649d5e 100644 --- a/lib/tasks/form_definition.rake +++ b/lib/tasks/form_definition.rake @@ -29,7 +29,7 @@ namespace :form_definition do puts args - path = "config/forms/schema/generic.json" + path = Rails.root.join("config/forms/schema/generic.json") file = File.open(path) schema = JSON.parse(file.read) meta_schema = JSON::Validator.validator_for_name("draft4").metaschema @@ -37,9 +37,9 @@ namespace :form_definition do puts path if JSON::Validator.validate(meta_schema, schema) - puts "schema valid" + puts "Schema Definition is Valid" else - puts "schema not valid" + puts "Schema Definition in #{path} is not valid against draft4 json schema." return end diff --git a/spec/lib/tasks/form_definition_validator_spec.rb b/spec/lib/tasks/form_definition_validator_spec.rb index 55479fe7b..a85664b7d 100644 --- a/spec/lib/tasks/form_definition_validator_spec.rb +++ b/spec/lib/tasks/form_definition_validator_spec.rb @@ -11,7 +11,7 @@ describe "rake form_definition:validate_all", type: :task do end it "runs the validate task for each form definition in the project" do - expect(Rake::Task["form_definition:validate"]).to receive(:invoke).exactly(5).times + expect(Rake::Task["form_definition:validate"]).to receive(:invoke).exactly(4).times task.invoke end end From b410ca04f81e131b76a94de818c26a2778fba013 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Mon, 15 Nov 2021 13:25:01 +0000 Subject: [PATCH 08/16] assume schema validation true for test --- lib/tasks/form_definition.rake | 12 ++++++------ spec/lib/tasks/form_definition_validator_spec.rb | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/tasks/form_definition.rake b/lib/tasks/form_definition.rake index 50b649d5e..d3ed40a2a 100644 --- a/lib/tasks/form_definition.rake +++ b/lib/tasks/form_definition.rake @@ -15,7 +15,6 @@ namespace :form_definition do desc "Validate JSON against Generic Form Schema" task validate_all: :environment do - directories = ["config/forms", "spec/fixtures/forms"] paths = get_all_form_paths(directories) @@ -26,7 +25,6 @@ namespace :form_definition do end task :validate, %i[path] => :environment do |_task, args| - puts args path = Rails.root.join("config/forms/schema/generic.json") @@ -36,11 +34,13 @@ namespace :form_definition do puts path + binding.pry + if JSON::Validator.validate(meta_schema, schema) - puts "Schema Definition is Valid" - else - puts "Schema Definition in #{path} is not valid against draft4 json schema." - return + puts "Schema Definition is Valid" + else + puts "Schema Definition in #{path} is not valid against draft4 json schema." + next end path = Rails.root.join(args.path) diff --git a/spec/lib/tasks/form_definition_validator_spec.rb b/spec/lib/tasks/form_definition_validator_spec.rb index a85664b7d..c61b39c3e 100644 --- a/spec/lib/tasks/form_definition_validator_spec.rb +++ b/spec/lib/tasks/form_definition_validator_spec.rb @@ -22,11 +22,12 @@ describe "rake form_definition:validate", type: :task do before do Rake.application.rake_require("tasks/form_definition") Rake::Task.define_task(:environment) + allow(JSON::Validator).to receive(:validate).and_return(true) task.reenable end it "runs the validate task for the given form definition" do - expect(JSON::Validator).to receive(:validate!).at_least(1).time + expect(JSON::Validator).to receive(:validate!).at_least(2).times task.invoke("config/forms/2021_2022.json") end end From 02a848e38fbea811b11cd120481c0aa6b3adc0c2 Mon Sep 17 00:00:00 2001 From: magicmilo Date: Mon, 15 Nov 2021 13:34:10 +0000 Subject: [PATCH 09/16] rubocop and remove dbg --- lib/tasks/form_definition.rake | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/tasks/form_definition.rake b/lib/tasks/form_definition.rake index d3ed40a2a..8155f76e1 100644 --- a/lib/tasks/form_definition.rake +++ b/lib/tasks/form_definition.rake @@ -12,7 +12,7 @@ def get_all_form_paths(directories) end namespace :form_definition do - desc "Validate JSON against Generic Form Schema" + desc "Validate all JSON against Generic Form Schema" task validate_all: :environment do directories = ["config/forms", "spec/fixtures/forms"] @@ -24,9 +24,9 @@ namespace :form_definition do end end - task :validate, %i[path] => :environment do |_task, args| - puts args + desc "Validate Single JSON against Generic Form Schema" + task :validate, %i[path] => :environment do |_task, args| path = Rails.root.join("config/forms/schema/generic.json") file = File.open(path) schema = JSON.parse(file.read) @@ -34,8 +34,6 @@ namespace :form_definition do puts path - binding.pry - if JSON::Validator.validate(meta_schema, schema) puts "Schema Definition is Valid" else From 6c4bb79b6a0ac6bc3f1320ac0088581d96051ccd Mon Sep 17 00:00:00 2001 From: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> Date: Mon, 15 Nov 2021 14:09:44 +0000 Subject: [PATCH 10/16] Bug fixes (#90) * Use the correct blue * Use SASS for hover * Use govuk frontend classes * Don't add new stylesheet --- app/javascript/styles/_task-list.scss | 5 ----- app/javascript/styles/application.scss | 9 ++++++++- app/models/case_log.rb | 4 ++-- app/views/case_logs/_log_list.html.erb | 2 +- app/views/case_logs/edit.html.erb | 2 +- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/javascript/styles/_task-list.scss b/app/javascript/styles/_task-list.scss index 755836e3f..603a6d55c 100644 --- a/app/javascript/styles/_task-list.scss +++ b/app/javascript/styles/_task-list.scss @@ -81,11 +81,6 @@ } } -// turbo-frame { -// display: block; -// border: 1px solid blue -// } - .app-task-list__item:target, .tasklist_item_highlight{ background-color: $govuk-focus-colour; diff --git a/app/javascript/styles/application.scss b/app/javascript/styles/application.scss index 4caf03bda..7e1158bd7 100644 --- a/app/javascript/styles/application.scss +++ b/app/javascript/styles/application.scss @@ -11,4 +11,11 @@ $govuk-image-url-function: frontend-image-url; @import "~govuk-frontend/govuk/all"; -@import '_task-list' +@import '_task-list'; + +$govuk-global-styles: true; + +// turbo-frame { +// display: block; +// border: 1px solid blue +// } diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 1842dc55a..5553b5a78 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -10,7 +10,7 @@ class CaseLogValidator < ActiveModel::Validator # If we've come from the form UI we only want to validate the specific fields # that have just been submitted. If we're submitting a log via API or Bulk Upload # we want to validate all data fields. - page_to_validate = options[:page] + page_to_validate = record.page if page_to_validate public_send("validate_#{page_to_validate}", record) if respond_to?("validate_#{page_to_validate}") else @@ -41,7 +41,7 @@ class CaseLog < ApplicationRecord default_scope -> { kept } scope :not_completed, -> { where.not(status: "completed") } - validates_with CaseLogValidator, ({ page: @page } || {}) + validates_with CaseLogValidator before_save :update_status! attr_accessor :page diff --git a/app/views/case_logs/_log_list.html.erb b/app/views/case_logs/_log_list.html.erb index 3edfabe06..c0f856f09 100644 --- a/app/views/case_logs/_log_list.html.erb +++ b/app/views/case_logs/_log_list.html.erb @@ -12,7 +12,7 @@ <% case_logs.map do |log| %> - <%= link_to log.id, case_log_path(log) %> + <%= link_to log.id, case_log_path(log), class: "govuk-link" %> <%= log.property_postcode %> diff --git a/app/views/case_logs/edit.html.erb b/app/views/case_logs/edit.html.erb index 83d442cb9..349cc6914 100644 --- a/app/views/case_logs/edit.html.erb +++ b/app/views/case_logs/edit.html.erb @@ -9,7 +9,7 @@

You've completed <%= get_subsections_count(@form, @case_log, :completed) %> of <%= get_subsections_count(@form, @case_log, :all) %> sections.

<% next_incomplete_section=get_next_incomplete_section(@form, @case_log) %> - > From 06ac7b8c35a07f62b9dac0a2afd2b2b610593d97 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 15 Nov 2021 14:23:55 +0000 Subject: [PATCH 11/16] Downcase case log status in tasklist --- app/views/case_logs/edit.html.erb | 2 +- spec/features/case_log_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/case_logs/edit.html.erb b/app/views/case_logs/edit.html.erb index 349cc6914..25668b2ac 100644 --- a/app/views/case_logs/edit.html.erb +++ b/app/views/case_logs/edit.html.erb @@ -5,7 +5,7 @@ <%= @case_log.id %>

This submission is - <%= @case_log.status.to_s.humanize %>

+ <%= @case_log.status.to_s.humanize.downcase %>

You've completed <%= get_subsections_count(@form, @case_log, :completed) %> of <%= get_subsections_count(@form, @case_log, :all) %> sections.

<% next_incomplete_section=get_next_incomplete_section(@form, @case_log) %> diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 5552b0c85..a616b7baf 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -43,7 +43,7 @@ RSpec.describe "Test Features" do it "displays a tasklist header" do visit("/case_logs/#{id}") expect(page).to have_content("Tasklist for log #{id}") - expect(page).to have_content("This submission is #{status.humanize}") + expect(page).to have_content("This submission is #{status.humanize.downcase}") end it "displays a section status" do From b358a67dde9cb57cca37691d129e139fa89cf9db Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 15 Nov 2021 15:42:11 +0000 Subject: [PATCH 12/16] CLDC-679: Update main tenancy type mappings (#91) * Update main tenancy type mappins * Fix the tests and validations --- app/constants/db_enums.rb | 9 ++++----- app/models/case_log.rb | 2 +- app/validations/tenancy_validations.rb | 6 +++--- config/forms/2021_2022.json | 11 +++++------ docs/api/DLUHC-CORE-Data.v1.json | 2 +- spec/fixtures/complete_case_log.json | 2 +- spec/models/case_log_spec.rb | 18 +++++++++--------- 7 files changed, 24 insertions(+), 26 deletions(-) diff --git a/app/constants/db_enums.rb b/app/constants/db_enums.rb index 23e4b4bd4..d28393105 100644 --- a/app/constants/db_enums.rb +++ b/app/constants/db_enums.rb @@ -166,11 +166,10 @@ module DbEnums def self.tenancy { - "Fixed term – Secure" => 1, - "Fixed term – Assured Shorthold Tenancy (AST)" => 4, - "Lifetime – Secure" => 100, - "Lifetime – Assured" => 2, - "License agreement" => 5, + "Secure (including flexible)" => 1, + "Assured" => 2, + "Assured Shorthold" => 4, + "Licence agreement (almshouses only)" => 5, "Other" => 3, } end diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 5553b5a78..f73df0264 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -230,7 +230,7 @@ private dynamically_not_required << "incfreq" end - if tenancy == "Fixed term – Secure" + if tenancy == "Secure (including flexible)" dynamically_not_required << "tenancylength" end diff --git a/app/validations/tenancy_validations.rb b/app/validations/tenancy_validations.rb index 32ff8e2b0..fc275f91e 100644 --- a/app/validations/tenancy_validations.rb +++ b/app/validations/tenancy_validations.rb @@ -4,12 +4,12 @@ module TenancyValidations def validate_fixed_term_tenancy(record) is_present = record.tenancylength.present? is_in_range = record.tenancylength.to_i.between?(2, 99) - is_secure = record.tenancy == "Fixed term – Secure" - is_ast = record.tenancy == "Fixed term – Assured Shorthold Tenancy (AST)" + is_secure = record.tenancy == "Secure (including flexible)" + is_ast = record.tenancy == "Assured Shorthold" conditions = [ { condition: !(is_secure || is_ast) && is_present, error: "You must only answer the fixed term tenancy length question if the tenancy type is fixed term" }, { condition: is_ast && !is_in_range, error: "Fixed term – Assured Shorthold Tenancy (AST) should be between 2 and 99 years" }, - { condition: is_secure && (!is_in_range && is_present), error: "Fixed term – Secure should be between 2 and 99 years or not specified" }, + { condition: is_secure && (!is_in_range && is_present), error: "Secure (including flexible) should be between 2 and 99 years or not specified" }, ] conditions.each { |condition| condition[:condition] ? (record.errors.add :tenancylength, condition[:error]) : nil } diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 859e5fa14..7433f4d7b 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -1068,12 +1068,11 @@ "hint_text": "", "type": "radio", "answer_options": { - "0": "Fixed term – Secure", - "1": "Fixed term – Assured Shorthold Tenancy (AST)", - "2": "Lifetime – Secure", - "3": "Lifetime – Assured", - "4": "License agreement", - "5": "Other" + "0": "Secure (including flexible)", + "1": "Assured", + "2": "Assured Shorthold", + "3": "Licence agreement (almshouses only)", + "4": "Other" }, "conditional_for": { "other_tenancy_type": ["Other"] diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json index 17560d14c..5723c9311 100644 --- a/docs/api/DLUHC-CORE-Data.v1.json +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -306,7 +306,7 @@ "startdate": "12/03/2019", "startertenancy": "No", "tenancylength": "No", - "tenancy": "Fixed term – Secure", + "tenancy": "Secure (including flexible)", "lettype": "Affordable Rent - General Needs", "landlord": "This landlord", "la": "Barnet", diff --git a/spec/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json index 4656a04f8..81f2735fc 100644 --- a/spec/fixtures/complete_case_log.json +++ b/spec/fixtures/complete_case_log.json @@ -52,7 +52,7 @@ "startdate": "12/03/2019", "startertenancy": "No", "tenancylength": "5", - "tenancy": "Fixed term – Secure", + "tenancy": "Secure (including flexible)", "lettype": "Affordable Rent - General Needs", "landlord": "This landlord", "la": "Barnet", diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index e663937db..fad6ca4b9 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -182,39 +182,39 @@ RSpec.describe Form, type: :model do it "Must be completed and between 2 and 99 if type of tenancy is Assured shorthold" do expect { - CaseLog.create!(tenancy: "Fixed term – Assured Shorthold Tenancy (AST)", + CaseLog.create!(tenancy: "Assured Shorthold", tenancylength: 1) }.to raise_error(ActiveRecord::RecordInvalid) expect { - CaseLog.create!(tenancy: "Fixed term – Assured Shorthold Tenancy (AST)", + CaseLog.create!(tenancy: "Assured Shorthold", tenancylength: nil) }.to raise_error(ActiveRecord::RecordInvalid) expect { - CaseLog.create!(tenancy: "Fixed term – Assured Shorthold Tenancy (AST)", + CaseLog.create!(tenancy: "Assured Shorthold", tenancylength: 2) }.not_to raise_error end it "Must be empty or between 2 and 99 if type of tenancy is Secure" do expect { - CaseLog.create!(tenancy: "Fixed term – Secure", + CaseLog.create!(tenancy: "Secure (including flexible)", tenancylength: 1) }.to raise_error(ActiveRecord::RecordInvalid) expect { - CaseLog.create!(tenancy: "Fixed term – Secure", + CaseLog.create!(tenancy: "Secure (including flexible)", tenancylength: 100) }.to raise_error(ActiveRecord::RecordInvalid) expect { - CaseLog.create!(tenancy: "Fixed term – Secure", + CaseLog.create!(tenancy: "Secure (including flexible)", tenancylength: nil) }.not_to raise_error expect { - CaseLog.create!(tenancy: "Fixed term – Secure", + CaseLog.create!(tenancy: "Secure (including flexible)", tenancylength: 2) }.not_to raise_error end @@ -294,12 +294,12 @@ RSpec.describe Form, type: :model do it "must not be provided if tenancy type is not other" do expect { - CaseLog.create!(tenancy: "Fixed term – Secure", + CaseLog.create!(tenancy: "Secure (including flexible)", tenancyother: "the other reason provided") }.to raise_error(ActiveRecord::RecordInvalid) expect { - CaseLog.create!(tenancy: "Fixed term – Secure", + CaseLog.create!(tenancy: "Secure (including flexible)", tenancyother: nil) }.not_to raise_error end From b787ad803fd0294bd7a193f5494909f8e5fac3a2 Mon Sep 17 00:00:00 2001 From: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> Date: Mon, 15 Nov 2021 16:38:21 +0000 Subject: [PATCH 13/16] CLDC-709: Conditional section status bugfix (#92) * Failing spec * Fix the failing spec * Fix rake spec --- app/helpers/check_answers_helper.rb | 32 +------------------ app/helpers/tasklist_helper.rb | 21 ++++++------ app/models/form.rb | 30 +++++++++++++++++ app/views/case_logs/_tasklist.html.erb | 4 +-- lib/tasks/form_definition.rake | 10 +++--- spec/factories/case_log.rb | 9 ++++++ spec/helpers/tasklist_helper_spec.rb | 27 ++++++++++------ .../tasks/form_definition_validator_spec.rb | 2 +- 8 files changed, 76 insertions(+), 59 deletions(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 6282f9d06..ba6373b72 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -16,7 +16,7 @@ module CheckAnswersHelper while page_name.to_s != "check_answers" && subsection_keys.include?(page_name) questions = form.questions_for_page(page_name) - applicable_questions = filter_conditional_questions(questions, case_log) + applicable_questions = form.filter_conditional_questions(questions, case_log) total_questions = total_questions.merge(applicable_questions) page_name = get_next_page_name(form, page_name, case_log) @@ -25,19 +25,6 @@ module CheckAnswersHelper total_questions end - def filter_conditional_questions(questions, case_log) - applicable_questions = questions - - questions.each do |k, question| - question.fetch("conditional_for", []).each do |conditional_question_key, condition| - if condition_not_met(case_log, k, question, condition) - applicable_questions = applicable_questions.reject { |z| z == conditional_question_key } - end - end - end - applicable_questions - end - def get_next_page_name(form, page_name, case_log) page = form.all_pages[page_name] if page.key?("conditional_route_to") @@ -50,23 +37,6 @@ module CheckAnswersHelper form.next_page(page_name) end - def condition_not_met(case_log, question_key, question, condition) - case question["type"] - when "numeric" - operator = condition[/[<>=]+/].to_sym - operand = condition[/\d+/].to_i - case_log[question_key].blank? || !case_log[question_key].send(operator, operand) - when "text" - case_log[question_key].blank? || !condition.include?(case_log[question_key]) - when "radio" - case_log[question_key].blank? || !condition.include?(case_log[question_key]) - when "select" - case_log[question_key].blank? || !condition.include?(case_log[question_key]) - else - raise "Not implemented yet" - end - 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 diff --git a/app/helpers/tasklist_helper.rb b/app/helpers/tasklist_helper.rb index 4ca9f182d..1d3f376c5 100644 --- a/app/helpers/tasklist_helper.rb +++ b/app/helpers/tasklist_helper.rb @@ -13,31 +13,32 @@ module TasklistHelper in_progress: "govuk-tag--blue", }.freeze - def get_subsection_status(subsection_name, case_log, questions) + def get_subsection_status(subsection_name, case_log, form, questions) + applicable_questions = form.filter_conditional_questions(questions, case_log).keys if subsection_name == "declaration" return case_log.completed? ? :not_started : :cannot_start_yet end - return :not_started if questions.all? { |question| case_log[question].blank? } - return :completed if questions.all? { |question| case_log[question].present? } + return :not_started if applicable_questions.all? { |question| case_log[question].blank? } + return :completed if applicable_questions.all? { |question| case_log[question].present? } :in_progress end def get_next_incomplete_section(form, case_log) subsections = form.all_subsections.keys - subsections.find { |subsection| is_incomplete?(subsection, case_log, form.questions_for_subsection(subsection).keys) } + subsections.find { |subsection| is_incomplete?(subsection, case_log, form, form.questions_for_subsection(subsection)) } end def get_subsections_count(form, case_log, status = :all) subsections = form.all_subsections.keys return subsections.count if status == :all - subsections.count { |subsection| get_subsection_status(subsection, case_log, form.questions_for_subsection(subsection).keys) == status } + subsections.count { |subsection| get_subsection_status(subsection, case_log, form, form.questions_for_subsection(subsection)) == status } end def get_first_page_or_check_answers(subsection, case_log, form, questions) - path = if is_started?(subsection, case_log, questions) + path = if is_started?(subsection, case_log, form, questions) "case_log_#{subsection}_check_answers_path" else "case_log_#{form.first_page_for_subsection(subsection)}_path" @@ -47,13 +48,13 @@ module TasklistHelper private - def is_incomplete?(subsection, case_log, questions) - status = get_subsection_status(subsection, case_log, questions) + def is_incomplete?(subsection, case_log, form, questions) + status = get_subsection_status(subsection, case_log, form, questions) %i[not_started in_progress].include?(status) end - def is_started?(subsection, case_log, questions) - status = get_subsection_status(subsection, case_log, questions) + def is_started?(subsection, case_log, form, questions) + status = get_subsection_status(subsection, case_log, form, questions) %i[in_progress completed].include?(status) end end diff --git a/app/models/form.rb b/app/models/form.rb index 3eeac8987..a2479e20b 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -97,6 +97,36 @@ class Form }.reduce(:merge) end + def filter_conditional_questions(questions, case_log) + applicable_questions = questions + + questions.each do |k, question| + question.fetch("conditional_for", []).each do |conditional_question_key, condition| + if condition_not_met(case_log, k, question, condition) + applicable_questions = applicable_questions.reject { |z| z == conditional_question_key } + end + end + end + applicable_questions + end + + def condition_not_met(case_log, question_key, question, condition) + case question["type"] + when "numeric" + operator = condition[/[<>=]+/].to_sym + operand = condition[/\d+/].to_i + case_log[question_key].blank? || !case_log[question_key].send(operator, operand) + when "text" + case_log[question_key].blank? || !condition.include?(case_log[question_key]) + when "radio" + case_log[question_key].blank? || !condition.include?(case_log[question_key]) + when "select" + case_log[question_key].blank? || !condition.include?(case_log[question_key]) + else + raise "Not implemented yet" + end + end + def get_answer_label(case_log, question_title) question = all_questions[question_title] if question["type"] == "checkbox" diff --git a/app/views/case_logs/_tasklist.html.erb b/app/views/case_logs/_tasklist.html.erb index fdf4c5aa9..1e75f207b 100644 --- a/app/views/case_logs/_tasklist.html.erb +++ b/app/views/case_logs/_tasklist.html.erb @@ -9,10 +9,10 @@