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

Model.find being called twice with acts_as_list on destroy #161

Closed
npearson72 opened this issue Apr 30, 2015 · 5 comments
Closed

Model.find being called twice with acts_as_list on destroy #161

npearson72 opened this issue Apr 30, 2015 · 5 comments

Comments

@npearson72
Copy link

I have a Category model that uses acts_as_list.

In my controller, I'm calling Category.find(params[:id])

When I try to run my tests, for some reason .find is called twice.

When I remove the acts_as_list method from my Category model, it works fine.

For more context, please refer to my rspec mocks issue:

rspec/rspec-mocks#939

Any ideas?

@npearson72
Copy link
Author

Seems this is being caused in:

lib/acts_as_list/active_record/acts/list.rb

Line 477

#reload_position is calling #reload

Does this need to call find? Seems odd.

@npearson72 npearson72 changed the title Mode.find being called twice with acts_as_list Model.find being called twice with acts_as_list Apr 30, 2015
@npearson72 npearson72 changed the title Model.find being called twice with acts_as_list Model.find being called twice with acts_as_list on destroy Apr 30, 2015
@swanandp
Copy link
Collaborator

swanandp commented May 1, 2015

@npearson72 ActiveRecord::Base#reload will call find internally.

As a general advice, I'd recommend changing your test slightly to assert that find is called at least once, instead of exactly once. Otherwise you are testing implementation, and not behaviour.

@npearson72
Copy link
Author

Ok, thanks. But are you saying this isn't an issue?

@Albin-Willman
Copy link
Contributor

This is not an issue, it is necessary in order to be certain that the positions are correct after the destroy is done.

@brendon
Copy link
Owner

brendon commented Apr 17, 2016

That's correct. We now use lock! instead but the outcome is the same.

@brendon brendon closed this as completed Apr 17, 2016
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

No branches or pull requests

4 participants