0

Have a problem regarding saving ActiveRecord associations and need your help :)

I need to add articles merging functionality to legacy code.

It's expected to work the following way:

  1. Merge "source" article's text to the "target" article.
  2. Check for "source"'s comments and, if any, re-associate them to the "target".
  3. Destroy the "source" article. Comments should be preserved and associated with "target".

Here's my Article model code (reduced for readability).

class Article < Content

  before_destroy :reload_associated_comments

  has_many :comments,   :dependent => :destroy, :order => "created_at ASC" do

  def reload_associated_comments
    unless self.comments.empty?
      article = Article.find(@merge_with)
      self.comments.each do |comment|
        comment.article = article
        article.save!
      end
    end
  end

  def merge_with(id)
    @merge_with = id
    article = Article.find(@merge_with)
    if !article.nil?
      text = article.body + body
      article.body = text
      article.save!
      self.destroy
      return article
    end
    nil
  end
end

Here's Comment model (also reduced):

class Comment < Feedback
  belongs_to :article
end

The problem is when I return from before_destroy hook nothing has been saved to the database. I check it via the following:

eval Article.find(target_article_id).comments

Save raises no exceptions. What I'm missing here?

Thanks in advance!

This worked for me

  def merge_with(id)
    @merge_with = id
    article = Article.find(@merge_with)
    unless article.nil?
      text = article.body + body
      article.body = text
      article.save!
      reload_associated_comments
      self.reload
      self.destroy
      return article
    end
    nil
  end
niebelung
  • 127
  • 8
  • In that method reload_associated_comments, it is supposed to be comment.save! instead of article.save! I think :) – Zippie Mar 13 '13 at 22:50
  • also i am not sure but i think you can't call self.destroy inside the method that is called on self. Return the article and then destroy it from the outside (where you call the `merge_with` method) – Zippie Mar 13 '13 at 22:51
  • The `reload_associated_comments` method is used as a `before_destroy` callback. If I call article.save!, doesn't it mean that comments are saved automatically? If doesn't, what is the correct order for saving. Comment and then article or vice versa? – niebelung Mar 13 '13 at 22:55
  • 1. for the second comment -> my mistake, when you call destroy, it will destroy it from the database and it wont destroy the object itself. 2. you can just do the comment.save cause it is saving the id of the article inside the comment. I think its not valid to do it with article.save since you are not modifying the article. Dear gosh i hope i am telling you the right things :) – Zippie Mar 13 '13 at 22:58
  • Good point. I'll try to call `destroy` from outside. Though it's not known to bother an `self`object itself. – niebelung Mar 13 '13 at 22:59
  • STOP! just try to remove the article.save and make it comment.save Have you tried with that? – Zippie Mar 13 '13 at 23:00
  • No(( I've tried and it still doesn't save comments into database. – niebelung Mar 13 '13 at 23:27
  • did you read the comment that alfonso posted? Why don't just call the method from the merge_with method and that's it? :) – Zippie Mar 13 '13 at 23:28
  • I'm reading the post right now) – niebelung Mar 13 '13 at 23:36
  • great, tell us if it worked :) – Zippie Mar 13 '13 at 23:37

1 Answers1

1

Actually Rails destroy every comment before calling the before_destroy callback for the article. That's sadly how rails work. Changing this behavior would imply breaking legacy apps. You can find more information on this issue here: https://github.com/rails/rails/issues/670

In my opinion the best solution is to override the destroy method and get rid of the callback:

class Article < Content

  def destroy
    reload_associated_comments
    super
  end

  has_many :comments,   :dependent => :destroy, :order => "created_at ASC" do

  def reload_associated_comments
    unless self.comments.empty?
      article = Article.find(@merge_with)
      self.comments.each do |comment|
        comment.article = article
        comment.save!
      end
    end
  end

  def merge_with(id)
    @merge_with = id
    article = Article.find(@merge_with)
    if !article.nil?
      text = article.body + body
      article.body = text
      article.save!
      self.destroy
      return article
    end
    nil
  end
end

Also, as @Zippie mentioned, you should call comment.save! instead of article.save!

Hope it helps!

alf
  • 18,372
  • 10
  • 61
  • 92
  • 1
    wow, what a answer! alfonso, wouldn't you say that the `article.save!` in the `reload_associated_comments` method isn't right? Shouldn't it be `comment.save`! And if it is right, how is it right? When we put a `.save!` on an object it effects all of its children? – Zippie Mar 13 '13 at 23:21
  • i am really glad, cause if i wasn't, ActiveRecord would freak me out! :) – Zippie Mar 13 '13 at 23:25
  • Really cool! Thanks a lot to both of you, guys! Frankly, callback isn't the best solution here. Used it just to play with callbacks)) – niebelung Mar 13 '13 at 23:40
  • Unfortunately, found out that it doesn't work even with solution provided by @alfonso. But it worked fine with `self.reload` and finally I managed to save associated comments to the target article. However, thanks again, guys! You helped me much! – niebelung Mar 14 '13 at 19:20