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 %>
1 Answer 1
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