Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tagged with rewrite #829

Merged
merged 10 commits into from
May 16, 2017
Merged

Tagged with rewrite #829

merged 10 commits into from
May 16, 2017

Conversation

rbritom
Copy link
Contributor

@rbritom rbritom commented May 15, 2017

Rewrite to have one SQL query for the array of tags and added missing test.

context 'retry 3 times on not unique exception' do
it 'performs 3 tries before raising the exception' do
allow(ActsAsTaggableOn::Tag).to receive(:named_any).and_raise(ActiveRecord::RecordNotUnique.new('error')) # trigger error inside block
expect(ActiveRecord::Base.connection).to receive(:execute).with('ROLLBACK').exactly(3).times
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expect(ActiveRecord::Base.connection).to receive(:execute).with('ROLLBACK').exactly(4).times

Datacleaner will rollback too.

Without datacleaner this test will rollback only 3 time.

@@ -170,7 +170,7 @@

context 'retry 3 times on not unique exception' do
it 'performs 3 tries before raising the exception' do
allow(ActsAsTaggableOn::Tag).to receive(:named_any).and_raise(ActiveRecord::RecordNotUnique) # trigger error inside block
Copy link
Collaborator

@seuros seuros May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still apply for AR 4.2+ ?

@@ -171,7 +171,7 @@
context 'retry 3 times on not unique exception' do
it 'performs 3 tries before raising the exception' do
allow(ActsAsTaggableOn::Tag).to receive(:named_any).and_raise(ActiveRecord::RecordNotUnique.new('error')) # trigger error inside block
expect(ActiveRecord::Base.connection).to receive(:execute).with('ROLLBACK').exactly(3).times
expect(ActiveRecord::Base.connection).to receive(:execute).with('ROLLBACK').exactly(4).times # one extra rollback from database cleaner
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let change the Datacleaner strategy to :deletion and correct the test
https://github.com/mbleigh/acts-as-taggable-on/blob/master/spec/support/database_cleaner.rb#L5

@seuros seuros merged commit 64ed839 into mbleigh:master May 16, 2017
@seuros
Copy link
Collaborator

seuros commented May 16, 2017

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants