Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

:history fails when trying to reuse slugs #1009

Open
mreq opened this issue May 26, 2023 · 1 comment
Open

:history fails when trying to reuse slugs #1009

mreq opened this issue May 26, 2023 · 1 comment

Comments

@mreq
Copy link

mreq commented May 26, 2023

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
Copy link
Author

mreq commented May 26, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant