How would you improve the following controller method?

[fizzi]

I made this code but someone told me it was flawed

How would you improve this piece of code in the reviewcontroller (mainly the namereview method)?

class ReviewController < ApplicationController
  before_action :set_reviews, only: [:show, :edit, :update, :destroy]

  def index
    @reviews = Review.all
  end

  def namereview
    @review = Review.find(params[:id])
    if @review.update_attribute(:description, sanitize(params[:description]))
     format.json { render json: { status: 200 } }
    else
     format.json { render json: { status: 500 } }
    end
  end
end

Thanks in advance

[Frederick Cheung]

On Wednesday, June 3, 2015 at 6:31:41 AM UTC+1, fizzi wrote:

I made this code but someone told me it was flawed

How would you improve this piece of code in the reviewcontroller (mainly the namereview method)?

Looking just at that method:

- I wouldn't call it namereview - that seems to suggest that it sets a review name but it doesn't.  I'd call it update, and make it behave like a normal update method (i.e. it should use params[:review][:description]). You can leave it only updating the description attribute, but at least bring the semantics closer to the 'normal' update action

- 422 is the usual http status for failed validations

- your code seems to let anyone edit any review - not sure if that is appropriate.

Fred