Can't update user in Ruby on Rails

Question

I am using ruby on rails try to update user information, but when I submit, the console will show an error saying the user exists and redirect to the correct page. What's wrong with my code?

The error message:

 Started PATCH "/users/6" for ::1 at 2015-06-08 21:27:00 -0500
  Processing by UsersController#update as HTML
  Parameters:  {"utf8"=>"✓",  "authenticity_token"=>"sJm38g36DAYDo4eXdpcIPRX0e40Jp6cECmMwEvhCAEhTlDwwmmgOfXZqeczglNmJ4K9pQXiyXAsRsgP/C8lScg==", "name"=>"test123", "department"=>"123", "commit"=>"Update User",  "id"=>"6"}

 User Load (0.1ms)  SELECT  "users".* FROM "users" WHERE  "users"."id" = ? LIMIT 1  [["id", 6]]   CACHE (0.0ms)
 SELECT   "users".* FROM "users" WHERE "users"."id" = ? LIMIT 1  [["id", "6"]]   (0.1ms)  
 begin transaction
 User Exists (0.2ms)
  SELECT  1 AS one  FROM "users" WHERE ("users"."email" = '[email protected]' AND "users"."id" != 6) LIMIT 1    (0.1ms)
rollback transaction
  Redirected to  http://localhost:3000/users/6
Completed 302 Found in 9ms  (ActiveRecord: 0.5ms)

Started GET "/users/6" for ::1 at 2015-06-08 21:27:00 -0500     
  Processing  by UsersController#show as HTML
  Parameters: {"id"=>"6"}  User Load  (0.1ms)
  SELECT  "users".* FROM "users" WHERE "users"."id"
= ? LIMIT 1  [["id", 6]]
  Rendered users/show.html.erb within layouts/application  (0.1ms)
  User Load (0.2ms)
  SELECT  "users".* FROM "users" WHERE  "users"."id" = ? LIMIT 1  [["id", 6]]   CACHE (0.0ms)
  SELECT   "users".* FROM "users" WHERE "users"."id" = ? LIMIT 1  [["id", 6]]
Completed 200 OK in 66ms (Views: 64.3ms | ActiveRecord: 0.3ms)

The edit page

<h1 class="center">Edit name</h1>

<div class="row">
  <div class="col-md-6 col-md-offset-3">

    <%= form_tag "/users/#{@user.id}", :method => 'patch' do %>

      <p>
        <%= label_tag :name %>
        <%= text_field_tag :name, @user.name %>
      </p>

      <p>
        <%= label_tag :department %>
        <%= text_field_tag :department, @user.dept %>
      </p>

      <input type="submit" name="commit" value="Update User">
    <% end %>
  </div>

</div>

The controller is like this

class UsersController < ApplicationController
  before_action :authorize, only: [:show, :edit, :update]

  def authorize
    @user = User.find_by(id: params[:id])
    if @user.blank? || session[:user_id] != @user.id
      redirect_to root_url, notice: "Nice try!"
    end
  end

  def new 
    @user = User.new 
  end

  def show
  end

  def edit
  end

  def update
    @user = User.find_by(id: params[:id])
    @user.name = params[:name]
    @user.dept = params[:department]
    @user.save
    redirect_to user_path(@user.id)
  end

  def create
    @user = User.new(email: params[:email], 
                    name: params[:name], 
                    password: params[:password],
                    role: params[:role],
                    dept: params[:dept])
    if @user.save
      redirect_to root_url, notice: "Thanks for signing up."
    else
      render "new"
    end
  end
end

The router concerning this part is like:

  # sign up
  get '/signup'               => 'users#new'
  post '/users'               => 'users#create'
  get '/users/:id'            => 'users#show', as: :user
  get '/users/:id/edit'       => 'users#edit', as: 'edit_user'
  patch '/users/:id'          => 'users#update'

Answer 1

Are you using rails 4?

You should update your controller to conform to strong_parameters if you are.

def update
  @user = User.find(params[:id)
  if @user.update_attributes(user_params)
    redirect_to user_path(@user.id)
  else
    render :edit
  end
end

private

def user_params
  params.require(:user).permit(:name, :dept)
end

Doing this will mean that you have to wrap your name and dept params inside a user scope, eg.

user: { name: "Howard Moon", dept: "Zookeeper" }

But is the standard way to handle params in the controller.

Hope this helps!

EDIT: Link to Strong Parameters which does a better job at explaining this than I can. Haha

Answer 2

The problem is in the form_tag,it should be like this

<%= form_tag({:action => :update}, {:method => :patch}) do %>

Also your code for form_tag looks vulnerable. Changing it to like this will be better.

<%= form_tag update_user_path(@user) do %> 

               or

<%= form_tag user_path(@user), :method => :patch do %>