From 40624b4007a7e552739785cecf8180a85dce4734 Mon Sep 17 00:00:00 2001 From: Joe Sak Date: Mon, 4 Jul 2022 14:17:50 -0500 Subject: [PATCH 1/8] Add raise: false or allow_nil: true option to the finder --- lib/friendly_id/finder_methods.rb | 8 ++++++-- test/finders_test.rb | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/friendly_id/finder_methods.rb b/lib/friendly_id/finder_methods.rb index 2a058973..018bffc9 100644 --- a/lib/friendly_id/finder_methods.rb +++ b/lib/friendly_id/finder_methods.rb @@ -13,13 +13,17 @@ module FinderMethods # # If you want to search only by the friendly id, use {#find_by_friendly_id}. # @raise ActiveRecord::RecordNotFound - def find(*args) + def find(*args, **options) id = args.first return super if args.count != 1 || id.unfriendly_id? first_by_friendly_id(id).tap { |result| return result unless result.nil? } return super if potential_primary_key?(id) - raise_not_found_exception(id) + options.symbolize_keys! + allow_nil = options.fetch(:allow_nil, false) + raise_exception = options.fetch(:raise, !allow_nil) + + raise_not_found_exception(id) if raise_exception end # Returns true if a record with the given id exists. diff --git a/test/finders_test.rb b/test/finders_test.rb index 9271f6ef..34363274 100644 --- a/test/finders_test.rb +++ b/test/finders_test.rb @@ -25,4 +25,28 @@ def model_class assert model_class.existing.find(record.friendly_id) end end + + test "allows nil with raise: false" do + with_instance_of(model_class) do |record| + assert_nil model_class.find("foo", raise: false) + end + end + + test "allows nil with allow_nil: true" do + with_instance_of(model_class) do |record| + assert_nil model_class.find("foo", allow_nil: true) + end + end + + test "allows nil on relations with raise: false" do + with_instance_of(model_class) do |record| + assert_nil model_class.existing.find("foo", raise: false) + end + end + + test "allows nil on relations with allow_nil: true" do + with_instance_of(model_class) do |record| + assert_nil model_class.existing.find("foo", allow_nil: true) + end + end end From 4e4333ca61024b4f09c40d9c2370546246c6f8a9 Mon Sep 17 00:00:00 2001 From: Joe Sak Date: Thu, 14 Jul 2022 18:11:57 -0500 Subject: [PATCH 2/8] Remove the raise: option, fix ruby 2.6 issue --- lib/friendly_id/finder_methods.rb | 7 +++---- test/finders_test.rb | 12 ------------ 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/lib/friendly_id/finder_methods.rb b/lib/friendly_id/finder_methods.rb index 018bffc9..d5be90ca 100644 --- a/lib/friendly_id/finder_methods.rb +++ b/lib/friendly_id/finder_methods.rb @@ -15,15 +15,14 @@ module FinderMethods # @raise ActiveRecord::RecordNotFound def find(*args, **options) id = args.first - return super if args.count != 1 || id.unfriendly_id? + return super(*args) if args.count != 1 || id.unfriendly_id? first_by_friendly_id(id).tap { |result| return result unless result.nil? } - return super if potential_primary_key?(id) + return super(*args) if potential_primary_key?(id) options.symbolize_keys! allow_nil = options.fetch(:allow_nil, false) - raise_exception = options.fetch(:raise, !allow_nil) - raise_not_found_exception(id) if raise_exception + raise_not_found_exception(id) unless allow_nil end # Returns true if a record with the given id exists. diff --git a/test/finders_test.rb b/test/finders_test.rb index 34363274..d40c1626 100644 --- a/test/finders_test.rb +++ b/test/finders_test.rb @@ -26,24 +26,12 @@ def model_class end end - test "allows nil with raise: false" do - with_instance_of(model_class) do |record| - assert_nil model_class.find("foo", raise: false) - end - end - test "allows nil with allow_nil: true" do with_instance_of(model_class) do |record| assert_nil model_class.find("foo", allow_nil: true) end end - test "allows nil on relations with raise: false" do - with_instance_of(model_class) do |record| - assert_nil model_class.existing.find("foo", raise: false) - end - end - test "allows nil on relations with allow_nil: true" do with_instance_of(model_class) do |record| assert_nil model_class.existing.find("foo", allow_nil: true) From 0ebc04899c88b2bb9cb9657a0e439a3e6dfa8a66 Mon Sep 17 00:00:00 2001 From: Joe Sak Date: Thu, 14 Jul 2022 18:14:11 -0500 Subject: [PATCH 3/8] Inline it --- lib/friendly_id/finder_methods.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/friendly_id/finder_methods.rb b/lib/friendly_id/finder_methods.rb index d5be90ca..f76b7010 100644 --- a/lib/friendly_id/finder_methods.rb +++ b/lib/friendly_id/finder_methods.rb @@ -19,10 +19,9 @@ def find(*args, **options) first_by_friendly_id(id).tap { |result| return result unless result.nil? } return super(*args) if potential_primary_key?(id) - options.symbolize_keys! - allow_nil = options.fetch(:allow_nil, false) - - raise_not_found_exception(id) unless allow_nil + unless options.symbolize_keys.fetch(:allow_nil, false) + raise_not_found_exception(id) + end end # Returns true if a record with the given id exists. From 31d10175de85de75e52d926ed08703592cc88541 Mon Sep 17 00:00:00 2001 From: Joe Sak Date: Thu, 14 Jul 2022 18:26:24 -0500 Subject: [PATCH 4/8] Improve the argument and add the docs --- README.md | 21 +++++++++++++++++++++ lib/friendly_id/finder_methods.rb | 18 ++++++++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 25707731..f0cdd00a 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,27 @@ existing users, do this from the console, runner, or add a Rake task: User.find_each(&:save) ``` +## Options + +### :allow_nil + +You can pass `allow_nil: true` to the `friendly.find()` method if you prefer to get +a nil instead of raising an `ActiveRecord::RecordNotFound` exception. + +#### Example + +```ruby +MyModel.friendly.find("bad-slug") # where bad-slug is not a valid slug +MyModel.friendly.find(123) # where 123 is not a valid primary key ID +MyModel.friendly.find(nil) # when you don't know if you even have a slug or ID +#=> raise ActiveRecord::RecordNotFound + +MyModel.friendly.find("bad-slug") # where bad-slug is not a valid slug +MyModel.friendly.find(123) # where 123 is not a valid primary key ID +MyModel.friendly.find(nil) # when you don't know if you even have a slug or ID +#=> nil +``` + ## Bugs Please report them on the [Github issue diff --git a/lib/friendly_id/finder_methods.rb b/lib/friendly_id/finder_methods.rb index f76b7010..46cd362d 100644 --- a/lib/friendly_id/finder_methods.rb +++ b/lib/friendly_id/finder_methods.rb @@ -7,21 +7,31 @@ module FinderMethods # id matching '123' and then fall back to looking for a record with the # numeric id '123'. # + # @param [Boolean] allow_nil (default: false) + # Use allow_nil: true if you'd like the finder to return nil instead of + # raising ActivRecord::RecordNotFound + # + # ### Example + # + # MyModel.friendly.find("bad-slug") + # #=> raise ActiveRecord::RecordNotFound + # + # MyModel.friendly.find("bad-slug", allow_nil: true) + # #=> nil + # # Since FriendlyId 5.0, if the id is a nonnumeric string like '123-foo' it # will *only* search by friendly id and not fall back to the regular find # method. # # If you want to search only by the friendly id, use {#find_by_friendly_id}. # @raise ActiveRecord::RecordNotFound - def find(*args, **options) + def find(*args, allow_nil: false) id = args.first return super(*args) if args.count != 1 || id.unfriendly_id? first_by_friendly_id(id).tap { |result| return result unless result.nil? } return super(*args) if potential_primary_key?(id) - unless options.symbolize_keys.fetch(:allow_nil, false) - raise_not_found_exception(id) - end + raise_not_found_exception(id) unless allow_nil end # Returns true if a record with the given id exists. From 70d9af2affb0e27bdfaf5d6b6323473906751d9f Mon Sep 17 00:00:00 2001 From: Joe Sak Date: Thu, 14 Jul 2022 18:29:03 -0500 Subject: [PATCH 5/8] Update the readme --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index f0cdd00a..61f0d0e4 100644 --- a/README.md +++ b/README.md @@ -108,8 +108,8 @@ User.find_each(&:save) ### :allow_nil -You can pass `allow_nil: true` to the `friendly.find()` method if you prefer to get -a nil instead of raising an `ActiveRecord::RecordNotFound` exception. +You can pass `allow_nil: true` to the `friendly.find()` method if you're want to +avoid raising `ActiveRecord::RecordNotFound` and accept a `nil`. #### Example @@ -119,9 +119,9 @@ MyModel.friendly.find(123) # where 123 is not a valid primary key ID MyModel.friendly.find(nil) # when you don't know if you even have a slug or ID #=> raise ActiveRecord::RecordNotFound -MyModel.friendly.find("bad-slug") # where bad-slug is not a valid slug -MyModel.friendly.find(123) # where 123 is not a valid primary key ID -MyModel.friendly.find(nil) # when you don't know if you even have a slug or ID +MyModel.friendly.find("bad-slug", allow_nil: true) # where bad-slug is not a valid slug +MyModel.friendly.find(123, allow_nil: true) # where 123 is not a valid primary key ID +MyModel.friendly.find(nil, allow_nil: true) # when you don't know if you even have a slug or ID #=> nil ``` From d146f4513e19fa5c1b725a0f2705ed4ebd509a39 Mon Sep 17 00:00:00 2001 From: Joe Sak Date: Thu, 14 Jul 2022 18:30:19 -0500 Subject: [PATCH 6/8] Update readme --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 61f0d0e4..374d10d0 100644 --- a/README.md +++ b/README.md @@ -116,12 +116,12 @@ avoid raising `ActiveRecord::RecordNotFound` and accept a `nil`. ```ruby MyModel.friendly.find("bad-slug") # where bad-slug is not a valid slug MyModel.friendly.find(123) # where 123 is not a valid primary key ID -MyModel.friendly.find(nil) # when you don't know if you even have a slug or ID +MyModel.friendly.find(nil) # when you have a variable/param that's possibly nil #=> raise ActiveRecord::RecordNotFound -MyModel.friendly.find("bad-slug", allow_nil: true) # where bad-slug is not a valid slug -MyModel.friendly.find(123, allow_nil: true) # where 123 is not a valid primary key ID -MyModel.friendly.find(nil, allow_nil: true) # when you don't know if you even have a slug or ID +MyModel.friendly.find("bad-slug", allow_nil: true) +MyModel.friendly.find(123, allow_nil: true) +MyModel.friendly.find(nil, allow_nil: true) #=> nil ``` From c46bb5fb074ef83a09535167a1b7da97ab1e6f29 Mon Sep 17 00:00:00 2001 From: Joe Sak Date: Thu, 14 Jul 2022 18:30:35 -0500 Subject: [PATCH 7/8] Update README.md Co-authored-by: Philip Arndt --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 374d10d0..3f0577e9 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ User.find_each(&:save) ## Options -### :allow_nil +### `:allow_nil` You can pass `allow_nil: true` to the `friendly.find()` method if you're want to avoid raising `ActiveRecord::RecordNotFound` and accept a `nil`. From 4bac55fd7eb17b3fc6f03312b5972368d8b64330 Mon Sep 17 00:00:00 2001 From: Joe Sak Date: Thu, 14 Jul 2022 18:33:39 -0500 Subject: [PATCH 8/8] Update the changelog --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 6f1d6ee0..07af279c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ suggestions, ideas and improvements to FriendlyId. * SimpleI18n: Handle regional locales ([#965](https://github.com/norman/friendly_id/pull/965)) * Fix: "unknown column" exception ([#943](https://github.com/norman/friendly_id/pull/943)) +* Add: `allow_nil: true` to the Finder ([#995])(https://github.com/norman/friendly_id/pull/995) ## 5.4.2 (2021-01-07)