Redirect in destroy action not working properly

Question

I am building a simple blog app using Ruby on Rails that allows users to log in/out, sign up and perform actions on their articles and profiles based on permissions and restrictions. I have encountered a problem with the destroy User action. In the users/index view(where all existing users are listed), it causes no errors due to the url path containing no {:id}, but the redirect_to root_path does not work. If the same action is executed in the users/show page(personal profile page with some info and associated articles), due to the url being localhost/users/id, when the user is deleted I get "Couldn't find User with 'id'=33" error shown below. If I manually go to the root route, the successful account deletion message shows up and the action is performed correctly. So this is not a metter of DESTROY not working, but of redirection I believe. I have tried redirecting to different paths but it still doesn't work. Here are the related files:

routes.rb

Rails.application.routes.draw do
root "pages#home"
get "about", to: "pages#about"

resources :articles

get "signup", to: "users#new"
resources :users, except: [:new] 

get 'login', to: 'sessions#new'
post 'login', to: 'sessions#create'
get 'logout' => :destroy, to: 'sessions#destroy'
end

pages_controller

class PagesController < ApplicationController
def home
    redirect_to articles_path if logged_in?
end

def about
end         
end

users_controller

class UsersController < ApplicationController
before_action :set_user, only: [:show, :edit, :update, :destroy]
before_action :require_user, only: [:edit, :update]
before_action :require_same_user, only: [:edit, :update, :destroy]

def index
    @users = User.all
end

def show
    @articles = @user.articles
end

def new
    @user = User.new
end

def edit
end

def create
    @user = User.new(user_params)
    if(@user.save)
        session[:user_id] = @user.id  #logs user in automatically once they are signed up
        flash[:notice] = "Welcome to AlphaBlog, #{@user.username}!"
        redirect_to articles_path
    else
        render 'new'
    end
end

def update
    if @user.update(user_params)
        flash[:notice] = "Account updated!"
        redirect_to @user
    else
        render 'edit'
    end
end

def destroy
    @user.destroy
    session[:user_id] = nil
    flash[:notice] = "Account and all associated articles deleted!"
    redirect_to root_path
end

private
def user_params
    params.require(:user).permit(:username, :email, :password)
end

def set_user
    @user = User.find(params[:id])
end

def require_same_user
    if current_user != @user
        flash[:alert] = "You can only edit your own profile!"
        redirect_to current_user
    end
end

end

sessions_controller

class SessionsController < ApplicationController

def new
end

def create
    user = User.find_by(email: params[:session][:email].downcase)
    if user && user.authenticate(params[:session][:password])
        session[:user_id] = user.id
        flash[:notice] = "Logged in successfully!"
        redirect_to user
    else
        flash.now[:alert] = "There was something wrong with your login details!"
        render 'new'
    end

end

def destroy
    session[:user_id] = nil
    flash[:notice] = "Logged out."
    redirect_to root_path
end

end

users/index.html.erb

<div class = "header">
<h1>
    AlphaBlog
    <% if logged_in? %>
        <%= link_to 'Articles', articles_path, method: :get, class: "index-button-to" %>
    <% else %>
        <%= link_to 'Home', root_path(), method: :get, class: "index-button-to" %>
        <%= link_to 'Articles', articles_path, method: :get, class: "index-button-to" %>
    <% end %>
    <%= render 'layouts/log_in_out_navigation'%>
</h1>
</div>

<h2>Alpha Bloggers</h2>

<div class="index-container">
<%# cycle through all articles and show them all in a table %>
<% @users.each do |user| %>

    <div class = "index-article-container">

        <div class="index-article-user" style = "color:rgb(16, 136, 255);">
            <%= user.username %>
        </div>

        <div class="white">

            <div class="index-article-title">
                <%= gravatar_for(user, size: 150) %>
            </div>

            <div class="index-article-description">
                <%# gives the plural word for multiple articles %>
                <%= pluralize(user.articles.count, "article") %>
            </div>

            <div class="index-article-actions">
                <%# shows selected article page %>
                <%= link_to 'View Profile', user, class: "index-link-to show" %>
                <% if logged_in? && current_user.username == user.username %>
                    <%# shows selected article EDIT page. edit_article_path because in routes, 
the prefix for edit is edit_article && (article) because we need the id for the path as well%>
                    <%= link_to 'Edit Profile', edit_user_path(user), data: { turbo_method: 
:get}, class: "index-link-to edit" %>
                    <%= link_to 'Delete Profile', user_path(current_user), data: { 
turbo_method: :delete, turbo_confirm: "Are you sure? (This will also delete all of your 
articles)" }, class: "index-link-to delete" %>
                <% end %>
            </div>
                    
        </div>
                    

        <div class="index-created-updated">
            Joined <%= time_ago_in_words(user.created_at) %> ago.
        </div>

    </div>
    
<% end %>

users/show.html.erb

<div class = "header">
<h1>
    AlphaBlog
    <% if logged_in? %>
        <%= link_to 'Articles', articles_path, method: :get, class: "index-button-to" %>
        <%= link_to 'Bloggers', users_path, method: :get, class: "index-button-to" %>
    <% else %>
        <%= link_to 'Home', root_path(), method: :get, class: "index-button-to" %>
        <%= link_to 'Articles', articles_path, method: :get, class: "index-button-to" %>
        <%= link_to 'Bloggers', users_path, method: :get, class: "index-button-to" %>
    <% end %>
    <%= render 'layouts/log_in_out_navigation'%>
</h1>
</div>

<h2> <%= @user.username %>'s profile </h2>

