From a0d481ef69f6e356ffdbe69df4894c32dfb2618c Mon Sep 17 00:00:00 2001
From: Kat <katrina@kosiak.co.uk>
Date: Tue, 18 Jun 2024 15:02:15 +0100
Subject: [PATCH] Allow clearing answers
---
...eck_answers_summary_list_card_component.rb | 2 +-
app/controllers/check_errors_controller.rb | 75 ++++++++++++++++++
.../check_your_errors_controller.rb | 23 ------
app/controllers/form_controller.rb | 53 +++++++++++--
app/frontend/styles/application.scss | 8 ++
.../confirm_clear_answer.html.erb | 31 ++++++++
app/views/form/check_errors.html.erb | 67 +++++++++++++---
app/views/form/page.html.erb | 5 ++
spec/requests/check_errors_controller_spec.rb | 79 ++++++++++++++++---
9 files changed, 289 insertions(+), 54 deletions(-)
create mode 100644 app/controllers/check_errors_controller.rb
delete mode 100644 app/controllers/check_your_errors_controller.rb
create mode 100644 app/views/check_errors/confirm_clear_answer.html.erb
diff --git a/app/components/check_answers_summary_list_card_component.rb b/app/components/check_answers_summary_list_card_component.rb
index b37e226b0..c95d1dd64 100644
--- a/app/components/check_answers_summary_list_card_component.rb
+++ b/app/components/check_answers_summary_list_card_component.rb
@@ -34,7 +34,7 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base
send("#{log.model_name.param_key}_#{question.page.id}_path", log, referrer:)
end
- def correct_validation_action_href(question, log, related_question_ids)
+ def correct_validation_action_href(question, log, _related_question_ids)
if question.displayed_as_answered?(log)
lettings_log_confirm_clear_answer_path(log, question_id: question.id)
else
diff --git a/app/controllers/check_errors_controller.rb b/app/controllers/check_errors_controller.rb
new file mode 100644
index 000000000..8a69106a9
--- /dev/null
+++ b/app/controllers/check_errors_controller.rb
@@ -0,0 +1,75 @@
+class CheckErrorsController < ApplicationController
+ include DuplicateLogsHelper
+
+ before_action :authenticate_user!
+ before_action :find_resource_by_named_id
+
+ def confirm_clear_answer
+ return render_not_found unless @log
+
+ @related_question_ids = params["lettings_log"].keys.reject { |id| id == "page_id" }
+ question_id = @related_question_ids.find { |id| !params[id].nil? }
+ @question = @log.form.get_question(question_id, @log)
+ @page = @log.form.get_page(params["lettings_log"]["page_id"])
+ end
+
+private
+
+ def find_resource_by_named_id
+ @log = if params[:sales_log_id].present?
+ current_user.sales_logs.visible.find_by(id: params[:sales_log_id])
+ else
+ current_user.lettings_logs.visible.find_by(id: params[:lettings_log_id])
+ end
+ end
+end
+
+# def restore_error_field_values(questions)
+# return unless questions
+
+# questions.each do |question|
+# if question&.type == "date" && @log.attributes.key?(question.id)
+# @log[question.id] = @log.send("#{question.id}_was")
+# end
+# end
+# end
+
+def responses_for_page(page)
+ page.questions.each_with_object({}) do |question, result|
+ question_params = params[@log.model_name.param_key][question.id]
+ if question.type == "date"
+ day = params[@log.model_name.param_key]["#{question.id}(3i)"]
+ month = params[@log.model_name.param_key]["#{question.id}(2i)"]
+ year = params[@log.model_name.param_key]["#{question.id}(1i)"]
+ next unless [day, month, year].any?(&:present?)
+
+ result[question.id] = if Date.valid_date?(year.to_i, month.to_i, day.to_i) && year.to_i.positive?
+ Date.new(year.to_i, month.to_i, day.to_i)
+ else
+ Date.new(0, 1, 1)
+ end
+ end
+
+ if question.id == "saledate" && set_managing_organisation_to_assigned_to_organisation?(result["saledate"])
+ result["managing_organisation_id"] = @log.assigned_to.organisation_id
+ end
+
+ next unless question_params
+
+ if %w[checkbox validation_override].include?(question.type)
+ question.answer_keys_without_dividers.each do |option|
+ result[option] = question_params.include?(option) ? 1 : 0
+ end
+ else
+ result[question.id] = question_params
+ end
+
+ if question.id == "owning_organisation_id"
+ owning_organisation = result["owning_organisation_id"].present? ? Organisation.find(result["owning_organisation_id"]) : nil
+
+ result["managing_organisation_id"] = owning_organisation.id if set_managing_organisation_to_owning_organisation?(owning_organisation)
+ end
+
+ result
+ end
+end
diff --git a/app/controllers/check_your_errors_controller.rb b/app/controllers/check_your_errors_controller.rb
deleted file mode 100644
index ca5e23349..000000000
--- a/app/controllers/check_your_errors_controller.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-class CheckYourErrorsController < ApplicationController
- include DuplicateLogsHelper
-
- before_action :authenticate_user!
- before_action :find_resource_by_named_id
-
- def index
- return render_not_found unless @log
-
- related_question_ids = params[:related_question_ids]
- @questions = @log.form.questions.select { |q| related_question_ids.include?(q.id.to_s) }
- end
-
-private
-
- def find_resource_by_named_id
- @log = if params[:sales_log_id].present?
- current_user.sales_logs.visible.find_by(id: params[:sales_log_id])
- else
- current_user.lettings_logs.visible.find_by(id: params[:lettings_log_id])
- end
- end
-end
diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb
index c2890fb89..2e170d4d5 100644
--- a/app/controllers/form_controller.rb
+++ b/app/controllers/form_controller.rb
@@ -9,6 +9,8 @@ class FormController < ApplicationController
def submit_form
if @log
@page = form.get_page(params[@log.model_name.param_key][:page])
+ return render_check_errors_page if params["check_errors"]
+
shown_page_ids_with_unanswered_questions_before_update = @page.subsection.pages
.select { |page| page.routed_to?(@log, current_user) }
.select { |page| page.has_unanswered_questions?(@log) }
@@ -34,12 +36,7 @@ class FormController < ApplicationController
Rails.logger.info "User triggered validation(s) on: #{error_attributes.join(', ')}"
@subsection = form.subsection_for_page(@page)
restore_error_field_values(@page&.questions)
- if params["check_errors"]
- @questions = @log.form.questions.select { |q| error_attributes.include?(q.id.to_sym) }
- render "form/check_errors"
- else
- render "form/page"
- end
+ render "form/page"
end
else
render_not_found
@@ -70,12 +67,23 @@ class FormController < ApplicationController
@interruption_page_referrer_type = from_referrer_query("referrer")
end
+ if adding_answer_from_check_errors_page?
+ @related_question_ids = request.params["related_question_ids"]
+ @original_page_id = request.params["original_page_id"]
+ @check_errors = true
+ end
+
if @log
page_id = request.path.split("/")[-1].underscore
@page = form.get_page(page_id)
@subsection = form.subsection_for_page(@page)
- if @page.routed_to?(@log, current_user) || is_referrer_type?("interruption_screen")
- render "form/page"
+ if @page.routed_to?(@log, current_user) || is_referrer_type?("interruption_screen") || adding_answer_from_check_errors_page?
+ if updated_answer_from_check_errors_page?
+ @questions = request.params["related_question_ids"].map { |id| @log.form.get_question(id, @log) }
+ render "form/check_errors"
+ else
+ render "form/page"
+ end
else
redirect_to @log.lettings? ? lettings_log_path(@log) : sales_log_path(@log)
end
@@ -219,6 +227,11 @@ private
return send("#{@log.class.name.underscore}_#{previous_interruption_screen_page_id}_path", @log, { referrer: previous_interruption_screen_referrer, original_log_id: original_duplicate_log_id_from_query }.compact)
end
+ if params["lettings_log"]["check_errors"]
+ @page = form.get_page(params["lettings_log"]["page"])
+ return send("#{@log.class.name.underscore}_#{params['lettings_log']['original_page_id']}_path", @log, { check_errors: true, related_question_ids: params["lettings_log"]["related_question_ids"].split(" ") }.compact)
+ end
+
is_new_answer_from_check_answers = is_referrer_type?("check_answers_new_answer")
redirect_path = form.next_page_redirect_path(@page, @log, current_user, ignore_answered: is_new_answer_from_check_answers)
referrer = is_new_answer_from_check_answers ? "check_answers_new_answer" : nil
@@ -380,4 +393,28 @@ private
true
end
+
+ def render_check_errors_page
+ if params["lettings_log"]["clear_question_id"]
+ question_id = params["lettings_log"]["clear_question_id"]
+ @log.form.get_question(question_id, @log).page.questions.map(&:id).each { |id| @log[id] = nil }
+ @log.save!
+ @questions = params["lettings_log"].keys.reject { |id| %w[clear_question_id page].include?(id) }.map { |id| @log.form.get_question(id, @log) }
+ else
+ responses_for_page = responses_for_page(@page)
+ @log.assign_attributes(responses_for_page)
+ @log.valid?
+ error_attributes = @log.errors.map(&:attribute)
+ @questions = @log.form.questions.select { |q| error_attributes.include?(q.id.to_sym) }
+ end
+ render "form/check_errors"
+ end
+
+ def adding_answer_from_check_errors_page?
+ request.params["referrer"] == "check_errors"
+ end
+
+ def updated_answer_from_check_errors_page?
+ params["check_errors"]
+ end
end
diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss
index a81214894..1367e78e9 100644
--- a/app/frontend/styles/application.scss
+++ b/app/frontend/styles/application.scss
@@ -81,3 +81,11 @@ $govuk-breakpoints: (
.govuk-footer {
border-top: govuk-spacing(2) solid $govuk-brand-colour;
}
+
+.submit-button-link {
+ background: none;
+ border: none;
+ color: #1d70b8;
+ text-decoration: underline;
+ cursor: pointer;
+}
\ No newline at end of file
diff --git a/app/views/check_errors/confirm_clear_answer.html.erb b/app/views/check_errors/confirm_clear_answer.html.erb
new file mode 100644
index 000000000..8549eae78
--- /dev/null
+++ b/app/views/check_errors/confirm_clear_answer.html.erb
@@ -0,0 +1,31 @@
+<% content_for :before_content do %>
+ <% content_for :title, "Are you sure you want to clear #{@question.check_answer_label}?" %>
+<% end %>
+
+<div class="govuk-grid-row">
+ <div class="govuk-grid-column-two-thirds-from-desktop">
+ <h1 class="govuk-heading-xl">
+ <%= content_for(:title) %>
+ </h1>
+
+ <%= govuk_warning_text(text: "This action is permanent") %>
+ <%= form_with model: @log, url: send("lettings_log_#{@page.id}_path", @log), method: "post", local: true do |f| %>
+
+ <% @related_question_ids.each do |id| %>
+ <%= f.hidden_field id, value: @log[id] %>
+ <% end %>
+
+ <%= f.hidden_field :clear_question_id, value: @question.id %>
+ <%= f.hidden_field :page, value: @page.id %>
+
+ <div class="govuk-button-group">
+ <%= f.govuk_submit "Confirm and continue", name: "check_errors" %>
+ <%= govuk_button_link_to(
+ "Cancel",
+ "javascript:history.back()",
+ secondary: true,
+ ) %>
+ </div>
+ <% end %>
+ </div>
+</div>
diff --git a/app/views/form/check_errors.html.erb b/app/views/form/check_errors.html.erb
index e498a0d40..94852618b 100644
--- a/app/views/form/check_errors.html.erb
+++ b/app/views/form/check_errors.html.erb
@@ -1,20 +1,63 @@
<div class="govuk-grid-row">
<div class="govuk-grid-column-three-quarters-from-desktop">
- <%= form_with model: @log, url: request.original_url, method: "post", local: true do |f| %>
- <%= f.govuk_error_summary %>
- <% end %>
- <h1 class="govuk-heading-m govuk-!-margin-bottom-6">
- <span class="govuk-grid-column-two-thirds">
- Make sure these answers are correct:
- </span>
- <span class="govuk-body govuk-!-text-align-right govuk-grid-column-one-third">
- <%= govuk_link_to "Clear all", lettings_log_confirm_clear_all_answers_path(@log) %>
- </span>
- </h1>
+
+ <%= form_with model: @log, url: lettings_log_confirm_clear_answer_path(@log), method: "post", local: true do |f| %>
+ <%= f.govuk_error_summary %>
+ <%= f.hidden_field :page_id, value: @page.id %>
+
+ <h1 class="govuk-heading-m govuk-!-margin-bottom-6">
+ <span class="govuk-grid-column-two-thirds">
+ Make sure these answers are correct:
+ </span>
+ <span class="govuk-body govuk-!-text-align-right govuk-grid-column-one-third">
+ <%= govuk_link_to "Clear all", lettings_log_confirm_clear_all_answers_path(@log) %>
+ </span>
+ </h1>
+
+ <div class="govuk-summary-card__content">
+ <% applicable_questions = @questions.reject { |q| q.hidden_in_check_answers?(@log, current_user) }%>
+ <% applicable_questions.each do |question| %>
+ <%= f.hidden_field question.id, value: @log[question.id] %>
+ <dl class="govuk-summary-list">
+ <div class="govuk-summary-list__row">
+ <dt class="govuk-summary-list__key">
+ <%= get_question_label(question) %>
+ </dt>
+ <dd class="govuk-summary-list__value">
+ <%= simple_format(
+ get_answer_label(question, @log),
+ wrapper_tag: "span",
+ class: "govuk-!-margin-right-4",
+ ) %>
- <%= render CheckAnswersSummaryListCardComponent.new(questions: @questions, log: @log, user: current_user, correcting_hard_validation: true) %>
+ <% extra_value = question.get_extra_check_answer_value(@log) %>
+ <% if extra_value && question.answer_label(@log).present? %>
+ <%= simple_format(
+ extra_value,
+ wrapper_tag: "span",
+ class: "govuk-!-font-weight-regular app-!-colour-muted",
+ ) %>
+ <% end %>
+
+ <% question.get_inferred_answers(@log).each do |inferred_answer| %>
+ <span class="govuk-!-font-weight-regular app-!-colour-muted"><%= inferred_answer %></span>
+ <% end %>
+ </dd>
+ <dd class="govuk-summary-list__actions">
+ <% if question.displayed_as_answered?(@log) %>
+ <input type="submit" value="Clear" name=<%= question.id %> class="govuk-body govuk-link submit-button-link" >
+ <% else %>
+ <%= govuk_link_to "Answer", send("#{@log.model_name.param_key}_#{question.page.id}_path", @log, referrer: "check_errors", original_page_id: @page.id, related_question_ids: applicable_questions.map(&:id)) %>
+ <% end %>
+ </dd>
+ </div>
+ </dl>
+ <% end %>
+ </div>
+ <% end %>
+
<%= govuk_button_link_to "Confirm and continue", "/" %>
</div>
</div>
diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb
index 48bfc4edc..5453de7f7 100644
--- a/app/views/form/page.html.erb
+++ b/app/views/form/page.html.erb
@@ -70,6 +70,11 @@
<%= f.hidden_field :page, value: @page.id %>
<%= f.hidden_field :interruption_page_id, value: @interruption_page_id %>
<%= f.hidden_field :interruption_page_referrer_type, value: @interruption_page_referrer_type %>
+ <%= f.hidden_field :related_question_ids, value: @related_question_ids %>
+ <%= f.hidden_field :original_page_id, value: @original_page_id %>
+ <% if @check_errors %>
+ <%= f.hidden_field :check_errors, value: @check_errors %>
+ <% end %>
<% if all_questions_with_errors.count > 1 %>
<div class="govuk-button-group">
diff --git a/spec/requests/check_errors_controller_spec.rb b/spec/requests/check_errors_controller_spec.rb
index 28e4131b8..c62c78b5f 100644
--- a/spec/requests/check_errors_controller_spec.rb
+++ b/spec/requests/check_errors_controller_spec.rb
@@ -50,9 +50,7 @@ RSpec.describe CheckErrorsController, type: :request do
end
it "displays correct clear links" do
- expect(page).to have_link("Clear", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?question_id=hhmemb")
- expect(page).to have_link("Clear", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?question_id=ecstat1")
- expect(page).to have_link("Clear", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?question_id=earnings")
+ expect(page).to have_button("Clear", count: 3)
expect(page).to have_link("Clear all", href: "/lettings-logs/#{lettings_log.id}/confirm-clear-all-answers")
end
end
@@ -62,7 +60,7 @@ RSpec.describe CheckErrorsController, type: :request do
describe "confirm clear answer page" do
context "when user is not signed in" do
it "redirects to sign in page" do
- get "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?original_page_id=income_amount&question_id=hhmemb&related_question_ids%5B%5D=hhmemb&related_question_ids%5B%5D=ecstat1&related_question_ids%5B%5D=earnings", params: {}
+ post "/lettings-logs/#{lettings_log.id}/confirm-clear-answer", params: {}
expect(response).to redirect_to("/account/sign-in")
end
end
@@ -75,23 +73,36 @@ RSpec.describe CheckErrorsController, type: :request do
end
it "renders page not found" do
- get "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?original_page_id=income_amount&question_id=hhmemb&related_question_ids%5B%5D=hhmemb&related_question_ids%5B%5D=ecstat1&related_question_ids%5B%5D=earnings", params: {}
+ post "/lettings-logs/#{lettings_log.id}/confirm-clear-answer", params: {}
expect(response).to have_http_status(:not_found)
end
end
context "when user is signed in" do
- context "with multiple error fields and answered questions" do
+ context "and clearing specific question" do
+ let(:params) do
+ {
+ id: lettings_log.id,
+ lettings_log: {
+ earnings: "100000",
+ incfreq: "1",
+ hhmemb: "2",
+ page_id: "income_amount",
+ },
+ hhmemb: "",
+ }
+ end
+
before do
sign_in user
- get "/lettings-logs/#{lettings_log.id}/confirm-clear-answer?original_page_id=income_amount&question_id=hhmemb&related_question_ids%5B%5D=hhmemb&related_question_ids%5B%5D=ecstat1&related_question_ids%5B%5D=earnings", params: {}
+ post "/lettings-logs/#{lettings_log.id}/confirm-clear-answer", params:
end
it "displays correct clear links" do
- expect(page).to have_content("Are you sure you want to clear Number of household members answer?")
+ expect(page).to have_content("Are you sure you want to clear Number of household members?")
expect(page).to have_content("This action is permanent")
- expect(page).to have_button("Cancel", href: "/lettings-logs/#{lettings_log.id}/income-amount")
- expect(page).to have_button("Confirm and continue", href: "/lettings-logs/#{lettings_log.id}/clear-answer?original_page_id=income_amount&question_id=hhmemb&related_question_ids%5B%5D=hhmemb&related_question_ids%5B%5D=ecstat1&related_question_ids%5B%5D=earnings")
+ expect(page).to have_link("Cancel")
+ expect(page).to have_button("Confirm and continue")
end
end
end
@@ -101,6 +112,54 @@ RSpec.describe CheckErrorsController, type: :request do
end
describe "clear answer" do
+ context "when user is not signed in" do
+ it "redirects to sign in page" do
+ post "/lettings-logs/#{lettings_log.id}/income-amount", params: {}
+ expect(response).to redirect_to("/account/sign-in")
+ end
+ end
+
+ context "when the user is from different organisation" do
+ let(:other_user) { create(:user) }
+
+ before do
+ sign_in other_user
+ end
+
+ it "renders page not found" do
+ post "/lettings-logs/#{lettings_log.id}/income-amount", params: {}
+ expect(response).to have_http_status(:not_found)
+ end
+ end
+
+ context "when user is signed in" do
+ context "and clearing specific question" do
+ let(:params) do
+ {
+ id: lettings_log.id,
+ lettings_log: {
+ earnings: "100000",
+ incfreq: "1",
+ hhmemb: "2",
+ clear_question_id: "hhmemb",
+ page: "income_amount",
+ },
+ check_errors: "",
+ }
+ end
+
+ before do
+ sign_in user
+ post "/lettings-logs/#{lettings_log.id}/income-amount", params:
+ end
+
+ it "displays correct clear links" do
+ expect(page).to have_content("Make sure these answers are correct")
+ expect(page).to have_content("You didn’t answer this question")
+ expect(page).to have_link("Answer")
+ end
+ end
+ end
end
describe "clear all answers" do