Can't explicitly set slug when using `scoped`

[joe-reich]

Hiya. Thanks for creating and maintaining this gem.

I ran into an issue: explicitly setting the slug doesn't seem to work when the model uses scoped.

Here's some pseudo-rails to give the gist.

Without scoped, it works fine:

class State < ApplicationRecord
  include FriendlyId

  friendly_id: :state_name
end

oh = State.create(state_name: 'Ohio')
oh.slug # returns "ohio" (normal)

ny = State.create(state_name: 'New York', slug: 'ny')
ny.slug #returns 'ny' (success)

With scoped, it runs into trouble:

class County < ApplicationRecord
  include FriendlyId
  belongs_to :state

  friendly_id: :county_name, use: :scoped, scope: :state
end

franklin = County.create(county_name: "Franklin", state: oh)
franklin.slug # returns "franklin" (normal)

kings = County.create(county_name: "Kings", state: ny, slug: "kgs")
kings.slug # returns "kings" (the bug)

Looks like the issue is here:

https://github.com/norman/friendly_id/blob/81442a69df30a604e63fbbd789ba22464de5c758/lib/friendly_id/scoped.rb#L138-L140

Within the gem codebase, it looks like this issue only arises in Scoped. But it would also arise for users who followed the suggestion to redefine should_generate_new_friendly_id? in their models.

If the maintainers agree with what I'm suggesting is the expected behavior, I might be able to find time to put a PR together.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[luctus]

Hey there @joe-reich! I just run into the same issue... were you able to find a workaround? Thanks!

[joe-reich]

@luctus I monkey-patched should_generate_new_friendly_id?. We ended up not shipping this for various reasons, and IIRC even this redefinition doesn't produce natural behavior in every set of circumstances. Proceed with caution!

    def should_generate_new_friendly_id?
      # hopeless if the base for setting slug is nil
      return false if public_send(friendly_id_config.base).nil?

      # Slug is nil.  Gotta try.
      return true if public_send(friendly_id_config.slug_column).nil?

      # Slug was set explicitly. Leave it.
      return false if friendly_id_config.slug_column.in?(changed)

      # Base for setting slug changed. Change the slug.
      return true if slug_base_changed?

      # Basis for scoping has changed.  Reset slug.
      return true if scoping_base_changed?

      false
    end

    # Can be overwritten in the model
    def slug_base_changed?
      friendly_id_config.base.to_s.in?(changed)
    end

    private

    def scoping_base_changed?
      return false unless friendly_id_config.uses?(:scoped)

      (changed & friendly_id_config.scope_columns).any?
    end

[Adeynack]

One of our model had a non-scoped slug for many years and creating directly with a manual slug, when user specify it, is established behavior. We have a change of model and now, that slug is scoped. This bug sadly breaks the slug overriding at creation time that our users are used to. We can of course patch this in the controller, updating right behind with the slug if it's in the parameters, but it would simply be cleaner to solve it.

I will try to find time to propose a PR, but to be realistic, I do not see that happening in the near future. I am however writing those words to emphasis our interest in it being solved at some point, if a maintainer can find the time.

Many thanks in advance.

[sandrasi]

The documentation says that the should_generate_new_friendly_id? method is added to the model and therefore it can be overridden if needed (i.e. no need to monkey-patch the gem or send a new PR that changes the current behavior).

class MyModel
  extend FriendlyId
  friendly_id :base_attr, use: %i[scoped slugged], scope: :scope_attr
  ...
  # Override method added by FriendlyId::Scoped to prevent overwriting explicitly set slugs.
  def should_generate_new_friendly_id?
    send(friendly_id_config.slug_column).nil? && super
  end
end

[dedekm]

I've also ran into this, very confusing...

I'm not sure if this behaviour (force new slug when scoped column is changed) is bug or it is required for some reason?

we used this approach to fix this

def slug_candidates
  %i[slug to_label]
end