Skip to content

Commit

Permalink
Wrap destroy and revive in a transaction
Browse files Browse the repository at this point in the history
Everything incl. changes to dependent records should be rolled back
when an error occurred.

This also adds some missing tests for dependent destroy behavior on
belongs_to associations.

Closes #45.
  • Loading branch information
mherold committed Mar 6, 2015
1 parent 7e6341c commit d65788d
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 6 deletions.
15 changes: 10 additions & 5 deletions lib/permanent_records.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,20 @@ def deleted?
end

def revive(validate = nil)
run_callbacks(:revive) { set_deleted_at(nil, validate) }
self
with_transaction_returning_status do
run_callbacks(:revive) { set_deleted_at(nil, validate) }
self
end
end

def destroy(force = nil)
if !is_permanent? || PermanentRecords.should_force_destroy?(force)
return permanently_delete_records_after { super() }
with_transaction_returning_status do
if !is_permanent? || PermanentRecords.should_force_destroy?(force)
permanently_delete_records_after { super() }
else
destroy_with_permanent_records force
end
end
destroy_with_permanent_records force
end

private
Expand Down
87 changes: 86 additions & 1 deletion spec/permanent_records_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
end
end


context 'when model has no deleted_at column' do
let(:record) { kitty }

Expand All @@ -100,29 +99,85 @@
it 'marks records as deleted' do
subject.muskrats.each {|m| m.should be_deleted }
end

context 'when error occurs' do
before { Hole.any_instance.stub(:valid?).and_return(false) }
it 'does not mark records as deleted' do
expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
expect(record.muskrats.not_deleted.count).to eq(1)
end
end

context 'with force delete' do
let(:should_force) { :force }
it('') { expect { subject }.to change { Muskrat.count }.by(-1) }
it('') { expect { subject }.to change { Comment.count }.by(-2) }

context 'when error occurs' do
before { Difficulty.any_instance.stub(:destroy).and_return(false) }
it('') { expect { subject }.not_to change { Muskrat.count } }
it('') { expect { subject }.not_to change { Comment.count } }
end
end
end

context 'with has_one cardinality' do
it 'marks records as deleted' do
subject.location.should be_deleted
end

context 'when error occurs' do
before { Hole.any_instance.stub(:valid?).and_return(false) }
it('does not mark records as deleted') do
expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
expect(record.location(true)).not_to be_deleted
end
end

context 'with force delete' do
let(:should_force) { :force }
it('') { expect { subject }.to change { Muskrat.count }.by(-1) }
it('') { expect { subject }.to change { Location.count }.by(-1) }

context 'when error occurs' do
before { Difficulty.any_instance.stub(:destroy).and_return(false) }
it('') { expect { subject }.not_to change { Muskrat.count } }
it('') { expect { subject }.not_to change { Location.count } }
end
end
end

context 'with belongs_to cardinality' do
it 'marks records as deleted' do
subject.dirt.should be_deleted
end

context 'when error occurs' do
before { Hole.any_instance.stub(:valid?).and_return(false) }
it 'does not mark records as deleted' do
expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
expect(record.dirt(true)).not_to be_deleted
end
end

context 'with force delete' do
let(:should_force) { :force }
it('') { expect { subject }.to change { Dirt.count }.by(-1) }

context 'when error occurs' do
before { Difficulty.any_instance.stub(:destroy).and_return(false) }
it('') { expect { subject }.not_to change { Dirt.count } }
end
end
end
end

context 'that are non-permanent' do
it 'removes them' do
expect { subject }.to change { Mole.count }.by(-1)
end
end

context 'as default scope' do
let(:load_comments) { Comment.unscoped.where(:hole_id => subject.id) }
context 'with :has_many cardinality' do
Expand Down Expand Up @@ -205,19 +260,49 @@
it 'revives them' do
subject.muskrats.each {|m| m.should_not be_deleted }
end
context 'when error occurs' do
before { Hole.any_instance.stub(:valid?).and_return(false) }
it 'does not revive them' do
expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
expect(record.muskrats.deleted.count).to eq(1)
end
end
end

context 'with has_one cardinality' do
it 'revives them' do
subject.location.should_not be_deleted
end
context 'when error occurs' do
before { Hole.any_instance.stub(:valid?).and_return(false) }
it('does not mark records as deleted') do
expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
expect(record.location(true)).to be_deleted
end
end
end

context 'with belongs_to cardinality' do
it 'revives them' do
subject.dirt.should_not be_deleted
end

context 'when error occurs' do
before { Hole.any_instance.stub(:valid?).and_return(false) }
it 'does not revive them' do
expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
expect(record.dirt(true)).to be_deleted
end
end
end
end

context 'that are non-permanent' do
it 'cannot revive them' do
expect { subject }.to_not change { Mole.count }
end
end

context 'as default scope' do
context 'with :has_many cardinality' do
its('comments.size') { should == 2 }
Expand Down

0 comments on commit d65788d

Please sign in to comment.