Rails controller method to accept one rating per IP address via AJAX

Question

My rating controller:

  def rate
    @ip = request.remote_ip
    @konkurrencer = Rating.where(:IP => @ip, :konkurrencer_id => params[:id])
    if @konkurrencer.empty?
      params[:kon][:IP] = request.remote_ip
      params[:kon][:konkurrencer_id] = params[:id]
      @konkurrencer = Rating.new(params[:kon])
      @konkurrencer.save
      @konkurrencer.konkurrencer.rating_score += params[:kon][:ratings].to_i
      @konkurrencer.konkurrencer.ratings += 1
      @konkurrencer.save
    else
      // render error message
    end
    render :nothing => true
   end
end

My AJAX:

 jQuery.ajax({
     url: frm.attr('action'), //your server side script
     data: frm.serialize(), //our data
     type: 'POST',
     success: function (data) {
          //write here something when everything went ok

     },
     error: function (jxhr, msg, err) {
         alert(msg); //something went wrong.
     }
 });

Have do I make my controller more DRY?

Answer 1

I would try and move code up into the model for a 'fat model, thin controller' approach. You want your business logic to be in the model whenever possible. This is a DRY approach that reduces duplication and makes testing easier.

The find: @konkurrencer = Rating.where(:IP => @ip, :konkurrencer_id => params[:id]) should be a model finder, maybe a model method called 'konkcurrent'

def koncurrent
  where(:IP => params[:id], :konkurrencer_id => params[:id]
end

Also this has a repeat:

  @konkurrencer.save
  @konkurrencer.konkurrencer.rating_score += params[:kon][:ratings].to_i
  @konkurrencer.konkurrencer.ratings += 1
  @konkurrencer.save

so I would remove the first .save I would also look to transfer this to the model, you could have a model method called 'increment_rating_score'

Add model testing for both of these.

Please note that symtax is aimed at rails3 and code is pseudo code to show approach, syntax might need cleaning up.

Answer 2

Controller code looks good.. You only need to refactor code little bit so that it will look more clean & robust. otherwise rest things are ok. regarding ajax response, you need to implement success/failure response either inside controller or js file.

just to give idea on implementing ajax response inside controller

def update
  render :update do |page|
    if success
      # here replace_html is prototype helper method
      page.replace_html "DIV_ID", "PARTIAL"
      # if you are using jquery then you can execute js code like
      page << "js code goes here"
    else
      page.replace_html "ERR_DIV", "Error occured"
    end
  end
end