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

Inconsistent behavior for #parse_friendly_id #970

Open
YushengLi opened this issue Apr 8, 2021 · 2 comments
Open

Inconsistent behavior for #parse_friendly_id #970

YushengLi opened this issue Apr 8, 2021 · 2 comments
Labels

Comments

@YushengLi
Copy link

YushengLi commented Apr 8, 2021

Thanks for the great work in this project 🙇

We have opted-in the #first_by_friendly_id case insensitive change while upgrading to 5.4.0.

With 5.4.2 released, we hope to maintain the same behavior by override #parse_friendly_id in our model, but we have noticed same inconsistent behavior for the finders.

With the following model definition

class Product < ApplicationRecord
  extend FriendlyId
  friendly_id :name, use: %i[slugged finders]
  
  has_many :skus

  scope :not_deleted, -> { where(deleted_at: nil) }
  # ...
  
  def self.parse_friendly_id(value)
    super.downcase
  end
end

We expect the finder to work for the following cases

Product.find('FriendlyID') # => Finds product with #slug friendlyid
Product.includes(:skus).find('FriendlyID')
Product.not_deleted.includes(:skus).find('FriendlyID')

But instead, the first Product.find('FriendlyID') works without issue, yet the second and third statement did not call the correct #parse_friendly_id and returns exception: ActiveRecord::RecordNotFound: can't find record with friendly id: "FriendlyID"

Both Product.includes(:skus) and Product.not_deleted.includes(:skus) returns Product::ActiveRecord_Relation, so I am guessing that Product::ActiveRecord_Relation did not pick up #parse_friendly_id definition from Product model as expected

Strangely, if I called Product.not_deleted.includes(:skus).parse_friendly_id('FriendlyID'), it will return the expected result friendlyid, and if I called Product.not_deleted.includes(:skus).find('FriendlyID') followed by that, the query will return expected result.

We are using

  • FriendlyID: 5.4.2
  • Rails: 5.2.5
  • Ruby 2.7.0

I've also tried moving friendly id settings into initializer with no luck

FriendlyId.defaults do |config|
  config.use :reserved
  config.reserved_words = %w[ #... ]
  config.use :finders
  config.use :slugged
  config.use(Module.new {
    def parse_friendly_id(value)
      super.downcase
    end
  })
end
@parndt parndt added the pinned label Jun 22, 2021
@tbolender
Copy link

We want to opt-in for case-insensitive slug and stumbled upon the same behavior. We use slugs without a scope, so our model Organization with

class Organization < ApplicationRecord
  extend FriendlyId

  friendly_id :name, use: :slugged

  def self.parse_friendly_id(value)
    super.downcase
  end
end

yields

> Organization.friendly.find('customer')
=> #<Organization ...>
> Organization.friendly.find('Customer')
ActiveRecord::RecordNotFound (can't find record with friendly id: "Customer")

and

> Organization.friendly.parse_friendly_id('Customer')
NoMethodError (super: no superclass method `parse_friendly_id' for #<Class:0x00007f8259b0c588>)

We run on FriendlyID: 5.4.2, Rails: 5.2.5, and Ruby 2.5.9.

@phoet
Copy link

phoet commented Mar 24, 2023

in addition to that, the example in the documentation has an instance method, not a class method:

# ### Example
#
#     class Person < ActiveRecord::Base
#       extend FriendlyId
#       friendly_id :name_and_location
#
#       def name_and_location
#         "#{name} from #{location}"
#       end
#
#       # Use default slug, but lower case
#       # If `id` is "Jane-Doe" or "JANE-DOE", this finds data by "jane-doe"
#       def parse_friendly_id(slug)
#         super.downcase
#       end
#     end

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

No branches or pull requests

4 participants