Rails 6: Attaching Devise current_user.id to form submissions

Question 1

In my Ruby on Rails 6 app:

I have a User class built with Devise, and I scaffolded a List class where each List should belong_to a User.

I want Lists created through my app to be automatically assigned to the logged-in user. This should happen on the back end, after the rest of the new List fields have been posted to the server.

I was able to make this happen by adding a line to the default list-params definition in the scaffolded controller:

def list_params
  params[:list].merge!(:user_id => current_user.id.to_s)
  params.require(:list).permit(:title, :active, :user_id)
end

My questions are:

Am I reinventing the wheel somehow with this approach?
Does modifying the list_params in this way affect the application outside of creating and updating Lists?
Have I introduced any potential security issues by doing things this way?

For reference, here is the rest of the controller:

class ListsController < ApplicationController
  before_action :authenticate_user!
  before_action :set_list, only: [:show, :edit, :update, :destroy]

# GET /lists
# GET /lists.json
def index
  @lists = List.all
end

# GET /lists/1
# GET /lists/1.json
def show
end

# GET /lists/new
def new
  @list = List.new
end

# GET /lists/1/edit
def edit
end

# POST /lists
# POST /lists.json
def create
  @list = List.new(list_params)

  respond_to do |format|
    if @list.save
      format.html { redirect_to @list, notice: "List was successfully created." }
      format.json { render :show, status: :created, location: @list }
    else
      format.html { render :new }
      format.json { render json: @list.errors, status: :unprocessable_entity }
    end
  end
end

# PATCH/PUT /lists/1
# PATCH/PUT /lists/1.json
def update
  respond_to do |format|
    if @list.update(list_params)
      format.html { redirect_to @list, notice: "List was successfully updated." }
      format.json { render :show, status: :ok, location: @list }
    else
      format.html { render :edit }
      format.json { render json: @list.errors, status: :unprocessable_entity }
    end
  end
end

# DELETE /lists/1
# DELETE /lists/1.json
def destroy
  @list.destroy
  respond_to do |format|
    format.html { redirect_to lists_url, notice: "List was successfully destroyed." }
    format.json { head :no_content }
  end
end

private

# Use callbacks to share common setup or constraints between actions.
def set_list
  @list = List.find(params[:id])
end

# Only allow a list of trusted parameters through.
def list_params
  params[:list].merge!(:user_id => current_user.id.to_s)
  params.require(:list).permit(:title, :active, :user_id)
end
end

And here is the form template:

<%= form_with(model: list, local: true) do |form| %>
    <% if list.errors.any? %>
      <div id="error_explanation">
        <h2><%= pluralize(list.errors.count, "error") %> prohibited this list from being saved:</h2>

        <ul>
          <% list.errors.full_messages.each do |message| %>
            <li><%= message %></li>
          <% end %>
        </ul>
      </div>
    <% end %>

    <div class="field">
      <%= form.label :title %>
      <%= form.text_field :title %>
    </div>

    <div class="field">
      <%= form.label :active %>
      <%= form.check_box :active %>
    </div>


    <div class="actions">
      <%= form.submit %>
    </div>
  <% end %>

Question 2

Why do we need user_id in params at all? Probably, there is a need to add some context to a current scope. And yes, you’re trying to write straightforward logic in a different way.

Here is some suggested refactoring:

class ListsController < ApplicationController
  def new
    build_resource
  end

  def create
    build_resource

    respond_to do |format|
      if update_resource
        # ...
      else
        # ...
      end
    end
  end

  def update
    update_resource
    # ...
  end

  def destroy
    destroy_resource
  end

  private

  def permitted_params
    params.require(:list).permit(:title, :active)
  end

  def resource
    @resource ||= current_user.lists.find(params[:id])
  end

  def build_resource
    @resource = current_user.lists.new
  end

  def update_resource
    resource.update(permitted_params)
  end

  def destroy_resource
    resource.destroy
  end
end

For those pure-CRUD controllers common logic can be easily moved into concern. We’ve created a gem some time ago https://github.com/cimon-io/unobtrusive_resources

Leave a Reply Cancel reply