norman / friendly_id

FriendlyId is the “Swiss Army bulldozer” of slugging and permalink plugins for ActiveRecord. It allows you to create pretty URL’s and work with human-friendly strings as if they were numeric ids for ActiveRecord models.
http://norman.github.io/friendly_id/
MIT License
6.15k stars 591 forks source link

:history fails when trying to reuse slugs #1009

Open mreq opened 1 year ago

mreq commented 1 year ago

Steps to reproduce:

1) create an instance a of a Model with slug foo 2) change slug of a to bar 3) try to create an instance b of a Model with slug foo 4) you'll get an ActiveRecord::RecordNotUnique exception

Ways to fix:

a) validate the slug respecting friendly_id_slugs table b) preferred - remove the old slug if it's not the current one and make the instance b invalid via validating if it is

mreq commented 1 year ago

Here's a concern implementing b) https://github.com/sinfin/folio/blob/master/test/models/concerns/folio/friendly_id/history_test.rb

module Folio::FriendlyId::History
  extend ActiveSupport::Concern

  included do
    extend FriendlyId

    before_save :remove_conflicting_history_slugs
  end

  private
    def remove_conflicting_history_slugs
      if slug.present?
        scope_names = if self.class.friendly_id_config.uses?(:scoped)
          self.class.friendly_id_config.scope_columns.sort.map { |column| "#{column}:#{send(column)}" }.join(",")
        else
          nil
        end

        existing_scope = FriendlyId::Slug.where(sluggable_type: self.class.base_class.to_s,
                                                slug:,
                                                scope: scope_names)
                                         .where.not(sluggable_id: id)

        existing_scope.each do |slug|
          last_slug_for_record = FriendlyId::Slug.where(sluggable_type: slug.sluggable_type,
                                                        sluggable_id: slug.sluggable_id,
                                                        scope: slug.scope)
                                                 .order(id: :asc)
                                                 .last

          slug.destroy if slug.id != last_slug_for_record.id
        end
      end
    end
end

and a simple test showcasing the fix https://github.com/sinfin/folio/blob/master/test/models/concerns/folio/friendly_id/history_test.rb

# frozen_string_literal: true

require "test_helper"

class Folio::FriendlyId::HistoryTest < ActiveSupport::TestCase
  test "remove_conflicting_history_slugs" do
    page = create(:folio_page, slug: "foo")
    page.update!(slug: "bar")

    assert FriendlyId::Slug.exists?(slug: "foo",
                                    sluggable_id: page.id,
                                    sluggable_type: "Folio::Page",
                                    scope: "site_id:")

    assert FriendlyId::Slug.exists?(slug: "bar",
                                    sluggable_id: page.id,
                                    sluggable_type: "Folio::Page",
                                    scope: "site_id:")

    # the following line fails without the fix
    new_page = create(:folio_page, slug: "foo")

    assert FriendlyId::Slug.exists?(slug: "foo",
                                    sluggable_id: new_page.id,
                                    sluggable_type: "Folio::Page",
                                    scope: "site_id:")

    assert_not FriendlyId::Slug.exists?(slug: "foo",
                                        sluggable_id: page.id,
                                        sluggable_type: "Folio::Page",
                                        scope: "site_id:")

    assert FriendlyId::Slug.exists?(slug: "bar",
                                    sluggable_id: page.id,
                                    sluggable_type: "Folio::Page",
                                    scope: "site_id:")
  end
end