Refactor AutocompleteControllerdefine-abstraction-levels

This refactors the AutocompleteController according to the guidelines
and boundaries discussed in
https://gitlab.com/gitlab-org/gitlab-ce/issues/49653. Specifically,
ActiveRecord logic is moved to different finders, which are then used in
the controller. View logic in turn is moved to presenters, instead of
directly using ActiveRecord's "to_json" method.

The finder MoveToProjectFinder is also adjusted according to the
abstraction guidelines and boundaries, resulting in a much more simple
finder.

By using finders (and other abstractions) more actively, we can push a
lot of logic out of the controller. We also remove the need for various
"before_action" hooks, though this could be achieved without using
finders as well.

The various finders related to AutcompleteController have also been
moved into a namespace. This removes the need for calling everything
"AutocompleteSmurfFinder", instead you can use
"Autocomplete::SmurfFinder".

4 files changed, 244 insertions, 10 deletions

diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb
index b909e04dfc3..3f52091698c 100644
--- a/spec/models/award_emoji_spec.rb
+++ b/spec/models/award_emoji_spec.rb
@@ -77,4 +77,27 @@ describe AwardEmoji do
       end
     end
   end
+
+  describe '.award_counts_for_user' do
+    let(:user) { create(:user) }
+
+    before do
+      create(:award_emoji, user: user, name: 'thumbsup')
+      create(:award_emoji, user: user, name: 'thumbsup')
+      create(:award_emoji, user: user, name: 'thumbsdown')
+      create(:award_emoji, user: user, name: '+1')
+    end
+
+    it 'returns the awarded emoji in descending order' do
+      awards = described_class.award_counts_for_user(user)
+
+      expect(awards).to eq('thumbsup' => 2, 'thumbsdown' => 1, '+1' => 1)
+    end
+
+    it 'limits the returned number of rows' do
+      awards = described_class.award_counts_for_user(user, 1)
+
+      expect(awards).to eq('thumbsup' => 2)
+    end
+  end
 end
diff --git a/spec/models/concerns/optionally_search_spec.rb b/spec/models/concerns/optionally_search_spec.rb
new file mode 100644
index 00000000000..ff4212ddf18
--- /dev/null
+++ b/spec/models/concerns/optionally_search_spec.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe OptionallySearch do
+  let(:model) do
+    Class.new(ActiveRecord::Base) do
+      self.table_name = 'users'
+
+      include OptionallySearch
+    end
+  end
+
+  describe '.search' do
+    it 'raises NotImplementedError' do
+      expect { model.search('foo') }.to raise_error(NotImplementedError)
+    end
+  end
+
+  describe '.optionally_search' do
+    context 'when a query is given' do
+      it 'delegates to the search method' do
+        expect(model)
+          .to receive(:search)
+          .with('foo')
+
+        model.optionally_search('foo')
+      end
+    end
+
+    context 'when no query is given' do
+      it 'returns the current relation' do
+        expect(model.optionally_search).to be_a_kind_of(ActiveRecord::Relation)
+      end
+    end
+
+    context 'when an empty query is given' do
+      it 'returns the current relation' do
+        expect(model.optionally_search(''))
+          .to be_a_kind_of(ActiveRecord::Relation)
+      end
+    end
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index d8a5e5f6869..56c07f5793b 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1478,6 +1478,53 @@ describe Project do
     end
   end
 
+  describe '.optionally_search' do
+    let(:project) { create(:project) }
+
+    it 'searches for projects matching the query if one is given' do
+      relation = described_class.optionally_search(project.name)
+
+      expect(relation).to eq([project])
+    end
+
+    it 'returns the current relation if no search query is given' do
+      relation = described_class.where(id: project.id)
+
+      expect(relation.optionally_search).to eq(relation)
+    end
+  end
+
+  describe '.paginate_in_descending_order_using_id' do
+    let!(:project1) { create(:project) }
+    let!(:project2) { create(:project) }
+
+    it 'orders the relation in descending order' do
+      expect(described_class.paginate_in_descending_order_using_id)
+        .to eq([project2, project1])
+    end
+
+    it 'applies a limit to the relation' do
+      expect(described_class.paginate_in_descending_order_using_id(limit: 1))
+        .to eq([project2])
+    end
+
+    it 'limits projects by and ID when given' do
+      expect(described_class.paginate_in_descending_order_using_id(before: project2.id))
+        .to eq([project1])
+    end
+  end
+
+  describe '.including_namespace_and_owner' do
+    it 'eager loads the namespace and namespace owner' do
+      create(:project)
+
+      row = described_class.eager_load_namespace_and_owner.to_a.first
+      recorder = ActiveRecord::QueryRecorder.new { row.namespace.owner }
+
+      expect(recorder.count).to be_zero
+    end
+  end
+
   describe '#expire_caches_before_rename' do
     let(:project) { create(:project, :repository) }
     let(:repo)    { double(:repo, exists?: true) }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index f5e2c977104..9763477a711 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -346,17 +346,55 @@ describe User do
       end
     end
 
