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

@@ -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)
 

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)

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)

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