Delayed Job - job cannot be created for non-persisted record after destroy object

Rails 4 and delayed_job 4.1.2. I attempt to delay recalculation of overall rating after destroying a Review, but obviously because after destroying a review object there is no ID for the Review object. So every time after trying to destroy an object, it tries to create a delayed job but throws this error:

ArgumentError (job cannot be created for non-persisted record: 
#<Review id: 44, review: "Bad", rating: 1, reviewable_id: 2, 
reviewable_type: "Spot", user_id: 1, created_at: "2016-05-30 17:13:29", 
updated_at: "2016-05-30 17:13:29">):
  app/controllers/reviews_controller.rb:40:in `destroy'

I have the following code:

# reviews_controller.rb
class ReviewsController < ApplicationController
  def destroy
    review.destroy
    flash[:success] = t("reviews.destroy.success")
  end
end

# review.rb
class Review < ActiveRecord::Base
  after_destroy :calculate_overall_rating

  def calculate_overall_rating
    if number_of_reviews > 0
      reviewable.update_attribute(:overall_rating, overall_rating)
    else
      reviewable.update_attribute(:overall_rating, 0)
    end
  end
  handle_asynchronously :calculate_overall_rating
end

It's good to note that the calculate_overall_rating doesn't require a Review object.

If I remove handle_asynchronously :calculate_overall_rating it will work, and recalculate. But I am trying to delay this job.

Solution

This error is indeed raised by delayed_job when you try to delay a method on a deleted (or not yet created) record. The direct cause of this error is because delayed_job passes self (i.e. the just-deleted review object) as the target object when calling thehandle_asynchronously method. I don't know why it behaves like that, I just found a statement from one of the gem's authors that says it works the same way as ActiveJob.

Anyway, it seems to me that you might have the recalculation method defined in the wrong place. If I understand it correctly, after destroying a review, an average rating is recalculated based on all reviews of the given reviewable (e.g. a spot). It seems weird to me that such code is defined as a method of a single review instance. A single review (what's more a deleted one) should not know anything about other reviews of the same spot and should not have to deal with them at all.

I guess the method should have been defined as a class method instead, with the reviewable as a parameter. Of course this would mean you'd have to make the other calculation methods overall_rating and number_of_reviews, class methods, too. But I think this is a good thing, again, because the domain of these methods lies outside a single review. Something like the following:

# review.rb
class Review < ActiveRecord::Base
  after_destroy :recalculate_overall_rating

  def recalculate_overall_rating
    self.class.calculate_overall_rating(reviewable)
  end

  def self.calculate_overall_rating(reviewable)
    if number_of_reviews(reviewable) > 0
      reviewable.update_attribute(:overall_rating, overall_rating(reviewable))
    else
      reviewable.update_attribute(:overall_rating, 0)
    end
  end
  handle_asynchronously :calculate_overall_rating
end

Another option (and I like it even a bit more I guess) would be to place the recalculation methods inside the reviewable class, e.g. inside the Post class. If you have more reviewable class types, you could make a module included from all these classes, e.g. Reviewable (I hope that would not clash with the Rails association name though) and place the recalculation methods inside it, this time as instance methods. Why? Because it is an instance of a reviewable which wants to get all its reviews recalculated and which still exists even after a review deletion so it can be easily run asynchronously. Something like the following:

# reviewable.rb
module Reviewable
  def calculate_overall_rating
    if number_of_reviews > 0
      update_attribute(:overall_rating, overall_rating)
    else
      update_attribute(:overall_rating, 0)
    end
  end
  handle_asynchronously :calculate_overall_rating

  # overall_rating and number_of_reviews are also defined in this module
end

# review.rb
class Review < ActiveRecord::Base
  after_destroy :recalculate_overall_rating

  def recalculate_overall_rating
    reviewable.calculate_overall_rating
  end
end

# post.rb
class Post < ActiveRecord::Base
  include Reviewable
end