Assign an instance variable based on a linked table field in Rails

Question

I'm trying to create a system that checks whether the current user is authorized to edit a record. I think the way to do it is to assign and instance variable of @user to the linked ID of the displayed record and a private function that redirects unless @user = current_user, but when I do this, I get redirect behavior every time (even when a user owns the record).

Here is the controller setup

def edit 
    @deal = Deal.find(params[:id]) 
    @user = User.where(params[:id] === @deal.user_id)
    correct_user
  end
...
private

  def correct_user
    unless @user === current_user
      redirect_to root_path
      flash[:error] = 'You can only edit deals that you own'
    end
  end

The user and deal models are related through has many and owns

class User < ApplicationRecord
  has_many :deals
...
class Deal < ApplicationRecord
  belongs_to :user

And the schema for deals (user id is assigned at creation to user_id)

 create_table "deals", force: :cascade do |t|
    t.text     "headline"
    t.string   "matter"
    t.text     "summary"
    t.integer  "user_id"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.index ["user_id"], name: "index_deals_on_user_id"
  end

The project is setup with Devise so those helpers are also available.

score 1 · Accepted Answer · edited May 23 '17 at 12:10

1

use == instead of === you just want to compare @user with current_user

@user == current_user

See the difference here === vs. == in Ruby

Also You need to change

@user = User.where(params[:id] === @deal.user_id)

to

@user = @deal.user

edited May 23 '17 at 12:10

Community

1
1

answered Mar 22 '17 at 12:56

Deepak Mahakale

22,834
10
68
88

score 0 · Answer 2 · answered Mar 22 '17 at 12:59

Ok, so where to start...

First of all, don't use === to compare records, use == instead.

Secondly:

def edit 
  @deal = Deal.find(params[:id]) 
  @user = User.find_by(params[:id]: @deal.user_id)
  correct_user
end

When trying to find user, you need to provide AR options as a hash, instead of boolean parameter. Either use find if you want to raise exception when record not found, or find_by to simply return nil.

And in correct_user again use == instead of ===.

However this code still isn't very pretty and could use some refactoring ;)

Assign an instance variable based on a linked table field in Rails

2 Answers2