diff --git a/app/services/condition_repository.rb b/app/services/condition_repository.rb index 860f2afca..15dd9857f 100644 --- a/app/services/condition_repository.rb +++ b/app/services/condition_repository.rb @@ -29,7 +29,15 @@ def save!(record) def destroy(record) condition = Api::V1::ConditionResource.new(record.attributes, true) condition.prefix_options = record.prefix_options - condition.destroy # rubocop:disable Rails/SaveBang + + begin + condition.destroy # rubocop:disable Rails/SaveBang + rescue ActiveResource::ResourceNotFound + # ActiveRecord::Persistence#destroy doesn't raise an error + # if record has already been destroyed, let's emulate that + end + + record end end end diff --git a/app/services/form_repository.rb b/app/services/form_repository.rb index 26f85118f..6f5cdaabd 100644 --- a/app/services/form_repository.rb +++ b/app/services/form_repository.rb @@ -40,7 +40,15 @@ def archive!(record) def destroy(record) form = Api::V1::FormResource.new(record.attributes, true) - form.destroy # rubocop:disable Rails/SaveBang + + begin + form.destroy # rubocop:disable Rails/SaveBang + rescue ActiveResource::ResourceNotFound + # ActiveRecord::Persistence#destroy doesn't raise an error + # if record has already been destroyed, let's emulate that + end + + record end def pages(record) diff --git a/app/services/page_repository.rb b/app/services/page_repository.rb index b6fbd1f77..c8621ba67 100644 --- a/app/services/page_repository.rb +++ b/app/services/page_repository.rb @@ -34,7 +34,15 @@ def save!(record) def destroy(record) page = Api::V1::PageResource.new(record.attributes, true) page.prefix_options = record.prefix_options - page.destroy # rubocop:disable Rails/SaveBang + + begin + page.destroy # rubocop:disable Rails/SaveBang + rescue ActiveResource::ResourceNotFound + # ActiveRecord::Persistence#destroy doesn't raise an error + # if record has already been destroyed, let's emulate that + end + + record end def move_page(record, direction) diff --git a/spec/requests/pages/routes_controller_spec.rb b/spec/requests/pages/routes_controller_spec.rb index 0ba5e479f..52504a0e2 100644 --- a/spec/requests/pages/routes_controller_spec.rb +++ b/spec/requests/pages/routes_controller_spec.rb @@ -60,9 +60,9 @@ end describe "#destroy" do - let(:condition) { build :condition, routing_page_id: selected_page.id, check_page_id: selected_page.id, goto_page_id: pages.last.id, answer_value: "Option 1" } + let(:condition) { build :condition, id: 1, routing_page_id: selected_page.id, check_page_id: selected_page.id, goto_page_id: pages.last.id, answer_value: "Option 1" } let(:secondary_skip_page) { form.pages[2] } - let(:secondary_skip) { build :condition, routing_page_id: secondary_skip_page.id, check_page_id: selected_page.id, goto_page_id: pages[3].id } + let(:secondary_skip) { build :condition, id: 2, routing_page_id: secondary_skip_page.id, check_page_id: selected_page.id, goto_page_id: pages[3].id } before do allow(PageRepository).to receive(:find).with(page_id: "101", form_id: 1).and_return(selected_page) @@ -84,6 +84,22 @@ expect(ConditionRepository).to receive(:destroy).with(have_attributes(id: secondary_skip.id)) delete destroy_routes_path(form_id: form.id, page_id: selected_page.id, pages_routes_delete_confirmation_input: { confirm: "yes" }) end + + context "but one of the routes is already deleted" do + before do + # forms-api may choose to delete the secondary skip when the condition is deleted + allow(ConditionRepository).to receive(:destroy).and_call_original + ActiveResource::HttpMock.respond_to do |mock| + mock.delete "/api/v1/forms/#{form.id}/pages/#{selected_page.id}/conditions/#{condition.id}", delete_headers, nil, 204 + mock.delete "/api/v1/forms/#{form.id}/pages/#{secondary_skip_page.id}/conditions/#{secondary_skip.id}", delete_headers, nil, 404 + end + end + + it "does not render an error page" do + delete destroy_routes_path(form_id: form.id, page_id: selected_page.id, pages_routes_delete_confirmation_input: { confirm: "yes" }) + expect(response).not_to be_client_error + end + end end context "when given invalid params" do diff --git a/spec/services/condition_repository_spec.rb b/spec/services/condition_repository_spec.rb index 1c91cf4eb..492da657b 100644 --- a/spec/services/condition_repository_spec.rb +++ b/spec/services/condition_repository_spec.rb @@ -72,5 +72,36 @@ described_class.destroy(condition) expect(Api::V1::ConditionResource.new(id: 4, form_id: 1, page_id: 2)).to have_been_deleted end + + it "returns the deleted condition" do + condition = described_class.find(condition_id: 4, form_id: 1, page_id: 2) + expect(described_class.destroy(condition)).to eq condition + end + + context "when the condition has already been deleted" do + it "does not raise an error" do + condition = described_class.find(condition_id: 4, form_id: 1, page_id: 2) + described_class.destroy(condition) + + ActiveResource::HttpMock.respond_to do |mock| + mock.delete "/api/v1/forms/1/pages/2/conditions/4", delete_headers, nil, 404 + end + + expect { + described_class.destroy(condition) + }.not_to raise_error + end + + it "returns the deleted condition" do + condition = described_class.find(condition_id: 4, form_id: 1, page_id: 2) + described_class.destroy(condition) + + ActiveResource::HttpMock.respond_to do |mock| + mock.delete "/api/v1/forms/1/pages/2/conditions/4", delete_headers, nil, 404 + end + + expect(described_class.destroy(condition)).to eq condition + end + end end end diff --git a/spec/services/form_repository_spec.rb b/spec/services/form_repository_spec.rb index 7d2daea4c..add82e3c3 100644 --- a/spec/services/form_repository_spec.rb +++ b/spec/services/form_repository_spec.rb @@ -144,6 +144,37 @@ described_class.destroy(form) expect(Api::V1::FormResource.new(id: 2)).to have_been_deleted end + + it "returns the deleted form" do + form = described_class.find(form_id: 2) + expect(described_class.destroy(form)).to eq form + end + + context "when the form has already been deleted" do + it "does not raise an error" do + form = described_class.find(form_id: 2) + described_class.destroy(form) + + ActiveResource::HttpMock.respond_to do |mock| + mock.delete "/api/v1/forms/2", delete_headers, nil, 404 + end + + expect { + described_class.destroy(form) + }.not_to raise_error + end + + it "returns the deleted form" do + form = described_class.find(form_id: 2) + described_class.destroy(form) + + ActiveResource::HttpMock.respond_to do |mock| + mock.delete "/api/v1/forms/2", delete_headers, nil, 404 + end + + expect(described_class.destroy(form)).to eq form + end + end end describe "#pages" do diff --git a/spec/services/page_repository_spec.rb b/spec/services/page_repository_spec.rb index 1423bce27..110065ffb 100644 --- a/spec/services/page_repository_spec.rb +++ b/spec/services/page_repository_spec.rb @@ -74,6 +74,37 @@ described_class.destroy(page) expect(Api::V1::PageResource.new(id: 2, form_id: 1)).to have_been_deleted end + + it "returns the deleted page" do + page = described_class.find(page_id: 2, form_id: 1) + expect(described_class.destroy(page)).to eq page + end + + context "when the page has already been deleted" do + it "does not raise an error" do + page = described_class.find(page_id: 2, form_id: 1) + described_class.destroy(page) + + ActiveResource::HttpMock.respond_to do |mock| + mock.delete "/api/v1/forms/1/pages/2", delete_headers, nil, 404 + end + + expect { + described_class.destroy(page) + }.not_to raise_error + end + + it "returns the deleted page" do + page = described_class.find(page_id: 2, form_id: 1) + described_class.destroy(page) + + ActiveResource::HttpMock.respond_to do |mock| + mock.delete "/api/v1/forms/1/pages/2", delete_headers, nil, 404 + end + + expect(described_class.destroy(page)).to eq page + end + end end describe "#move_page" do