<div class="show-users-image">
<%# gravatar_for method created in helpers/application_helper %>
<%= gravatar_for @user, size: 200 %>
<% if logged_in? && current_user.username == @user.username %>
    <div class="index-profile-actions">
        <%= link_to "Edit Profile", edit_user_path(@user), class: "index-link-to edit" %>
        <%= link_to 'Delete Profile', user_path(current_user), data: { turbo_method: :delete, 
turbo_confirm: "Are you sure? (This will also delete all of your articles)" }, class: "index- 
link- 
to delete", style: "margin-top:0.3vh" %>
    </div>
<% end %>
</div>

<h3 style = "text-align:center">Articles</h3>
<%= render 'articles/article' %>

error page

Answer 1

The way to do this in Rails 7 is to update the destroy action in the UsersController by adding status: :see_other after the redirect, as follows:

def destroy
    @user.destroy
    session[:user_id] = nil
    flash[:notice] = "Account and all associated articles deleted!"
    redirect_to root_path, status: :see_other
end

Answer 2

I think the answer here is really a very different layout of your routes and controller (or to not reivent the wheel in the first place). Passing the user id through the parameters would be fine if your making a system where you are managing other users - but its pretty wonky when users are managing their own profiles.

For example this is how users CRUD their own profiles in a vanilla Devise setup:

Verb    URI Pattern               Controller#Action                    
------------------------------------------------------------------------
GET     /users/cancel(.:format)   devise/registrations#cancel 
GET     /users/sign_up(.:format)  devise/registrations#new
GET     /users/edit(.:format)     devise/registrations#edit
PATCH   /users(.:format)          devise/registrations#update                                                                          
PUT     /users(.:format)          devise/registrations#update                                                                          
DELETE  /users(.:format)          devise/registrations#destroy                                                                         
POST    /users(.:format)          devise/registrations#create  

Note the lack of the :id parameter in the URI Pattern. Thats because its implied that the resource in question is the currently signed in user, and that the user is identified through the session (or a token).

The controller is named Registrations to avoid the ambiguity if the programmer later wants to add a UsersController to manage other users.

If you want to do something similiar you can generate singular routes by using the resource macro instead of resources.

# routes for user registration
resource :registrations, 
    only: [:new, :edit, :update, :create, :destroy]

# routes for viewing other users 
resources :users, only: [:index, :show]

Which will generate:

Prefix                Verb   URI Pattern                    Controller#Action
-----------------------------------------------------------------------
new_registrations     GET    /registrations/new(.:format)   registrations#new
edit_registrations    GET    /registrations/edit(.:format)  registrations#edit
registrations         GET    /registrations(.:format)       registrations#show    
                      PATCH  /registrations(.:format)       registrations#update  
                      PUT    /registrations(.:format)       registrations#update  
                      DELETE /registrations(.:format)       registrations#destroy
                      POST   /registrations(.:format)       registrations#create

Name it whatever you want. The core takeaway here is to not confuse two completely different problems - user management and user registrations and have separate endpoints and controllers for each responsibilty.

Then in your controller you simply authenticate the user from the session and redirect the user if they are not authenticated:

# Handles user account registration, updates and deleting accounts
class RegistrationsController < ApplicationController
  before_action :require_user, except: [:new, :create]

  # Displays the form for signing up a user
  # GET /registrations
  def new
    @user = User.new
  end

  # Register a new user and sign them in
  # POST /registrations
  def create
    @user = User.new(user_params)
    if @user.save 
      reset_session # avoids session fixation attacks
      session[:user_id] = @user.id  #logs user in automatically once they are signed up
      flash[:notice] = "Welcome to AlphaBlog, #{@user.username}!"
      redirect_to articles_path
    else
      render :new
    end
  end

  # Form for editing the users own profile
  # GET /registrations/edit
  def edit
    @user = current_user
  end

  # Update the currently signed in user
  # PATCH /registrations
  def update
    @user = current_user
    if @user.update(user_params)
      flash[:notice] = "Account updated!"
      redirect_to current_user
    else
      render :new
    end
  end

  # Cancel the current users registration 
  # DELETE /registrations
  def delete
    current_user.delete
    reset_session # avoids session fixation attacks
    flash[:notice] = "Account and all associated articles deleted!"
    redirect_to root_path
  end
  
  private 

  def user_params
    params.require(:user).permit(:username, :email, :password)
  end
end

# Displays users
# Managing accounts is handled by RegistrationsController
class UsersController < ApplicationController
  # GET /users
  def index
    @users = User.all
  end
  
  # GET /users/1
  def show
    @user = User.find(params[:id])
    @articles = @user.articles
  end
end

Since their is no id in the path you you need to set the delete button to send to the right path:

<%= button_to "Delete your account", registrations_path, method: :delete %>

And adjust your forms:

<%= form_with(model: @user, url: registrations_path) do |form| %>
  # ...
<% end %>

Doing this the correct way would really be to do it the same way that Devise does and have a normal link that sends a GET request to a "Are you sure you want to delete your account?" page when then requires the user to enter their password or email and submits a DELETE request so that users don't accidentially delete their accounts.

But then again don't reinvent the authentication wheel unless you want a long and tedious lesson into wheelmaking.

Answer 3

I'm not sure you have truly got to the bottom of this. In your original approach I suspect two things are going on:

1. In users/index.html.erb you have link_to 'Delete Profile', user_path(current_user) but I think you want user_path(user). What you currently have will have every delete button try to delete the same user.

2. The fact that error says your are attempting to execute 'show' rather than 'destroy' makes me suspect that you do not have turbo loaded properly. You don't say what version of rails you are using, but for versions earlier than 7 you don't get turbo out of the box, and you should use UJS instead. compare this https://guides.rubyonrails.org/getting_started.html#deleting-an-article with this https://guides.rubyonrails.org/v6.1/getting_started.html#deleting-an-article