Soft-delete on has_many through association

[Dmytrii Nagirniak]

Hi,

What is the easiest way to implement soft-deletes on has_many through association?

What I want is something like this:

class Company > ActiveRecord::Base
  has_many :staffings
  has_many :users, through: :staffings, conditions: {staffings: {active: true}}
end

I want to use Company#users the following way:

• the Company#users should be a normal association so that it works with forms.
• when adding a new user, a new Staffing with active: true is created.
• when removing a user, the existing Staffing is updated active: false (currently it just get deleted).
• when adding a previously removed user (so that Staffing#active == false) the Staffing is updated to active: true.

I thought about overriding the Company#users= method, but it really isn't good enough since there are other ways of updating the associations.

What is the Rails Way of doing this sort of thing?

(I asked it at SO too - http://stackoverflow.com/questions/10441633/soft-delete-on-has-many-through-association)

What I came up with so far is:

- override user= and user_ids

- deny the use of any other association modifier methods:

But all that feels really, really dirty.

has_many :developments, through: :development_participations,

          conditions: {development_participations: {allowed: true}} do |a|

            %w{<< delete clear build create}.each do |association_method|

              # Disable assoc modifier methods for now

              # http://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html#method-i-has_many

              define_method(association_method) do |*args|

                raise 'nah, not yet supported. Use `developments=` instead.'

              end

            end

          end

def users=(others)

  self.development_ids = others.collect &:id

end

def development_ids=(other_ids)

 # All the merging magic here  

end

Any thought?

Cheers,

Dmytrii

http://ApproachE.com

[Chris Herring]

I would just use something likes acts_as_paranoid and then then have the dependent destroy conditions on your associations.

--
You received this message because you are subscribed to the Google Groups "Ruby or Rails Oceania" group.
To post to this group, send email to rails-...@googlegroups.com.
To unsubscribe from this group, send email to rails-oceani...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rails-oceania?hl=en.

[Craig Read]

I didn't know about acts_as_paranoid.

I was thinking a callback on before_destroy.

def before_destroy
  update_attribute(:active, false)
  return false
end

--
Craig Read

@Catharz

https://github.com/Catharz

http://stackoverflow.com/users/158893/catharz

[Pat Allan]

I'd be extremely wary of modifying the behaviour of ActiveRecord's own methods. I'd recommend creating your own custom destroy method which behaves how you want, and make sure you're using that wherever appropriate. Hence, it's been a long time since I've used acts_as_paranoid.

Not quite sure how that would fit into an association setup, though. I'm sure it'd be possible to code something together.

--
Pat

[Dmytrii Nagirniak]

Yeah, modifying the AR methods is way too dirty.

The problem here is to preserve the initial interface/contract on Company#users and add Company#staffings on top of it.

So that:

company.users << user1

company.save!

company.should have(1).staffing

company.users = []

company.save!

company.should have(1).staffing

company.should have(0)users

I'm not sure if the before_destroy callback will do the job. Need to play a bit with.

Also need to play with acts_as_paranoid.

It seems to be non-maintained anymore.

Probably something from https://www.ruby-toolbox.com/categories/Active_Record_Soft_Delete

would be a better choice.

Anyway, I'll play with all that.

Cheers.

[Craig Read]

I'm not saying to modify the behaviour of before_destroy, but to use it.

Either by:

class User < ActiveRecord::Base
  before_destroy :deactivate_user

protected
  def deactivate_user

   update_attribute(:active, false)
   return false
  end

end
 
or

class User < ActiveRecord::Base

protected

  def before_destroy
   update_attribute(:active, false)
   return false
  end

end

According to the rails 3 way, both do methods do exactly the same thing.

Cheers,

Craig.

[Ben Hoskings]

You're better off making a second set of tables, like deleted_companies and deleted_staffings, inserting rows into them in a destroy callback on the main models.

You could use is_paranoid (which superceded acts_as_paranoid), but it's a bit of a nightmare. Having used it extensively before I can tell you that from experience. By design, it doesn't handle associations properly [1], and is quite invasive in that it adds a "AND deleted_at IS NULL OR deleted_at < now()" to every select query you run on those tables. I found that it complicated things quite a bit.

Creating extra tables doesn't violate DRY if you organise your code well, and is a more correct representation of the data: deleted records are presumably there for archival purposes, and don't form part of the working set, so they shouldn't sit alongside active data.

[1] When deleting a record, it's not clear what to do with associated ones:

- If you don't mark them as deleted too, you'd have to join the parent table to exclude records with a deleted parent;
- If you do, then when it's time to undelete, you can't tell if the child record was actually deleted or if it was just marked that way when the parent was deleted.

The whole thing is a big messy concern that should be separated, table-wise.

- Ben

[Dmytrii Nagirniak]

On 04/05/2012, at 2:54 PM, Ben Hoskings wrote:

> You're better off making a second set of tables, like deleted_companies and deleted_staffings, inserting rows into them in a destroy callback on the main models.

There's already such a table - one that resolves many-to-many association (staffings).
I thought about using PG array to store the IDs or another table. But I don't feel those are right places.

> You could use is_paranoid (which superceded acts_as_paranoid), but it's a bit of a nightmare.

Thanks. I see. Feels like much more than I really need.

> Creating extra tables doesn't violate DRY if you organise your code well, and is a more correct representation of the data: deleted records are presumably there for archival purposes, and don't form part of the working set, so they shouldn't sit alongside active data.

The thing is that the records are no deleted ("soft deletion" is used as a concept here) , those are "inactive" and represent the status of the user within the company.
Thus the many-to-many resolving table feels like the right place for it.

I only want to preserve the existing interface where there was no such a notion before. (has_and_belongs_to_many => has_many through).
This is really easy with the condition:

has_many :users, through: :staffings, conditions: {staffings: {active: true}}

but unfortunately I don't know how to set "active = false" instead of deleting the Staffing record.

Staffing#before_destroy doesn't seem to work when clearing association `company.users = []`

[Ben Hoskings]

On 04/05/2012, at 3:16 PM, Dmytrii Nagirniak wrote:

> The thing is that the records are no deleted ("soft deletion" is used as a concept here) , those are "inactive" and represent the status of the user within the company.

Ahh, right. I was responding to a slightly different question then :)

Yep I agree, the join table sounds like the right place for it. I'd just not use deletion; how about defining a Staffing#deactivate method/action?

I'd say it's a bad idea to override destroy to do something else like deactivation -- it's surprising for a #destroy action to not actually remove the resource.

- Ben

[Dmytrii Nagirniak]

On 04/05/2012, at 3:20 PM, Ben Hoskings wrote:
>
> Ahh, right. I was responding to a slightly different question then :)

