Where does the Build last commiter logic belong?

[richardkmichael]

When a build is created, we:

• ensure the last commiter has a user account
• ensure the last commiter is a collaborator on the build's project

At the moment, the BuildsController sets up the last commiter:

      @build.last_commiter = User.find_by_email(params[:last_commiter]) ||
                             User.new( :email => params[:last_commiter],
                                       :password => '*LOCKED*' )

and the Build model adds the last commiter to the collaborations:

after_save :add_last_commiter_to_collaborations

However, the BuildsController could also look up the collaborations membership (instead of doing this in the model), and add it if required:

 @build.project.users << @build.last_commiter

Similarly, the model could check for the last commiter user account and create one if required.

How to make decisions about which components (controllers vs. models) should be responsible for which logic?

One problem with the BuildsController manipulating the collaborations is that we don't want the collaborations list altered until after the Build has been saved (known to be valid, etc.) ; suggesting this logic belongs in a model callback.

[franckverrot]

The controller should never contain any logic. If you think some logic shouldn't belong to the model either, you might wanna create a "service" that does it for you which will be the glue between your models.