-    describe '.todo_authors' do
-      it 'filters users' do
-        create :user
-        user_2 = create :user
-        user_3 = create :user
-        current_user = create :user
-        create(:todo, user: current_user, author: user_2, state: :done)
-        create(:todo, user: current_user, author: user_3, state: :pending)
+    describe '.limit_to_todo_authors' do
+      context 'when filtering by todo authors' do
+        let(:user1) { create(:user) }
+        let(:user2) { create(:user) }
 
-        expect(described_class.todo_authors(current_user.id, 'pending')).to eq [user_3]
-        expect(described_class.todo_authors(current_user.id, 'done')).to eq [user_2]
+        before do
+          create(:todo, user: user1, author: user1, state: :done)
+          create(:todo, user: user2, author: user2, state: :pending)
+        end
+
+        it 'only returns users that have authored todos' do
+          users = described_class.limit_to_todo_authors(
+            user: user2,
+            with_todos: true,
+            todo_state: :pending
+          )
+
+          expect(users).to eq([user2])
+        end
+
+        it 'ignores users that do not have a todo in the matching state' do
+          users = described_class.limit_to_todo_authors(
+            user: user1,
+            with_todos: true,
+            todo_state: :pending
+          )
+
+          expect(users).to be_empty
+        end
+      end
+
+      context 'when not filtering by todo authors' do
+        it 'returns the input relation' do
+          user1 = create(:user)
+          user2 = create(:user)
+          rel = described_class.limit_to_todo_authors(user: user1)
+
+          expect(rel).to include(user1, user2)
+        end
+      end
+
+      context 'when no user is provided' do
+        it 'returns the input relation' do
+          user1 = create(:user)
+          user2 = create(:user)
+          rel = described_class.limit_to_todo_authors
+
+          expect(rel).to include(user1, user2)
+        end
       end
     end
   end
@@ -2901,4 +2939,86 @@ describe User do
       let(:uploader_class) { AttachmentUploader }
     end
   end
+
+  describe '.union_with_user' do
+    context 'when no user ID is provided' do
+      it 'returns the input relation' do
+        user = create(:user)
+
+        expect(described_class.union_with_user).to eq([user])
+      end
+    end
+
+    context 'when a user ID is provided' do
+      it 'includes the user object in the returned relation' do
+        user1 = create(:user)
+        user2 = create(:user)
+        users = described_class.where(id: user1.id).union_with_user(user2.id)
+
+        expect(users).to include(user1)
+        expect(users).to include(user2)
+      end
+
+      it 'does not re-apply any WHERE conditions on the outer query' do
+        relation = described_class.where(id: 1).union_with_user(2)
+
+        expect(relation.arel.where_sql).to be_nil
+      end
+    end
+  end
+
+  describe '.optionally_search' do
+    context 'using nil as the argument' do
+      it 'returns the current relation' do
+        user = create(:user)
+
+        expect(described_class.optionally_search).to eq([user])
+      end
+    end
+
+    context 'using an empty String as the argument' do
+      it 'returns the current relation' do
+        user = create(:user)
+
+        expect(described_class.optionally_search('')).to eq([user])
+      end
+    end
+
+    context 'using a non-empty String' do
+      it 'returns users matching the search query' do
+        user1 = create(:user)
+        create(:user)
+
+        expect(described_class.optionally_search(user1.name)).to eq([user1])
+      end
+    end
+  end
+
+  describe '.where_not_in' do
+    context 'without an argument' do
+      it 'returns the current relation' do
+        user = create(:user)
+
+        expect(described_class.where_not_in).to eq([user])
+      end
+    end
+
+    context 'using a list of user IDs' do
+      it 'excludes the users from the returned relation' do
+        user1 = create(:user)
+        user2 = create(:user)
+
+        expect(described_class.where_not_in([user2.id])).to eq([user1])
+      end
+    end
+  end
+
+  describe '.reorder_by_name' do
+    it 'reorders the input relation' do
+      user1 = create(:user, name: 'A')
+      user2 = create(:user, name: 'B')
+
+      expect(described_class.reorder_by_name).to eq([user1, user2])
+    end
+  end
 end