Skip to content

Add support for before and after add/remove callbacks to belongs_to associations #586

Closed
lighthouse-import opened this Issue May 16, 2011 · 24 comments
@lighthouse-import

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/1564
Created by sob - 2011-02-14 09:36:21 UTC

belongs_to currently does not support before/after_add/remove callbacks as has_many does. There are a variety of applications in which this is beneficial. I found an original ticket from a few years ago but it only implemented the before/after_add callbacks and did not apply cleanly any more.

I've reworked the existing patch from the link below to support all necessary callbacks including before_add, before_remove, after_add, and after_remove. More information on the original request can be found on the ticket here:

http://dev.rubyonrails.org/ticket/6934

Thanks!

  • sob
@lighthouse-import

Imported from Lighthouse.
Comment by Trek - 2010-04-13 22:55:14 UTC

+1 for this (although I imagine it won't apply cleanly anymore).

@lighthouse-import

Imported from Lighthouse.
Comment by joel (at developwithstyle) - 2010-05-04 11:17:38 UTC

+1. Surprised it's not already in!

@lighthouse-import

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-05-04 17:48:37 UTC

[bulk edit]

@lighthouse-import

Attachments saved to Gist: http://gist.github.com/971611

@josevalim
Ruby on Rails member

-1. This is quite easy to manage with the dirty functionality.

@josevalim josevalim closed this May 16, 2011
@jmacdonald

How is this managed with "dirty functionality"? I have an excellent use case for this feature; can you please explain how this functionality can be implemented otherwise?

@jmacdonald

An example might help explain why this would be useful.

I have three classes modelled using single-table inheritance. Employee is the base class, which the Manager and Worker classes extend. A manager has_many workers, and a worker belongs_to a manager. If a new worker is added and its manager_id field references a worker, that worker should be automatically changed to a manager and have the worker added as a child.

Ideally, you'd bind to a before_add event on the has_many association in the Manager class, since the conversion from a worker to manager object has nothing to do with the new worker object. Unfortunately, the relationship doesn't exist on the soon-to-be manager just yet, so the next best thing would be to add it to the before_add on the worker's belongs_to association, but as this issue indicates, this is not possible.

You'd think you could then put it on the before_save call, but that won't work because the validations will fail as the manager_id points to an as-of-yet unconverted worker object. Putting it on before_validation would work, but if someone called .valid? on the new worker object, it would convert the worker object referenced by manager_id, and that's not right. If the object wasn't saved after that, the validation would have made invalid changes to the database.

I'm currently resorting to an add_worker method on the Worker class which does the conversion and then creates the association. It's unfortunate, mostly because I have to litter my controller's update and create actions to check for a manager_id, strip it out, create/update the object, and then call this add_worker method afterwards.

I can't think of a better way to model this problem, but if you have any suggestions that are more appropriate than using the functionality presented in this issue, I'm all ears.

Thanks!

@josevalim josevalim reopened this Jan 25, 2012
@josevalim
Ruby on Rails member

Reopening for discussion. /cc @jonleighton

@jonleighton
Ruby on Rails member
class Post < ActiveRecord::Base
  belongs_to :author

  def author=(a)
    super
    author_assigned
  end

  def author_assigned
    # do some stuff
  end
end

Does this provide a solution?

@jmacdonald

Unfortunately not, although it's a more elegant solution than putting it on the validation.

Following along with my example, if we override the association attribute writer method for the Employee class (like you've done on the Post class above), the type conversion (from Employee to Manager) on the associated object has to occur before the save, or we'll get an AssociationTypeMismatch exception when we save the employee because an employee can only belong to a manager.

If we do the type conversion as part of the attribute writer method, we have to save it to avoid the aforementioned exception, and if the attribute is updated but never persisted to the database, we could wind up with a recently-converted manager that has no employees.

The beauty of doing this in a before_add association callback is that we can make (and save) the type conversion of an employee to a manager in a situation where we are absolutely certain that the relationship that's triggering it is about to be established and saved. These two operations really need to be part of a transaction since they're useless individually.

From a semantic perspective, I believe the before_add callback is the most appropriate place to put this code. As I've mentioned before, the ideal place would be in a before_add on the Manager class' has_many relationship, since this action really relates to house keeping on the associated object, and not on the object we're saving. However, since the Employee object hasn't been converted to a Manager yet, this isn't possible; it has to be done on the Employee class.

I hope that helps clarify my use case. It may not be common, but I still think it's the most elegant approach, and I'm sure there are other situations in which it would be useful.

@jmacdonald

If I implement this functionality, provided that it's properly tested, will the pull request be accepted? I'd to help in whatever way I can to get this capability added, but I don't want to do the work unless it's going to be used.

@rafaelfranca
Ruby on Rails member

@jonleighton please see the @jmacdonald proposition. Can we close this issue or should he work to fix it?

@jonleighton jonleighton was assigned Apr 27, 2012
@irongaze

+1 on this issue - for consistency's sake if nothing else. Just refactored my code to remove a bunch of monkey-patching using :before_add on my associations. has_many worked a charm, cleaned things up immensely. has_one blew up with an invalid key on the association and I found my way here. Use case, for the discussion:

All models can have files associated with them. Originally I wrote this with a simple has_many relationship, but found that I kept needing to segment the files to allow grouping of different kinds of files, eg a Hospital could has_many :scanned_pdfs, but also want to has_one :logo. The elegant solution here was to create a "group" parameter on my file models, then set it appropriately in the has_many and has_one association conditions. Now eg @hospital.logo.build works great, but if I create the file elsewhere and associate it with the hospital, no attribute update is performed and the group is not set correctly. Having :before_add work on has_one relationships, identical to has_many, would solve this problem in the most correct possible manner, IMHO.

@kuraga
kuraga commented May 21, 2012

+1.
Let's unify these features!

@mjrk
mjrk commented Feb 2, 2013

+1.
has_one should have the callback as has_many has it.

@misza222

+1

@rafaelfranca
Ruby on Rails member

This feature request is stale from a long time so I'm closing this one since we want to use the issues tracker to track issues and pull request.

I suggest to open a pull request or if you want to discuss more send an email to the Rails Core mailing list.

Thank you

@mmlac-bv

It is very much still an issue that should be dealt with. It's not the child's job to inform the parent that a parent's value has changed. (Even though it may not have changed in the mongoDB entry itself, but it changed the value of the resulting Mongoid-object)

Child.create(parent: Parent.last) will not trigger anything in the Parent class if I recall correctly. This functionality is implemented for has_many and I see no reason this should not be implemented for has_one. I agree that it has no place on the belongs_to side -> rename Issue?

@DrMavenRebe

+1

If anyone has a good monkey patch in the mean time, I'd love to see it.

@dv
dv commented Nov 12, 2013

+1 this too.

(edit removed code that obviously wouldn't work for the use case that's most interesting)

@Javierchik

guys you can use inverse_of on each of the has_one and belongs_to associations....

@D1plo1d
D1plo1d commented Mar 11, 2014

@Javierchik except in the case of belongs_to :through and has_one :through.

Callbacks could be used as a work around the problematic lack of :inverse_of when :through is present.

@bbugh
bbugh commented Sep 16, 2014

+1

This just came up for me on a client project, and I was surprised that Rails has callbacks for everything except 1:1 associations. The hacky way that jonleighton described is what we ended up going with. It's an acceptable solution, but it is weird that it's inconsistent with callbacks. Rails can't try to be everything to everyone or it would be Java.

@jolim
jolim commented Oct 3, 2014

+1

With the jonleighton method, don't forget that you may need to handle the assignment by ID when doing it through a controller in response to a form submission.

def author_id=(a_id)
    if a_id.empty?
        self.author = nil
    else
        self.author=Author.find(a_id)
    end
end

I invite any edits to improve this hack.

@rafaelfranca rafaelfranca locked and limited conversation to collaborators Oct 3, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Something went wrong with that request. Please try again.