Skip to content

Commit

Permalink
Change repository destroy methods to not raise error if record has al…
Browse files Browse the repository at this point in the history
…ready been deleted

`ActiveRecord::Persistence#destroy` does not raise an error if a record
has already been deleted, let's change our repository `destroy` methods
to behave in the same way.
  • Loading branch information
lfdebrux committed Feb 13, 2025
1 parent 17ac11d commit ec550fd
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 3 deletions.
9 changes: 8 additions & 1 deletion app/services/condition_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,14 @@ 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
Expand Down
9 changes: 8 additions & 1 deletion app/services/form_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,14 @@ 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

Expand Down
9 changes: 8 additions & 1 deletion app/services/page_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,14 @@ 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

Expand Down
26 changes: 26 additions & 0 deletions spec/services/condition_repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,31 @@
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
26 changes: 26 additions & 0 deletions spec/services/form_repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,32 @@
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
Expand Down
26 changes: 26 additions & 0 deletions spec/services/page_repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,32 @@
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
Expand Down

0 comments on commit ec550fd

Please sign in to comment.