diff --git a/Gemfile b/Gemfile index 2b84ee40c..b42128991 100644 --- a/Gemfile +++ b/Gemfile @@ -29,6 +29,8 @@ gem "discard" gem "activeadmin" # Admin charts gem "chartkick" +# Spreadsheet parsing +gem "roo" # Json Schema gem "json-schema" gem "uk_postcode" diff --git a/Gemfile.lock b/Gemfile.lock index a5d222b0c..b6f1816d1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -302,6 +302,9 @@ GEM actionpack (>= 5.0) railties (>= 5.0) rexml (3.2.5) + roo (2.8.3) + nokogiri (~> 1) + rubyzip (>= 1.3.0, < 3.0.0) rubocop (1.21.0) parallel (~> 1.10) parser (>= 3.0.0.0) @@ -420,6 +423,7 @@ DEPENDENCIES puma (~> 5.0) rack-mini-profiler (~> 2.0) rails (~> 6.1.4) + roo rspec-core! rspec-expectations! rspec-mocks! diff --git a/README.md b/README.md index b637e606e..c1afaf7f5 100644 --- a/README.md +++ b/README.md @@ -163,12 +163,16 @@ 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_2022.json"]` -This will validate all forms in: -directories = ["config/forms", "spec/fixtures/forms"] +n.b. You may have to escape square brackets in zsh +`rake form_definition:validate\["config/forms/2021_2022.json"\]` -against the schema in (config/forms/schema/generic.json) +This will validate the given form definition 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/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/controllers/bulk_upload_controller.rb b/app/controllers/bulk_upload_controller.rb new file mode 100644 index 000000000..ba552cb49 --- /dev/null +++ b/app/controllers/bulk_upload_controller.rb @@ -0,0 +1,22 @@ +class BulkUploadController < ApplicationController + def show + @bulk_upload = BulkUpload.new(nil, nil) + render "case_logs/bulk_upload" + end + + def bulk_upload + file = upload_params.tempfile + content_type = upload_params.content_type + @bulk_upload = BulkUpload.new(file, content_type) + @bulk_upload.process + if @bulk_upload.errors.present? + render "case_logs/bulk_upload", status: :unprocessable_entity + else + redirect_to(case_logs_path) + end + end + + def upload_params + params.require("bulk_upload")["case_log_bulk_upload"] + end +end diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 2755e0763..b3bffb4f4 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -109,6 +109,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 diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 1cc1bc65e..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,21 +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]) - 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/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/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/bulk_upload.rb b/app/models/bulk_upload.rb new file mode 100644 index 000000000..f546ea56d --- /dev/null +++ b/app/models/bulk_upload.rb @@ -0,0 +1,199 @@ +class BulkUpload + include ActiveModel::Model + include ActiveModel::Validations + include ActiveModel::Conversion + + SPREADSHEET_CONTENT_TYPES = %w[ + application/vnd.ms-excel + application/vnd.openxmlformats-officedocument.spreadsheetml.sheet + ].freeze + + FIRST_DATA_ROW = 7 + + def initialize(file, content_type) + @file = file + @content_type = content_type + end + + def process + return unless valid_content_type? + + xlsx = Roo::Spreadsheet.open(@file, extension: :xlsx) + sheet = xlsx.sheet(0) + last_row = sheet.last_row + if last_row < FIRST_DATA_ROW + errors.add(:case_log_bulk_upload, "No data found") + else + data_range = FIRST_DATA_ROW..last_row + data_range.map do |row_num| + case_log = CaseLog.create + map_row(sheet.row(row_num)).each do |attr_key, attr_val| + begin + case_log.update_attribute(attr_key, attr_val) + rescue ArgumentError + end + end + end + end + end + + def valid_content_type? + if SPREADSHEET_CONTENT_TYPES.include?(@content_type) + true + else + errors.add(:case_log_bulk_upload, "Invalid file type") + false + end + end + + def map_row(row) + { + lettype: row[1], + landlord: row[2], + # reg_num_la_core_code: row[3], + # managementgroup: row[4], + # schemecode: row[5], + # firstletting: row[6], + tenant_code: row[7], + startertenancy: row[8], + tenancy: row[9], + tenancyother: row[10], + # tenancyduration: row[11], + other_hhmemb: other_hhmemb(row), + hhmemb: other_hhmemb(row) + 1, + age1: row[12], + age2: row[13], + age3: row[14], + age4: row[15], + age5: row[16], + age6: row[17], + age7: row[18], + age8: row[19], + sex1: row[20], + sex2: row[21], + sex3: row[22], + sex4: row[23], + sex5: row[24], + sex6: row[25], + sex7: row[26], + sex8: row[27], + relat2: row[28], + relat3: row[29], + relat4: row[30], + relat5: row[31], + relat6: row[32], + relat7: row[33], + relat8: row[34], + ecstat1: row[35], + ecstat2: row[36], + ecstat3: row[37], + ecstat4: row[38], + ecstat5: row[39], + ecstat6: row[40], + ecstat7: row[41], + ecstat8: row[42], + ethnic: row[43], + national: row[44], + armed_forces: row[45], + reservist: row[46], + preg_occ: row[47], + hb: row[48], + benefits: row[49], + net_income_known: row[50].present? ? 1 : nil, + earnings: row[50], + # increfused: row[51], + reason: row[52], + other_reason_for_leaving_last_settled_home: row[53], + underoccupation_benefitcap: row[54], + housingneeds_a: row[55], + housingneeds_b: row[56], + housingneeds_c: row[57], + housingneeds_f: row[58], + housingneeds_g: row[59], + housingneeds_h: row[60], + prevten: row[61], + prevloc: row[62], + # ppostc1: row[63], + # ppostc2: row[64], + # prevpco_unknown: row[65], + layear: row[66], + lawaitlist: row[67], + homeless: row[68], + reasonpref: row[69], + rp_homeless: row[70], + rp_insan_unsat: row[71], + rp_medwel: row[72], + rp_hardship: row[73], + rp_dontknow: row[74], + cbl: row[75], + chr: row[76], + cap: row[77], + # referral_source: row[78], + period: row[79], + brent: row[80], + scharge: row[81], + pscharge: row[82], + supcharg: row[83], + tcharge: row[84], + # tcharge_care_homes: row[85], + # no_rent_or_charge: row[86], + hbrentshortfall: row[87], + tshortfall: row[88], + property_void_date: row[89].to_s + row[90].to_s + row[91].to_s, + # property_void_date_day: row[89], + # property_void_date_month: row[90], + # property_void_date_year: row[91], + majorrepairs: row[92].present? ? "1" : nil, + mrcdate: row[92].to_s + row[93].to_s + row[94].to_s, + mrcday: row[92], + mrcmonth: row[93], + mrcyear: row[94], + # supported_scheme: row[95], + startdate: row[96].to_s + row[97].to_s + row[98].to_s, + # startdate_day: row[96], + # startdate_month: row[97], + # startdate_year: row[98], + offered: row[99], + # property_reference: row[100], + beds: row[101], + unittype_gn: row[102], + property_building_type: row[103], + wchair: row[104], + property_relet: row[105], + rsnvac: row[106], + la: row[107], + # postcode: row[108], + # postcod2: row[109], + # row[110] removed + property_owner_organisation: row[111], + # username: row[112], + property_manager_organisation: row[113], + leftreg: row[114], + # uprn: row[115], + incfreq: row[116], + # sheltered_accom: row[117], + illness: row[118], + illness_type_1: row[119], + illness_type_2: row[120], + illness_type_3: row[121], + illness_type_4: row[122], + illness_type_8: row[123], + illness_type_5: row[124], + illness_type_6: row[125], + illness_type_7: row[126], + illness_type_9: row[127], + illness_type_10: row[128], + # london_affordable: row[129], + rent_type: row[130], + intermediate_rent_product_name: row[131], + # data_protection: row[132], + sale_or_letting: "letting", + gdpr_acceptance: 1, + gdpr_declined: 0, + } + end + + def other_hhmemb(row) + [13, 14, 15, 16, 17, 18, 19].count { |idx| row[idx].present? } + end +end diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 1842dc55a..5b5dc6f40 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 @@ -39,9 +39,8 @@ class CaseLog < ApplicationRecord include SoftValidations include DbEnums 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 @@ -129,6 +128,8 @@ class CaseLog < ApplicationRecord end def weekly_net_income + return unless earnings && incfreq + case incfreq when "Weekly" earnings @@ -230,7 +231,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/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/validations/household_validations.rb b/app/validations/household_validations.rb index d3536ae8a..bb04599c7 100644 --- a/app/validations/household_validations.rb +++ b/app/validations/household_validations.rb @@ -45,7 +45,7 @@ module HouseholdValidations end end - def validate_household_pregnancy(record) + def validate_pregnancy(record) if (record.preg_occ == "Yes" || record.preg_occ == "Prefer not to say") && !women_of_child_bearing_age_in_household(record) record.errors.add :preg_occ, "You must answer no as there are no female tenants aged 16-50 in the property" 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/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/_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 @@