Create a gist now

Instantly share code, notes, and snippets.

What would you like to do?
class Ticket < ActiveRecord::Base
belongs_to :grouper
belongs_to :user
validate :user_cant_be_blacklisted, on: :confirmation
validate :user_cant_double_book, on: :confirmation
validate :grouper_cant_be_full, on: :confirmation
validate :grouper_cant_have_occurred, on: :confirmation
validate :ticket_cant_have_been_confirmed, on: :confirmation
def confirm
update confirmed: true if confirmable?
end
def confirmable?
valid? :confirmation
end
private
def user_cant_be_blacklisted
errors.add :user, "can't book a Grouper at this time" if user.blacklisted?
end
def user_cant_double_book
errors.add :user, 'are already going to a Grouper on that day' if user.has_existing_grouper?(grouper)
end
def grouper_cant_be_full
errors.add :grouper, 'has already occurred!' if grouper.full?
end
def grouper_cant_have_occurred
errors.add :grouper, 'has already occurred!' if grouper.past?
end
def ticket_cant_have_been_confirmed
errors.add :user, 'have already confirmed this ticket' if confirmed?
end
end

marten commented Mar 20, 2014

I like this much more than that mess of policy objects. Wouldn't this mean that if a view calls ticket.confirmable? somewhere, e.g. to see if a "Confirm" button should be shown, after that point the ticket might have a bunch of errors set on it? Might have strange interactions if you further down in the view you show ticket.errors. Not that I've ever found this to be an issue in real life though.

wrong error message on line 32, not has already occurred!

errors.add :grouper, 'is already full!' if grouper.full?

I didn't know that this is possible and judging from all the if: or unless: constructs I'm obviously not the only one. It's a pity though that I can't pass an array to the on: or context: option. Is this intended?

David, how would you handle the logic in the controller which sets the redirect based on the error message? I think to couple the controller code to the message text is not optimal, in case one changes the message text. I suppose one could define constants for these, and go off of the constants. And then the question is whether the controller should have all the logic pairing up the specific error message with the right redirect. I would tend to write code directly in the controller at first, and then refactor into a separate class where I could verify the logic separate from the actual controller, per my example here: justin808/fat-code-refactoring-techniques#6

I wish there was an :except_on. I want to validate association presence except in tests, where I don't want to necessarily create a huge object graph just to test some isolated functionality. My strategy has been to use an attr_accessor to set :skip_association_presence_validations in tests. I wish I could use contexts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment