0

I am using the friendly_id gem to handle URL Slugs and when applying a fix to avoid 404's when the slug changes from the documentation, my code doesn't work properly.

The problem is that it simply redirects to the post's show view when I click on the edit button and won't let me make a new post because it "can't find post with ID..." because it's using the find_post method.

I do have the friendly_id_slugs table to store history as well.

In my Post Model:

class Post < ApplicationRecord
  extend FriendlyId
  friendly_id :title, use: :slugged

  ...

  def should_generate_new_friendly_id?
    slug.nil? || title_changed?
  end
end

Post Controller:

class PostsController < ApplicationController
  before_action :find_post

  ...

  def find_post
    @post = Post.friendly.find(params[:id])

    # If an old id or a numeric id was used to find the record, then
    # the request path will not match the post_path, and we should do
    # a 301 redirect that uses the current friendly id.
    if request.path != post_path(@post)
      return redirect_to @post, :status => :moved_permanently
    end
  end
end

I've tried using before_filter but asks me if I mean before_action and I've tried the find_post method in both the public & private section of my controller.

Jake
  • 1,086
  • 12
  • 38
  • `post_path` is the *show* action, whereas you're performing an *edit* action - therefore this will always redirect. Do you actually want this `before_action` to trigger on the `edit` action? – Tom Lord Mar 11 '19 at 22:48
  • @TomLord Oh no, I don't actually as it would always be the up-to-date URL via `@post = Post.friendly.find(params[:id])` I only wanted to implement this from the docs to solve any old URLs in case I update them. – Jake Mar 11 '19 at 23:31

1 Answers1

1

It sounds to me like you may want to skip that redirect logic for anything but the show action, since redirect_to @post only sends you to the show route.

def find_post
  @post = Post.find params[:id]

  if action_name == 'show' && request.path != post_path(@post)
    return redirect_to @post, :status => :moved_permanently
  end
end

Alternately, you can decouple the redirecting behavior from the pre-loading of the post with something like this:

before_action :find_post
before_action :redirect_to_canonical_route, only: :show

def find_post
  @post = Post.find params[:id]
end

def redirect_to_canonical_route
  if request.path != post_path(@post)
    return redirect_to @post, :status => :moved_permanently
  end
end
Unixmonkey
  • 18,485
  • 7
  • 55
  • 78
  • 1
    It would be more conventional to write `before_action :find_post, only: :show` – Tom Lord Mar 11 '19 at 22:50
  • @TomLord Possibly, but `find_post` could also be useful for the other actions...you just don't want to redirect for all of them. Another good option would be extract that into a separate `before_action :redirect_to_canonical_route, only: :show`. – Unixmonkey Mar 11 '19 at 22:51
  • @TomLord I've incorporated this into my answer as an alternative. – Unixmonkey Mar 11 '19 at 22:55
  • What are the benefits of handling the redirecting behavior separately from the pre-load? Anything worth the extra code? – Jake Mar 11 '19 at 23:36
  • @TomLord I have to agree with you, I do like the simplicity of simply having to add `, only: :show` to my code for everything to work. – Jake Mar 12 '19 at 00:41