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 %> + +
+
+

+ <%= content_for(:title) %> +

+ + <%= 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 %> + +
+ <%= f.govuk_submit "Confirm and continue", name: "check_errors" %> + <%= govuk_button_link_to( + "Cancel", + "javascript:history.back()", + secondary: true, + ) %> +
+ <% end %> +
+
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 @@
- <%= form_with model: @log, url: request.original_url, method: "post", local: true do |f| %> - <%= f.govuk_error_summary %> - <% end %> -

- - Make sure these answers are correct: - - - <%= govuk_link_to "Clear all", lettings_log_confirm_clear_all_answers_path(@log) %> - -

+ + <%= 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 %> + +

+ + Make sure these answers are correct: + + + <%= govuk_link_to "Clear all", lettings_log_confirm_clear_all_answers_path(@log) %> + +

+ +
+ <% 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] %> +
+
+
+ <%= get_question_label(question) %> +
+
+ <%= 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| %> + <%= inferred_answer %> + <% end %> +
+
+ <% if question.displayed_as_answered?(@log) %> + 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 %> +
+
+
+ <% end %> +
+ <% end %> + <%= govuk_button_link_to "Confirm and continue", "/" %>
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 %>
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