Rails 3: Should I explicitly save an object in an after_create callback?

Question

Relevant Code: http://pastebin.com/EnLJUJ8G

class Task < ActiveRecord::Base
  after_create :check_room_schedule

  ...

  scope :for_date, lambda { |date| where(day: date) }
  scope :for_room, lambda { |room| where(room: room) }

  scope :room_stats, lambda { |room| where(room: room) }
  scope :gear_stats, lambda { |gear| where(gear: gear) } 

  def check_room_schedule
    @tasks = Task.for_date(self.day).for_room(self.room).list_in_asc_order
    @self_position = @tasks.index(self)

    if @tasks.length <= 2
      if @self_position == 0 
        self.notes = "There is another meeting in 
    this room beginning at # {@tasks[1].begin.strftime("%I:%M%P")}."
        self.save
      end
    end
  end

  private

    def self.list_in_asc_order
      order('begin asc')
    end
end

I'm making a small task app. Each task is assigned to a room. Once I add a task, I want to use a callback to check to see if there are tasks in the same room before and or after the task I just added (although my code only handles one edge case right now).

So I decided to use after_create (since the user will manually check for this if they edit it, hence not after_save) so I could use two scopes and a class method to query the tasks on the day, in the room, and order them by time. I then find the object in the array and start using if statements.

I have to explicitly save the object. It works. But it feels weird that I'm doing that. I'm not too experienced (first app), so I'm not sure if this is frowned upon or if it is convention. I've searched a bunch and looked through a reference book, but I haven't see anything this specific.

Thanks.

Answer 1

This looks like a task for before_create to me. If you have to save in your after_* callback, you probably meant to use a before_* callback instead.

In before_create you wouldn't have to call save, as the save happens after the callback code runs for you.

And rather than saving then querying to see if you get 2 or more objects returns, you should be querying for one object that will clash before you save.

In psuedo code, what you have now:

after creation
  now that I'm saved, find all tasks in my room and at my time
  did I find more than one?
    Am I the first one?
      yes: add note about another task, then save again
      no: everything is fine, no need to re-save any edits

What you should have:

before creation
  is there at least 1 task in this room at the same time?
    yes: add note about another task
    no: everything is fine, allow saving without modification

Something more like this:

before_create :check_room_schedule
def check_room_schedule
  conflicting_task = Task.for_date(self.day)
                         .for_room(self.room)
                         .where(begin: self.begin) # unsure what logic you need here...
                         .first
  if conflicting_task
    self.notes =
      "There is another meeting in this room beginning at #{conflicting_task.begin.strftime("%I:%M%P")}."
  end
end