From bd3aab45de0d9bf0725e18e24496d26a4051f060 Mon Sep 17 00:00:00 2001
From: Joe Sak <joe@joesak.com>
Date: Thu, 21 Jul 2022 18:34:39 -0500
Subject: [PATCH] Fix issues with `allow_nil: true` (#997)

Turns out, the documentation I added in #995 wasn't 100% accurate. I had
to add tests for such examples and improve the internal code to support
the option. This should now provide the fully expected behavior.

Co-authored-by: Philip Arndt <git@p.arndt.io>
---
 Changelog.md                      |  2 +-
 README.md                         |  2 +-
 lib/friendly_id/finder_methods.rb |  4 +++-
 test/finders_test.rb              | 36 +++++++++++++++++++++++++++++++
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/Changelog.md b/Changelog.md
index 07af279c..b7b502b1 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -7,7 +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)
+* Add: `allow_nil: true` to the Finder ([#995](https://github.com/norman/friendly_id/pull/995) and [#997](https://github.com/norman/friendly_id/pull/997))
 
 ## 5.4.2 (2021-01-07)
 
diff --git a/README.md b/README.md
index 3f0577e9..5ed9f9ab 100644
--- a/README.md
+++ b/README.md
@@ -116,7 +116,7 @@ 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 have a variable/param that's possibly nil
+MyModel.friendly.find(nil)        # maybe you have a variable/param that's potentially nil
 #=> raise ActiveRecord::RecordNotFound
 
 MyModel.friendly.find("bad-slug", allow_nil: true)
diff --git a/lib/friendly_id/finder_methods.rb b/lib/friendly_id/finder_methods.rb
index 46cd362d..a13272a7 100644
--- a/lib/friendly_id/finder_methods.rb
+++ b/lib/friendly_id/finder_methods.rb
@@ -32,6 +32,8 @@ def find(*args, allow_nil: false)
       return super(*args) if potential_primary_key?(id)
 
       raise_not_found_exception(id) unless allow_nil
+    rescue ActiveRecord::RecordNotFound => exception
+      raise exception unless allow_nil
     end
 
     # Returns true if a record with the given id exists.
@@ -45,7 +47,7 @@ def exists?(conditions = :none)
     # `find`.
     # @raise ActiveRecord::RecordNotFound
     def find_by_friendly_id(id)
-      first_by_friendly_id(id) or raise raise_not_found_exception(id)
+      first_by_friendly_id(id) or raise_not_found_exception(id)
     end
 
     def exists_by_friendly_id?(id)
diff --git a/test/finders_test.rb b/test/finders_test.rb
index d40c1626..14677e93 100644
--- a/test/finders_test.rb
+++ b/test/finders_test.rb
@@ -37,4 +37,40 @@ def model_class
       assert_nil model_class.existing.find("foo", allow_nil: true)
     end
   end
+
+  test "allows nil with a bad primary key ID and allow_nil: true" do
+    with_instance_of(model_class) do |record|
+      assert_nil model_class.find(0, allow_nil: true)
+    end
+  end
+
+  test "allows nil on relations with a bad primary key ID and allow_nil: true" do
+    with_instance_of(model_class) do |record|
+      assert_nil model_class.existing.find(0, allow_nil: true)
+    end
+  end
+
+  test "allows nil with a bad potential primary key ID and allow_nil: true" do
+    with_instance_of(model_class) do |record|
+      assert_nil model_class.find("0", allow_nil: true)
+    end
+  end
+
+  test "allows nil on relations with a bad potential primary key ID and allow_nil: true" do
+    with_instance_of(model_class) do |record|
+      assert_nil model_class.existing.find("0", allow_nil: true)
+    end
+  end
+
+  test "allows nil with nil ID and allow_nil: true" do
+    with_instance_of(model_class) do |record|
+      assert_nil model_class.find(nil, allow_nil: true)
+    end
+  end
+
+  test "allows nil on relations with nil ID and allow_nil: true" do
+    with_instance_of(model_class) do |record|
+      assert_nil model_class.existing.find(nil, allow_nil: true)
+    end
+  end
 end