From c937c1218daba2133f8980b4a5ea343ef056b709 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Thu, 15 Sep 2022 13:08:24 +0100 Subject: [PATCH] Logs type refactoring (#871) * Log where generic * Move question invalidation to form class * Rubocop --- app/models/form.rb | 24 +++++ app/models/form/page.rb | 4 +- app/models/form/question.rb | 94 +++++++++---------- app/models/form/setup/pages/created_by.rb | 2 +- app/models/form/setup/pages/organisation.rb | 2 +- .../form/setup/questions/created_by_id.rb | 10 +- .../setup/questions/owning_organisation_id.rb | 6 +- app/models/form/subsection.rb | 28 +++--- app/models/lettings_log.rb | 33 +------ app/models/log.rb | 12 +++ 10 files changed, 110 insertions(+), 105 deletions(-) diff --git a/app/models/form.rb b/app/models/form.rb index fd8e66317..a463e497f 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -155,6 +155,30 @@ class Form questions.reject { |q| q.page.routed_to?(log, current_user) || q.derived? || callback_questions.include?(q.id) } || [] end + def reset_not_routed_questions(log) + enabled_questions = enabled_page_questions(log) + enabled_question_ids = enabled_questions.map(&:id) + + invalidated_page_questions(log).each do |question| + if %w[radio checkbox].include?(question.type) + enabled_answer_options = enabled_question_ids.include?(question.id) ? enabled_questions.find { |q| q.id == question.id }.answer_options : {} + current_answer_option_valid = enabled_answer_options.present? ? enabled_answer_options.key?(log.public_send(question.id).to_s) : false + if !current_answer_option_valid && log.respond_to?(question.id.to_s) + Rails.logger.debug("Cleared #{question.id} value") + log.public_send("#{question.id}=", nil) + else + (question.answer_options.keys - enabled_answer_options.keys).map do |invalid_answer_option| + Rails.logger.debug("Cleared #{invalid_answer_option} value") + log.public_send("#{invalid_answer_option}=", nil) if log.respond_to?(invalid_answer_option) + end + end + else + Rails.logger.debug("Cleared #{question.id} value") + log.public_send("#{question.id}=", nil) unless enabled_question_ids.include?(question.id) + end + end + end + def enabled_page_questions(log) questions - invalidated_page_questions(log) end diff --git a/app/models/form/page.rb b/app/models/form/page.rb index 031b812a6..3ee1932cc 100644 --- a/app/models/form/page.rb +++ b/app/models/form/page.rb @@ -18,10 +18,10 @@ class Form::Page delegate :form, to: :subsection - def routed_to?(lettings_log, _current_user) + def routed_to?(log, _current_user) return true unless depends_on || subsection.depends_on - subsection.enabled?(lettings_log) && form.depends_on_met(depends_on, lettings_log) + subsection.enabled?(log) && form.depends_on_met(depends_on, log) end def non_conditional_questions diff --git a/app/models/form/question.rb b/app/models/form/question.rb index 8e5778000..4478f63fd 100644 --- a/app/models/form/question.rb +++ b/app/models/form/question.rb @@ -44,31 +44,31 @@ class Form::Question delegate :subsection, to: :page delegate :form, to: :subsection - def answer_label(lettings_log) - return checkbox_answer_label(lettings_log) if type == "checkbox" - return lettings_log[id]&.to_formatted_s(:govuk_date).to_s if type == "date" + def answer_label(log) + return checkbox_answer_label(log) if type == "checkbox" + return log[id]&.to_formatted_s(:govuk_date).to_s if type == "date" - answer = label_from_value(lettings_log[id]) if lettings_log[id].present? - answer_label = [prefix, format_value(answer), suffix_label(lettings_log)].join("") if answer + answer = label_from_value(log[id]) if log[id].present? + answer_label = [prefix, format_value(answer), suffix_label(log)].join("") if answer return answer_label if answer_label - has_inferred_check_answers_value?(lettings_log) ? inferred_check_answers_value["value"] : "" + has_inferred_check_answers_value?(log) ? inferred_check_answers_value["value"] : "" end - def get_inferred_answers(lettings_log) + def get_inferred_answers(log) return [] unless inferred_answers - enabled_inferred_answers(inferred_answers, lettings_log).keys.map do |question_id| - question = form.get_question(question_id, lettings_log) + enabled_inferred_answers(inferred_answers, log).keys.map do |question_id| + question = form.get_question(question_id, log) if question.present? - question.label_from_value(lettings_log[question_id]) + question.label_from_value(log[question_id]) else - Array(question_id.to_s.split(".")).inject(lettings_log) { |log, method| log.present? ? log.public_send(*method) : "" } + Array(question_id.to_s.split(".")).inject(log) { |l, method| l.present? ? l.public_send(*method) : "" } end end end - def get_extra_check_answer_value(_lettings_log) + def get_extra_check_answer_value(_log) nil end @@ -76,48 +76,48 @@ class Form::Question !!readonly end - def enabled?(lettings_log) + def enabled?(log) return true if conditional_on.blank? - conditional_on.all? { |condition| evaluate_condition(condition, lettings_log) } + conditional_on.all? { |condition| evaluate_condition(condition, log) } end - def hidden_in_check_answers?(lettings_log, _current_user = nil) + def hidden_in_check_answers?(log, _current_user = nil) if hidden_in_check_answers.is_a?(Hash) - form.depends_on_met(hidden_in_check_answers["depends_on"], lettings_log) + form.depends_on_met(hidden_in_check_answers["depends_on"], log) else hidden_in_check_answers end end - def displayed_to_user?(lettings_log) - page.routed_to?(lettings_log, nil) && enabled?(lettings_log) + def displayed_to_user?(log) + page.routed_to?(log, nil) && enabled?(log) end def derived? !!derived end - def has_inferred_check_answers_value?(lettings_log) - return true if selected_answer_option_is_derived?(lettings_log) - return inferred_check_answers_value["condition"].values[0] == lettings_log[inferred_check_answers_value["condition"].keys[0]] if inferred_check_answers_value.present? + def has_inferred_check_answers_value?(log) + return true if selected_answer_option_is_derived?(log) + return inferred_check_answers_value["condition"].values[0] == log[inferred_check_answers_value["condition"].keys[0]] if inferred_check_answers_value.present? false end - def displayed_answer_options(lettings_log) + def displayed_answer_options(log) answer_options.select do |_key, val| - !val.is_a?(Hash) || !val["depends_on"] || form.depends_on_met(val["depends_on"], lettings_log) + !val.is_a?(Hash) || !val["depends_on"] || form.depends_on_met(val["depends_on"], log) end end - def action_text(lettings_log) - if has_inferred_check_answers_value?(lettings_log) + def action_text(log) + if has_inferred_check_answers_value?(log) "Change" elsif type == "checkbox" - answer_options.keys.any? { |key| value_is_yes?(lettings_log[key]) } ? "Change" : "Answer" + answer_options.keys.any? { |key| value_is_yes?(log[key]) } ? "Change" : "Answer" else - lettings_log[id].blank? ? "Answer" : "Change" + log[id].blank? ? "Answer" : "Change" end end @@ -125,10 +125,10 @@ class Form::Question "/#{log.model_name.param_key.dasherize}s/#{log.id}/#{page_id.to_s.dasherize}?referrer=check_answers" end - def completed?(lettings_log) - return answer_options.keys.any? { |key| value_is_yes?(lettings_log[key]) } if type == "checkbox" + def completed?(log) + return answer_options.keys.any? { |key| value_is_yes?(log[key]) } if type == "checkbox" - lettings_log[id].present? || !lettings_log.respond_to?(id.to_sym) || has_inferred_display_value?(lettings_log) + log[id].present? || !log.respond_to?(id.to_sym) || has_inferred_display_value?(log) end def value_from_label(label) @@ -203,7 +203,7 @@ class Form::Question I18n.t("validations.not_answered", question: display_label.downcase) end - def suffix_label(lettings_log) + def suffix_label(log) return "" unless suffix return suffix if suffix.is_a?(String) @@ -213,7 +213,7 @@ class Form::Question condition = s["depends_on"] next unless condition - answer = lettings_log.send(condition.keys.first) + answer = log.send(condition.keys.first) if answer == condition.values.first label = s["label"] end @@ -239,10 +239,10 @@ class Form::Question resource.hint end - def answer_selected?(lettings_log, answer) + def answer_selected?(log, answer) return false unless type == "select" - lettings_log[id].to_s == answer.id.to_s + log[id].to_s == answer.id.to_s end def top_guidance? @@ -255,20 +255,20 @@ class Form::Question private - def selected_answer_option_is_derived?(lettings_log) - selected_option = answer_options&.dig(lettings_log[id].to_s.presence) - selected_option.is_a?(Hash) && selected_option["depends_on"] && form.depends_on_met(selected_option["depends_on"], lettings_log) + def selected_answer_option_is_derived?(log) + selected_option = answer_options&.dig(log[id].to_s.presence) + selected_option.is_a?(Hash) && selected_option["depends_on"] && form.depends_on_met(selected_option["depends_on"], log) end - def has_inferred_display_value?(lettings_log) - inferred_check_answers_value.present? && lettings_log[inferred_check_answers_value["condition"].keys.first] == inferred_check_answers_value["condition"].values.first + def has_inferred_display_value?(log) + inferred_check_answers_value.present? && log[inferred_check_answers_value["condition"].keys.first] == inferred_check_answers_value["condition"].values.first end - def checkbox_answer_label(lettings_log) + def checkbox_answer_label(log) answer = [] - return "Yes" if id == "declaration" && value_is_yes?(lettings_log["declaration"]) + return "Yes" if id == "declaration" && value_is_yes?(log["declaration"]) - answer_options.each { |key, options| value_is_yes?(lettings_log[key]) ? answer << options["value"] : nil } + answer_options.each { |key, options| value_is_yes?(log[key]) ? answer << options["value"] : nil } answer.join(", ") end @@ -282,21 +282,21 @@ private end end - def evaluate_condition(condition, lettings_log) + def evaluate_condition(condition, log) case page.questions.find { |q| q.id == condition[:from] }.type when "numeric" operator = condition[:cond][/[<>=]+/].to_sym operand = condition[:cond][/\d+/].to_i - lettings_log[condition[:from]].present? && lettings_log[condition[:from]].send(operator, operand) + log[condition[:from]].present? && log[condition[:from]].send(operator, operand) when "text", "radio", "select" - lettings_log[condition[:from]].present? && condition[:cond].include?(lettings_log[condition[:from]]) + log[condition[:from]].present? && condition[:cond].include?(log[condition[:from]]) else raise "Not implemented yet" end end - def enabled_inferred_answers(inferred_answers, lettings_log) - inferred_answers.filter { |_key, value| value.all? { |condition_key, condition_value| lettings_log[condition_key] == condition_value } } + def enabled_inferred_answers(inferred_answers, log) + inferred_answers.filter { |_key, value| value.all? { |condition_key, condition_value| log[condition_key] == condition_value } } end RADIO_YES_VALUE = { diff --git a/app/models/form/setup/pages/created_by.rb b/app/models/form/setup/pages/created_by.rb index 8878e9b75..979171e5d 100644 --- a/app/models/form/setup/pages/created_by.rb +++ b/app/models/form/setup/pages/created_by.rb @@ -13,7 +13,7 @@ class Form::Setup::Pages::CreatedBy < ::Form::Page ] end - def routed_to?(_lettings_log, current_user) + def routed_to?(_log, current_user) !!current_user&.support? end end diff --git a/app/models/form/setup/pages/organisation.rb b/app/models/form/setup/pages/organisation.rb index 2b0c56d12..4844627c2 100644 --- a/app/models/form/setup/pages/organisation.rb +++ b/app/models/form/setup/pages/organisation.rb @@ -13,7 +13,7 @@ class Form::Setup::Pages::Organisation < ::Form::Page ] end - def routed_to?(_lettings_log, current_user) + def routed_to?(_log, current_user) !!current_user&.support? end end diff --git a/app/models/form/setup/questions/created_by_id.rb b/app/models/form/setup/questions/created_by_id.rb index 85567f882..f25d4917c 100644 --- a/app/models/form/setup/questions/created_by_id.rb +++ b/app/models/form/setup/questions/created_by_id.rb @@ -19,10 +19,10 @@ class Form::Setup::Questions::CreatedById < ::Form::Question end end - def displayed_answer_options(lettings_log) - return answer_options unless lettings_log.owning_organisation + def displayed_answer_options(log) + return answer_options unless log.owning_organisation - user_ids = lettings_log.owning_organisation.users.pluck(:id) + [""] + user_ids = log.owning_organisation.users.pluck(:id) + [""] answer_options.select { |k, _v| user_ids.include?(k) } end @@ -32,7 +32,7 @@ class Form::Setup::Questions::CreatedById < ::Form::Question answer_options[value] end - def hidden_in_check_answers?(_lettings_log, current_user) + def hidden_in_check_answers?(_log, current_user) !current_user.support? end @@ -42,7 +42,7 @@ class Form::Setup::Questions::CreatedById < ::Form::Question private - def selected_answer_option_is_derived?(_lettings_log) + def selected_answer_option_is_derived?(_log) false end end diff --git a/app/models/form/setup/questions/owning_organisation_id.rb b/app/models/form/setup/questions/owning_organisation_id.rb index a8fd15e50..ea3da3e25 100644 --- a/app/models/form/setup/questions/owning_organisation_id.rb +++ b/app/models/form/setup/questions/owning_organisation_id.rb @@ -19,7 +19,7 @@ class Form::Setup::Questions::OwningOrganisationId < ::Form::Question end end - def displayed_answer_options(_lettings_log) + def displayed_answer_options(_log) answer_options end @@ -29,7 +29,7 @@ class Form::Setup::Questions::OwningOrganisationId < ::Form::Question answer_options[value] end - def hidden_in_check_answers?(_lettings_log, current_user) + def hidden_in_check_answers?(_log, current_user) !current_user.support? end @@ -39,7 +39,7 @@ class Form::Setup::Questions::OwningOrganisationId < ::Form::Question private - def selected_answer_option_is_derived?(_lettings_log) + def selected_answer_option_is_derived?(_log) false end end diff --git a/app/models/form/subsection.rb b/app/models/form/subsection.rb index d42f8dadb..d3ab0ddd2 100644 --- a/app/models/form/subsection.rb +++ b/app/models/form/subsection.rb @@ -17,40 +17,40 @@ class Form::Subsection @questions ||= pages.flat_map(&:questions) end - def enabled?(lettings_log) + def enabled?(log) return true unless depends_on depends_on.any? do |conditions_set| conditions_set.all? do |subsection_id, dependent_status| - form.get_subsection(subsection_id).status(lettings_log) == dependent_status.to_sym + form.get_subsection(subsection_id).status(log) == dependent_status.to_sym end end end - def status(lettings_log) - unless enabled?(lettings_log) + def status(log) + unless enabled?(log) return :cannot_start_yet end - qs = applicable_questions(lettings_log) - qs_optional_removed = qs.reject { |q| lettings_log.optional_fields.include?(q.id) } - return :not_started if qs.count.positive? && qs.all? { |question| lettings_log[question.id].blank? || question.read_only? || question.derived? } - return :completed if qs_optional_removed.all? { |question| question.completed?(lettings_log) } + qs = applicable_questions(log) + qs_optional_removed = qs.reject { |q| log.optional_fields.include?(q.id) } + return :not_started if qs.count.positive? && qs.all? { |question| log[question.id].blank? || question.read_only? || question.derived? } + return :completed if qs_optional_removed.all? { |question| question.completed?(log) } :in_progress end - def is_incomplete?(lettings_log) - %i[not_started in_progress].include?(status(lettings_log)) + def is_incomplete?(log) + %i[not_started in_progress].include?(status(log)) end - def is_started?(lettings_log) - %i[in_progress completed].include?(status(lettings_log)) + def is_started?(log) + %i[in_progress completed].include?(status(log)) end - def applicable_questions(lettings_log) + def applicable_questions(log) questions.select do |q| - (q.displayed_to_user?(lettings_log) && !q.derived?) || q.has_inferred_check_answers_value?(lettings_log) + (q.displayed_to_user?(log) && !q.derived?) || q.has_inferred_check_answers_value?(log) end end end diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 2db5e2101..33d5974ce 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -505,30 +505,6 @@ private PIO = PostcodeService.new - def reset_not_routed_questions - enabled_questions = form.enabled_page_questions(self) - enabled_question_ids = enabled_questions.map(&:id) - - form.invalidated_page_questions(self).each do |question| - if %w[radio checkbox].include?(question.type) - enabled_answer_options = enabled_question_ids.include?(question.id) ? enabled_questions.find { |q| q.id == question.id }.answer_options : {} - current_answer_option_valid = enabled_answer_options.present? ? enabled_answer_options.key?(public_send(question.id).to_s) : false - if !current_answer_option_valid && respond_to?(question.id.to_s) - Rails.logger.debug("Cleared #{question.id} value") - public_send("#{question.id}=", nil) - else - (question.answer_options.keys - enabled_answer_options.keys).map do |invalid_answer_option| - Rails.logger.debug("Cleared #{invalid_answer_option} value") - public_send("#{invalid_answer_option}=", nil) if respond_to?(invalid_answer_option) - end - end - else - Rails.logger.debug("Cleared #{question.id} value") - public_send("#{question.id}=", nil) unless enabled_question_ids.include?(question.id) - end - end - end - def reset_derived_questions dependent_questions = { waityear: [{ key: :renewal, value: 0 }], referral: [{ key: :renewal, value: 0 }], @@ -546,12 +522,6 @@ private end end - def reset_created_by - return unless created_by && owning_organisation - - self.created_by = nil if created_by.organisation != owning_organisation - end - def reset_scheme return unless scheme && owning_organisation @@ -559,11 +529,10 @@ private end def reset_invalidated_dependent_fields! - return unless form + super reset_created_by reset_scheme - reset_not_routed_questions reset_derived_questions end diff --git a/app/models/log.rb b/app/models/log.rb index d0a866ae9..e24a205fc 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -58,4 +58,16 @@ private subsection_statuses = form.subsections.map { |subsection| subsection.status(self) }.uniq subsection_statuses.all? { |status| not_started_statuses.include?(status) } end + + def reset_created_by + return unless created_by && owning_organisation + + self.created_by = nil if created_by.organisation != owning_organisation + end + + def reset_invalidated_dependent_fields! + return unless form + + form.reset_not_routed_questions(self) + end end