Adds a way to prevent the default_scope when desired

[hannesfostie]

Sometimes setting the default_scope is not desired. This PR will allow you to add an option to acts_as_paranoid to never set the default_scope in a model.

[hannesfostie]

This branch is kind of a proof of concept with a very minimal test. I'm going to use this version in our project to see what happens and if everything behaves the way I expect it to.

It's also a WIP as I figure out what's required to make this behave properly. See my last WIP commit. Do you prefer not to have this as a PR already? Should I make an issue instead while I work on it?

To give some background:

We currently use a lib we wrote ourselves but decided that reinventing the wheel is probably a bit over the top. I started by ripping out our implementation, added paranoia, and started fixing tests. One of our associations (a has_many through with inverse_of) was giving us problems, in the sense that deleted_blog_post_comment.blog was nil, whereas deleted_blog_post_comment.blog_post.blog was not.

I could have changed all these but then realized we actually don't have a default_scope right now, so instead of carefully adding with_deleted where needed I decided to give this a go. We generally don't remove deleted objects from our queries, so it made no sense to have that default scope in there in the first place.

I hope that makes sense. Happy to listen to suggestions while I see if this works for us!

[radar]

I have considered making the default_scope OFF by default, so you have to be explicit about finding only deleted records. In my opinion, that would be better.

But then associations break. I spent hours looking at Active Record code (ew) to figure out how to get around this and I couldn't figure it out. So I gave up.

I will only merge this PR if others are interested in this feature. I don't want to go adding features that nobody else is interested in. This is mainly because you're adding this feature for yourself, and then if I merged it I would be responsible for maintaining more code; not something that I want to do.

[hannesfostie]

@radar I understand, and I appreciate you taking the time to have a look.

How can we figure out if more people could use this?

Do you have some old code where you turned the default scope off by default? I'd be happy to give it a go as well. If not, do you remember vaguely what you did and what broke? I don't immediately understand what you describe as the problem.

[jhawthorn]

I'm quite interested in this, it would be nice to have the means to selectively disable parts of paranoia's functionality. Could be very helpful when migrating away from it to tracking soft deletions manually.

I'll try to take a look later this week to see what would need to be done.

[radar]

Here is the code that I was helping with to disabling paranoia by default: https://github.com/oshiho3/paranoia/tree/remove_default_scope

It caused issues around the associations and I couldn't think of a good way to fix it. I believe I got stuck on this test:

def test_default_scope_for_has_many_through_relationships

You're welcome to adapt that code for your needs. Good luck!

[hannesfostie]

@radar after reading the test, here's what I found:

The breaking line is #303:

assert_equal 0, employee.employers.paranoia.count

I believe this test fails because the classes have an association that doesn't really make sense. I changed the associations as such:

class Employer < ActiveRecord::Base
  acts_as_paranoid
  has_many :jobs
  has_many :current_jobs, -> { where :deleted_at => nil }, :class_name => 'Job'
  has_many :employees, :through => :current_jobs
end

class Employee < ActiveRecord::Base
  acts_as_paranoid
  has_many :jobs
  has_many :current_jobs, -> { where :deleted_at => nil }, :class_name => 'Job'
  has_many :employers, :through => :current_jobs
end

The tests weren't all fixed right away of course, because they reflect the current/old way of thinking about these associations.

That said, I wonder if we aren't shooting ourselves in the foot by using this real world example (jobs, employers, employees). The way I "fixed" the associations makes complete sense because when a job is no longer around, you don't have an employer. You just have past a employer.

I need to think about this again in another context to see if we can make this work. Happy to hear your thoughts as well!

[hannesfostie]

Thought about it some more this morning with the Patient <-> Physician through Appointment example from the RoR guides, as well as a (Lego)Castle <-> Block through Component example and no matter how I try to think about it, I still seem to come to the same conclusion:

You need to either use #merge in your query to only select the non-deleted objects of your through table, or you have to add a second association where this scope is active. It's something you just need to be aware of, but you should know your domain logic anyway so I'm not sure if that is a problem.

[hannesfostie]

@radar I'd love your input on my previous comment. How do you feel about it?

[aganov]

+1

[tagrudev]

+1

[rbr]

Is this equal to what #301 added?

[hannesfostie]

It looks like it!

Sadly I am no longer using paranoia in any of my projects, and don't have the bandwidth to properly follow up with this and get things up to date. I think this PR can be closed in favor of the other one.