From d8b496de68bc7c8fa7a44526f3541e44f58e724a Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 4 Feb 2025 14:40:25 +0200 Subject: [PATCH] Refactor DeleteConditionInput - Don't mock DeleteConditionInput object in request specs - Delegate to condition record rather than requiring caller to unpack - Add spec for validation errors --- .../pages/conditions_controller.rb | 6 +++--- .../pages/delete_condition_input.rb | 4 +++- .../pages/delete_condition_input_spec.rb | 2 +- .../pages/conditions_controller_spec.rb | 21 ++++--------------- .../pages/conditions/delete.html.erb_spec.rb | 15 ++++++++++++- 5 files changed, 25 insertions(+), 23 deletions(-) diff --git a/app/controllers/pages/conditions_controller.rb b/app/controllers/pages/conditions_controller.rb index 3d4dbf01c..de84a5e88 100644 --- a/app/controllers/pages/conditions_controller.rb +++ b/app/controllers/pages/conditions_controller.rb @@ -60,7 +60,7 @@ def update def delete condition = ConditionRepository.find(condition_id: params[:condition_id], form_id: current_form.id, page_id: page.id) - delete_condition_input = Pages::DeleteConditionInput.new(form: current_form, page:, record: condition, answer_value: condition.answer_value, goto_page_id: condition.goto_page_id) + delete_condition_input = Pages::DeleteConditionInput.new(form: current_form, page:, record: condition) render template: "pages/conditions/delete", locals: { delete_condition_input: } end @@ -68,7 +68,7 @@ def delete def destroy condition = ConditionRepository.find(condition_id: params[:condition_id], form_id: current_form.id, page_id: page.id) - form_params = delete_condition_input_params.merge(record: condition, answer_value: condition.answer_value, goto_page_id: condition.goto_page_id) + form_params = delete_condition_input_params.merge(record: condition) delete_condition_input = Pages::DeleteConditionInput.new(form_params) @@ -94,7 +94,7 @@ def condition_input_params end def delete_condition_input_params - params.require(:pages_delete_condition_input).permit(:answer_value, :goto_page_id, :confirm).merge(form: current_form, page:) + params.require(:pages_delete_condition_input).permit(:confirm).merge(form: current_form, page:) end def new_condition_or_show_routes_path(page) diff --git a/app/input_objects/pages/delete_condition_input.rb b/app/input_objects/pages/delete_condition_input.rb index 95c918ac9..a534c5b3a 100644 --- a/app/input_objects/pages/delete_condition_input.rb +++ b/app/input_objects/pages/delete_condition_input.rb @@ -1,5 +1,7 @@ class Pages::DeleteConditionInput < ConfirmActionInput - attr_accessor :form, :page, :check_page_id, :routing_page_id, :answer_value, :goto_page_id, :record + attr_accessor :form, :page, :record + + delegate :check_page_id, :routing_page_id, :goto_page_id, :answer_value, to: :record def submit return false if invalid? diff --git a/spec/input_objects/pages/delete_condition_input_spec.rb b/spec/input_objects/pages/delete_condition_input_spec.rb index e967fa637..2cabc05a6 100644 --- a/spec/input_objects/pages/delete_condition_input_spec.rb +++ b/spec/input_objects/pages/delete_condition_input_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" RSpec.describe Pages::DeleteConditionInput, type: :model do - let(:delete_condition_input) { described_class.new(form:, page:, record: condition, goto_page_id:) } + let(:delete_condition_input) { described_class.new(form:, page:, record: condition) } let(:form) { build :form, :ready_for_routing, id: 1 } let(:pages) { form.pages } let(:page) { pages.second } diff --git a/spec/requests/pages/conditions_controller_spec.rb b/spec/requests/pages/conditions_controller_spec.rb index 90fa60bdb..36b38946d 100644 --- a/spec/requests/pages/conditions_controller_spec.rb +++ b/spec/requests/pages/conditions_controller_spec.rb @@ -305,12 +305,6 @@ allow(ConditionRepository).to receive(:find).and_return(condition) - delete_condition_input = Pages::DeleteConditionInput.new(form:, page: selected_page, record: condition, answer_value: "Yes", goto_page_id: 3) - - allow(delete_condition_input).to receive(:goto_page_question_text).and_return("What is your name?") - - allow(Pages::DeleteConditionInput).to receive(:new).and_return(delete_condition_input) - get delete_condition_path(form_id: 1, page_id: selected_page.id, condition_id: condition.id) end @@ -338,25 +332,19 @@ describe "#destroy" do let(:condition) { build :condition, id: 1, routing_page_id: pages.first.id, check_page_id: pages.first.id, answer_value: "Wales", goto_page_id: pages.last.id } let(:confirm) { "yes" } - let(:submit_bool) { true } + let(:destroy_bool) { true } before do selected_page.routing_conditions = [condition] selected_page.position = 1 allow(PageRepository).to receive(:find).and_return(selected_page) - allow(ConditionRepository).to receive_messages(find: condition, destroy: nil) - - delete_condition_input = Pages::DeleteConditionInput.new(form:, page: selected_page, record: condition, answer_value: "Wales", goto_page_id: 3, confirm:) - - allow(delete_condition_input).to receive_messages(goto_page_question_text: "What is your name?", submit: submit_bool) - - allow(Pages::DeleteConditionInput).to receive(:new).and_return(delete_condition_input) + allow(ConditionRepository).to receive_messages(find: condition, destroy: destroy_bool) delete destroy_condition_path(form_id: form.id, page_id: selected_page.id, condition_id: condition.id, - params: { pages_delete_condition_input: { confirm:, goto_page_id: 3, answer_value: "Wales" } }) + params: { pages_delete_condition_input: { confirm: } }) end it "reads the form" do @@ -381,7 +369,7 @@ end context "when the destroy fails" do - let(:submit_bool) { false } + let(:destroy_bool) { false } it "return 422 error code" do expect(response).to have_http_status(:unprocessable_entity) @@ -389,7 +377,6 @@ end context "when form submit fails" do - let(:submit_bool) { false } let(:confirm) { nil } it "return 422 error code" do diff --git a/spec/views/pages/conditions/delete.html.erb_spec.rb b/spec/views/pages/conditions/delete.html.erb_spec.rb index a27f91832..02beeaff1 100644 --- a/spec/views/pages/conditions/delete.html.erb_spec.rb +++ b/spec/views/pages/conditions/delete.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "pages/conditions/delete.html.erb" do - let(:delete_condition_input) { Pages::DeleteConditionInput.new(form:, page:, record: condition, answer_value: condition.answer_value, goto_page_id: condition.goto_page_id) } + let(:delete_condition_input) { Pages::DeleteConditionInput.new(form:, page:, record: condition) } let(:form) { build :form, :ready_for_routing, id: 1 } let(:condition) { build :condition, id: 1, routing_page_id: 1, check_page_id: 1, answer_value: "Wales", goto_page_id: pages.last.id } let(:pages) { form.pages } @@ -30,4 +30,17 @@ it "has a submit button" do expect(rendered).to have_css("button[type='submit'].govuk-button", text: I18n.t("save_and_continue")) end + + context "when there is a validation error" do + let(:delete_condition_input) do + delete_condition_input = Pages::DeleteConditionInput.new(form:, page:, record: condition) + delete_condition_input.confirm = nil + delete_condition_input.validate + delete_condition_input + end + + it "renders an error summary" do + expect(rendered).to have_css ".govuk-error-summary" + end + end end