Controller validation and duck trading! How would you do it?

For the sake of example let’s say you have an application where users list their ducks for sale. The services offers two Plans: Bronze, which allows you to list 3 ducks at a time; and Platinum, which allows you to list an unlimited number of ducks. To handle this logic in the model we add a custom validation to check if user.current_ducks_listed > user.plan.max_number_of_ducks, and throw an error accordingly. Ideally, we don’t want the problem to get this far. It’s best to prevent the user from adding a duck if he has reached his limit. This introduces a chunk of logic whose place isn’t quite clear. Let’s put it in the controller: def new @user = current_user max_ducks = @user.plan.max_number_of_ducks duck_count = @user.ducks.count flash[notice: “You have reached your limit. Upgrade your plan.”] if duck_count > max_ducks @duck = Duck.new end We repeat this logic in the create action. Then, to stay DRY, move this logic to a before_filter that runs on new and create. We are still duplicating the model logic in the controller, though. And what if plans get more complex? This will add bloat to the controller method, and will require updates to the model, the controller, and possibly the view. Also, there’s the question of handling the visual side of this: do we hide the form from the user? If so how?

Is this about how everyone handles this type of scenario? Or is there a better way? Bearing in mind I’m two weeks into my Rails and Ruby studies, perhaps there’s a much better way? How would you handle visual feedback to the user?

Firstly, don't have a current_ducks_listed attribute in the database, just use @user.ducks.count. Then have a validation in the Duck model that prevents the duck from being saved if its owner already has his max allowance.

Colin

For the sake of example let's say you have an application where users list their ducks for sale.

Viaduct? Vi-a no chicken? :wink:

It's best to prevent the user from adding a duck if he has reached his limit. This introduces a chunk of logic whose place isn't quite clear. Let's put it in the controller:

def new

I think you're on the right track having a *warning* show up in ducks_controller#new. If, as Colin suggested, you have the *model* check for excessive ducks as well, #create shouldn't need any alterations from the standard scaffold-generated style. (At least, not for this reason.) It will make the save fail, detected by #create, which will just render #new again, with the errors carried by @duck.

Bearing in mind I'm two weeks into my Rails and Ruby studies,

You're showing a very good grasp of the concepts for only two weeks in! Well done!

-Dave

It would probably just annoy people if they were not told until they'd entered all their "new duck" requirements that they'd reached their limit. So I'd also keep a permissions-type check in the controller too. As Colin says the code to check for this is in the model, so can be re-used by the permissions check and the validation.

  class DucksController < AR:Base     before_filter :check_duck_limit, :only => [:new, :create]

    def new       @duck = Duck.new     end

    private     def check_duck_limit       flash[notice: "You have reached your limit. Upgrade your plan."] if current_user.reached_duck_limit?     end   end

  class User < AR:Base     validate :has_duck_availability

    def reached_duck_limit?       self.ducks.count >= self.max_number_of_ducks     end

    # in case a user can not yet have a plan, it's handy to make sure it exists, and return a safe value     def max_number_of_ducks       plan ? plan.max_number_of_ducks || 0 : 0     end

    private     def has_duck_availability       errors.add(:duck_count, "has been reached") if reached_duck_limit?     end end

PS you don't need to assign "current_user" to an instance variable - just use current_user :wink:

PPS Your current code alerts when the limit is exceeded, rather than reached

Thanks everyone. I appreciate the input. It’s always tricky to decide when logic belongs in the model. I wish there was a clean and obvious definition. I guess it comes with experience.

Not really that tricky.. business logic *always* belongs in the model :wink:

The problem sometimes comes in deciding what is business logic and what is not.

Colin