Respect finder's filter in FinderMethods#find

Rails' `#find_by!(id: id)` and `find(id)` behave the same.

This wasn't true for FinderMethods#find_by! and FinderMethods#find
since the latter did not `execute` the complete query ignoring potential
filters.

No user-facing changes expected.

app/finders/concerns/finder_methods.rb

@@ -13,16 +13,23 @@ module FinderMethods
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  # rubocop: disable CodeReuse/ActiveRecord
   def find(*args)
-    raise_not_found_unless_authorized model.find(*args)
+    raise_not_found_unless_authorized execute.reorder(nil).find(*args)
   end
+  # rubocop: enable CodeReuse/ActiveRecord
 
   private
 
   def raise_not_found_unless_authorized(result)
     result = if_authorized(result)
 
-    raise(ActiveRecord::RecordNotFound, "Couldn't find #{model}") unless result
+    unless result
+      # This fetches the model from the `ActiveRecord::Relation` but does not
+      # actually execute the query.
+      model = execute.model
+      raise ActiveRecord::RecordNotFound, "Couldn't find #{model}"
+    end
 
     result
   end
@@ -32,11 +39,7 @@ module FinderMethods
     # this is currently the case in the `MilestoneFinder`
     return result unless respond_to?(:current_user, true)
 
-    if can_read_object?(result)
-      result
-    else
-      nil
-    end
+    result if can_read_object?(result)
   end
 
   def can_read_object?(object)
@@ -53,10 +56,4 @@ module FinderMethods
     # Not all objects define `#to_ability_name`, so attempt to derive it:
     object.model_name.singular
   end
-
-  # This fetches the model from the `ActiveRecord::Relation` but does not
-  # actually execute the query.
-  def model
-    execute.model
-  end
 end

spec/finders/concerns/finder_methods_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe FinderMethods do
       end
 
       def execute
-        Project.all.order(id: :desc)
+        Project.where.not(name: 'foo').order(id: :desc)
       end
 
       private
@@ -23,12 +23,14 @@ RSpec.describe FinderMethods do
 
   let_it_be(:user) { create(:user) }
   let_it_be(:authorized_project) { create(:project) }
+  let_it_be(:unmatched_project) { create(:project, name: 'foo') }
   let_it_be(:unauthorized_project) { create(:project) }
 
   subject(:finder) { finder_class.new(user) }
 
   before_all do
     authorized_project.add_developer(user)
+    unmatched_project.add_developer(user)
   end
 
   # rubocop:disable Rails/FindById
@@ -37,10 +39,14 @@ RSpec.describe FinderMethods do
       expect(finder.find_by!(id: authorized_project.id)).to eq(authorized_project)
     end
 
-    it 'raises not found when the project is not found' do
+    it 'raises not found when the project is not found by id' do
       expect { finder.find_by!(id: non_existing_record_id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
 
+    it 'raises not found when the project is not found by filter' do
+      expect { finder.find_by!(id: unmatched_project.id) }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
     it 'raises not found the user does not have access' do
       expect { finder.find_by!(id: unauthorized_project.id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
@@ -62,13 +68,27 @@ RSpec.describe FinderMethods do
       expect(finder.find(authorized_project.id)).to eq(authorized_project)
     end
 
-    it 'raises not found when the project is not found' do
+    it 'raises not found when the project is not found by id' do
       expect { finder.find(non_existing_record_id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
 
+    it 'raises not found when the project is not found by filter' do
+      expect { finder.find(unmatched_project.id) }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
     it 'raises not found the user does not have access' do
       expect { finder.find(unauthorized_project.id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
+
+    it 'ignores ordering' do
+      # Memoise the finder result so we can add message expectations to it
+      relation = finder.execute
+      allow(finder).to receive(:execute).and_return(relation)
+
+      expect(relation).to receive(:reorder).with(nil).and_call_original
+
+      finder.find(authorized_project.id)
+    end
   end
 
   describe '#find_by' do
@@ -76,10 +96,14 @@ RSpec.describe FinderMethods do
       expect(finder.find_by(id: authorized_project.id)).to eq(authorized_project)
     end
 
-    it 'returns nil when the project is not found' do
+    it 'returns nil when the project is not found by id' do
       expect(finder.find_by(id: non_existing_record_id)).to be_nil
     end
 
+    it 'returns nil when the project is not found by filter' do
+      expect(finder.find_by(id: unmatched_project.id)).to be_nil
+    end
+
     it 'returns nil when the user does not have access' do
       expect(finder.find_by(id: unauthorized_project.id)).to be_nil
     end