← Back to Upcase

Change the user_id using a select box


(Scott Hollinshead) #1

Quick overview of what I want my app to do

I have a Job Sheet app where users can submit job sheets after the work they have done.

There are 2 types of users there is normal users and admin users.

The admin Users will submit a form with the customers info on and when the person doing the job has finished the job he will log in and fill in his details in for the job.

The Problem

I want to have a select box that only admin users can see when submitting a job sheet that will containt a list of the users and the value’s will be their user id.

If the user is not an admin he will not see this.

I have the select box working and the paramaters are pointing to the correct user_id when posting. However beasue I am setting the user_id in the controller it overides the user_id from the form, how could i re-write this code to fix the problem I am having?

The Code

class JobSheetsController < ApplicationController
  before_filter :authorize

  def index
    @company = find_company
    @job_sheets = @company.job_sheets.for_user(current_user)
  end

  def new
    @company = find_company
    @job_sheet = @company.job_sheets.new(user: current_user)
    @job_sheet.products.new
    @users = User.all
  end

  def create
    @company = find_company
    @job_sheet = @company.job_sheets.new(job_sheet_params)
    @job_sheet.user = current_user
    @job_sheet.save
    redirect_to company_job_sheets_path(@company)
  end

  def show
    @company = find_company
    @job_sheet = JobSheet.for_user(current_user).find(params[:id])
  end

  def edit
    @company = find_company
    @job_sheet = find_job_sheet(@company)
  end

  def update
    @company = find_company
    @job_sheet = find_job_sheet(@company)
    @job_sheet.update_attributes(job_sheet_params)
    redirect_to company_job_sheets_path(@company)
  end

  def destroy
    @company = find_company
    @job_sheet = find_job_sheet(@company)
    @job_sheet.destroy
    respond_to do |format|
      format.html { redirect_to company_job_sheets_path(@company) }
      format.js
    end
  end



  private

  def find_company
    Company.find(params[:company_id])
  end

  def find_job_sheet(company)
    company.job_sheets.for_user(current_user).find(params[:id])
  end

  def job_sheet_params
    params.require(:job_sheet).permit(:user_id, :address, :postcode, :date, :po_number,:contact,
                                      :phone, :job_type, :reg_no, :vehicle, :vin_no,
                                      :vehicle_milage, :job_instructions, :check_pre,
                                      :check_info, :check_reported_to, :check_customer_aknowledgment,
                                      :post, :info, :reported_to, :customer_aknowledgment,
                                      :certify, :confirm, :name, :comment, :zibox_no, :sim_no,
                                      :safe_box, :new_zibox_no, :new_sim_no, :returned,
                                      :service_call_info, :mobile, products_attributes: [:id, :product_code, :product_description, :alloc_qty, :used, :returned, :_destroy] )
  end
end
class JobSheet < ActiveRecord::Base
  belongs_to :company
  belongs_to :user
  has_many :products, dependent: :destroy
  accepts_nested_attributes_for :products, reject_if: :reject_products, allow_destroy: true

  def self.for_user(user)
    if user.aas_admin?
      order("id DESC")
    else
      where(user: user)
    end
  end

  def reject_products(attributed)
    attributed['product_code'].blank?
  end
end
<%= form_for([@company, @job_sheet]) do |form| %>
  <%= form.select :user_id, @users.map{|user| [user.full_name, user.id]} if current_user.aas_admin? %>
  <div>
    <%= form.label :address, "Installation Address" %><br>
    <%= form.text_area :address %><br>
    <%= form.label :postcode %><br>
    <%= form.text_field :postcode %>
  </div>
...

Any help is appreciated, if you need more info please ask :smile:


(Joel Quenneville) #2

If I understand the problem correctly, you want the following behavior:

  1. Admins get to set any user_id when creating a job sheet
  2. Everyone else automatically has their user_id set when creating a job sheet

The issue is in your create action:

  def create
    @company = find_company
    @job_sheet = @company.job_sheets.new(job_sheet_params)
    @job_sheet.user = current_user # Always current_user even if a different value was set in the form
    @job_sheet.save
    redirect_to company_job_sheets_path(@company)
  end

Essentially, you want to conditionally set the the user_id if it wasn’t already set in the form.

# JobSheetsController
def create
  # ...
    @job_sheet.user_id ||= current_user.id
  # ...
end

As a side note, job_sheet_params allows anyone submitting the form to send the user_id. You probably want to only permit(:user_id) for admins.


(Scott Hollinshead) #3

thanks @joelq I have a few questions about what you have said.

  1. How does @job_sheet.user_id ||= current_user.id work?
    I know that it looks for @job_sheet.user_id and if it is not set the right side comes into action and sets the left side(I think), but how is it getting the user_id on the left side?

  2. @georgebrock did say that permiting the user_id was a big security whole and I was better off creating an AdminsController however I am not 100% sure how I would do that?

Thanks again for your help.

UPDATE

I think I know the answer to my first question could you confirm what I think is right?

Becasue I am doing @job_sheet = @company.job_sheets.new(job_sheet_params) if the user_id is present it the params @job_sheet.user_id will be found from the params, but if it can’t find it in the params, it set’s it to the current_user’s id ?


(George Brocklehurst) #4

@scott I was talking generally about using admin controllers for situations where the admin and the user have access to significantly different behaviour.

In this case the user and and admin behaviour are close enough that you could start by modifying your job_sheet_params method to do different things for admin users and regular users.


(Scott Hollinshead) #5

@georgebrock Oh right okay.

I have changed my params, but it dosn’t seem to be adding the :user_id to my attributes, can you see why?

def job_sheet_params
  params.require(:job_sheet).permit(*job_sheet_attributes)
end

def job_sheet_attributes
    attributes = [:address, :postcode, :date, :po_number,:contact,
    :phone, :job_type, :reg_no, :vehicle, :vin_no,
    :vehicle_milage, :job_instructions, :check_pre,
    :check_info, :check_reported_to, :check_customer_aknowledgment,
    :post, :info, :reported_to, :customer_aknowledgment,
    :certify, :confirm, :name, :comment, :zibox_no, :sim_no,
    :safe_box, :new_zibox_no, :new_sim_no, :returned,
    :service_call_info, :mobile,
    products_attributes: [:id, :product_code, :product_description, :alloc_qty, :used, :returned, :_destroy]]

    if current_user.aas_admin?
      attributes << :user_id
    end
  end

(Scott Hollinshead) #6

adding return attributes at the end of my job_sheet_attributes method seems to have fixed the issue.


(Joel Quenneville) #7

Correct. If the user_id is present in the params, it will get set on @job_sheet. ||= current_user.id sets the current_user’s id if the id has not been set.