Sorry, I should have said it at the very beginning :)

> Yep I agree, the join table sounds like the right place for it. I'd just not use deletion; how about defining a Staffing#deactivate method/action?

That totally makes sense.

The only reason for all this dance is to preserve the existing interface where Company#users association was used as "active users" only.
So that all the forms and other code that relies on it still works.

I got the before_destroy to work (forgot to set the :dependent => :destroy).

But now there's another problem.

1) company.users << first; company.save!
2) company.users.clear; company.save!
---> the record is "deactivated" as expected
3) company.users << first; company.save!
---> additional record gets inserted, so there is one active and one inactive (violating constraint).

> I'd say it's a bad idea to override destroy to do something else like deactivation -- it's surprising for a #destroy action to not actually remove the resource.

Isn't Model#destroy supposed to be used the same way as Model#save ?
It returns truthy/falsy indicating the result.

[Craig Read]

You might need to use before_create to check that the row doesn't already exist (in a deactivated state).

--
You received this message because you are subscribed to the Google Groups "Ruby or Rails Oceania" group.
To post to this group, send email to rails-...@googlegroups.com.
To unsubscribe from this group, send email to rails-oceani...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rails-oceania?hl=en.

[Craig Read]

One of the gotchyas with using the callback methods, is if you return false (not nil) from them, the following will happen:

• Active Record will halt the execution chain, and no further callbacks will be called.
  
• save will return false and save! will raise a RecordNotSaved error
  

So you need to make sure you aren't returning false by mistake.
I'm not sure if AR will delete the row if you do return anything other than false though.

[Dmytrii Nagirniak]

On 04/05/2012, at 3:43 PM, Craig Read wrote:

You might need to use before_create to check that the row doesn't already exist (in a deactivated state).

That was my first thought too:

before_create do

  others = Staffing.where(:company_id => company_id, user_id: user_id)

  return false if others.any?

end

but then I'm getting RecordNotSaved exception every time trying to re-add an object (which is correct).

I can't see a way of handling it transparently and marry the callbacks.

[Craig Read]

Hmm, not sure how to get around that.

For a moment, I was thinking you could update the active attribute on before_create (similar to the way before_delete works), and then return true.
But I'm pretty sure before_create only works on a new record, and would be called in the scope of the "new" User, not the "original" deactivated User.

I'm sure there must be a clean way of handling it, but I can't think of it atm.
Too close to beer'o'clock. ;)

[Gregory McIntyre]

Given the pain I've seen in Rails projects that use default scopes and
non-POLS AR helpers, I'd go for changing the current API to make it
all very explicit.

Still, if you write a spec for it and invite people to do it as a code
kata / challenge, it might be fun.

-Greg