brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

ActiveRecord associations are no longer required (even though belongs_to_required_by_default == true) #268

Closed mattscilipoti closed 7 years ago

mattscilipoti commented 7 years ago

I'm using Rails 5.0.2, belongs_to_required_by_default = true. I have tests that verify my important associations are required.

After upgrading from 0.8.2 to 0.9.5, the specs in my app that ensure associations are required began to fail. I identified that it happened during the transition to 0.9.0. I attempted to test this within acts_as_list, but was not successful. Instead, I referenced act_as_list via local "path:" in my Gemfile and used git bisect on the cloned acts_as_list dir while testing via my app. This led me to: [4abf1371b243dbd3d924b74fc073e7da0952edbd] Improve load method (#240) and PR #240 by @brendon. There is a lot going on in there. I'm hoping that @brendon may have an idea.

I had a similar experience with simple_token_authentication: https://github.com/gonzalo-bulnes/simple_token_authentication/issues/297. The workaround I found for them does not work for acts_as_list.

I hope we can use what we learn to help each other.

brendon commented 7 years ago

Hi @mattscilipoti, the diff's in that PR are messed up. There's actually very little that changed apart from where things are required from and how the acts_as_list method is added to AR::Base.

It might pay to try another diffing application to see if that provides more clarity on the change that caused the problem. I use Diffmerge on the mac but there are other good ones too.

What we really need (and you allude to this) is a failing test in acts_as_list in isolation from your app, otherwise it's hard to say either way where the problem lies. I think it's likely to be a load order thing. This PR was in response to an issue about that but I'm unable to find the issue and we never linked the two together unfortunately.

In the meantime, could you have a go at trying to replicate this with a failing test in our repo?

mattscilipoti commented 7 years ago

Thanks! I'll try some other diff viewers. I tried to replicate the problem with a fresh Rails app: https://github.com/mattscilipoti/test_belongs_to_required_by_default. No luck so far. I'll try some various appraisal configs next.

brendon commented 7 years ago

That's interesting. Take a look at your initialisers. Because we changed the order of loading (I think it's earlier now) that could be causing grief with something that didn't expect it to be loaded yet?

mattscilipoti commented 7 years ago

I've tried a few things:

In the test project:

In the "failing" project:

I even tried both minitests and rspec tests (just in case it was dependent on their load paths).

This is quite strange. I do not understand why the test project is responding to acts_as_list differently than the "failing" project.

brendon commented 7 years ago

Yes that is very strange. I suspect something non-standard in the initialisation routine in you project.

If it's been through a few rails versions and you haven't done the following, I'd suggest giving it a go:

I usually will generate a blank rails project with all the settings (mysql as db engine) etc... set as flags on the rails command so you get everything set up as it should be (skip the bundle install). I then use diffmerge to compare it to my project and fix the differences. The bin and config directories will be the main culprits I'd say.

The other option is perhaps there's a monkey-patch somewhere in your project hiding away that is causing problems?