Merge branch '31914-graphql-todo-query-support-multiple-query' into 'master'

Enable support for multiple field query in GraphQL Todo API

See merge request gitlab-org/gitlab!19576

app/graphql/resolvers/todo_resolver.rb

@@ -38,53 +38,15 @@ module Resolvers
 
     private
 
-    # TODO: Support multiple queries for e.g. state and type on TodosFinder:
-    #
-    # https://gitlab.com/gitlab-org/gitlab/merge_requests/18487
-    # https://gitlab.com/gitlab-org/gitlab/merge_requests/18518
-    #
-    # As soon as these MR's are merged, we can refactor this to query by
-    # multiple contents.
-    #
     def todo_finder_params(args)
       {
-        state: first_state(args),
-        type: first_type(args),
-        group_id: first_group_id(args),
-        author_id: first_author_id(args),
-        action_id: first_action(args),
-        project_id: first_project(args)
+        state: args[:state],
+        type: args[:type],
+        group_id: args[:group_id],
+        author_id: args[:author_id],
+        action_id: args[:action],
+        project_id: args[:project_id]
       }
     end
-
-    def first_project(args)
-      first_query_field(args, :project_id)
-    end
-
-    def first_action(args)
-      first_query_field(args, :action)
-    end
-
-    def first_author_id(args)
-      first_query_field(args, :author_id)
-    end
-
-    def first_group_id(args)
-      first_query_field(args, :group_id)
-    end
-
-    def first_state(args)
-      first_query_field(args, :state)
-    end
-
-    def first_type(args)
-      first_query_field(args, :type)
-    end
-
-    def first_query_field(query, field)
-      return unless query.key?(field)
-
-      query[field].first if query[field].respond_to?(:first)
-    end
   end
 end

changelogs/unreleased/31914-graphql-todo-query-support-multiple-query.yml

+---
+title: Enable support for multiple content query in GraphQL Todo API
+merge_request: 19576
+author:
+type: changed

spec/graphql/resolvers/todo_resolver_spec.rb

@@ -7,13 +7,12 @@ describe Resolvers::TodoResolver do
 
   describe '#resolve' do
     let_it_be(:current_user) { create(:user) }
-    let_it_be(:user) { create(:user) }
     let_it_be(:author1) { create(:user) }
     let_it_be(:author2) { create(:user) }
 
-    let_it_be(:todo1) { create(:todo, user: user, target_type: 'MergeRequest', state: :pending, action: Todo::MENTIONED, author: author1) }
-    let_it_be(:todo2) { create(:todo, user: user, state: :done, action: Todo::ASSIGNED, author: author2) }
-    let_it_be(:todo3) { create(:todo, user: user, state: :pending, action: Todo::ASSIGNED, author: author1) }
+    let_it_be(:todo1) { create(:todo, user: current_user, target_type: 'MergeRequest', state: :pending, action: Todo::MENTIONED, author: author1) }
+    let_it_be(:todo2) { create(:todo, user: current_user, state: :done, action: Todo::ASSIGNED, author: author2) }
+    let_it_be(:todo3) { create(:todo, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1) }
 
     it 'calls TodosFinder' do
       expect_next_instance_of(TodosFinder) do |finder|
@@ -25,68 +24,71 @@ describe Resolvers::TodoResolver do
 
     context 'when using no filter' do
       it 'returns expected todos' do
-        todos = resolve(described_class, obj: user, args: {}, ctx: { current_user: user })
-
-        expect(todos).to contain_exactly(todo1, todo3)
+        expect(resolve_todos).to contain_exactly(todo1, todo3)
       end
     end
 
     context 'when using filters' do
-      # TODO These can be removed as soon as we support filtering for multiple field contents for todos
-
-      it 'just uses the first state' do
-        todos = resolve(described_class, obj: user, args: { state: [:done, :pending] }, ctx: { current_user: user })
+      it 'returns the todos for multiple states' do
+        todos = resolve_todos({ state: [:done, :pending] })
 
-        expect(todos).to contain_exactly(todo2)
+        expect(todos).to contain_exactly(todo1, todo2, todo3)
       end
 
-      it 'just uses the first action' do
-        todos = resolve(described_class, obj: user, args: { action: [Todo::MENTIONED, Todo::ASSIGNED] }, ctx: { current_user: user })
+      it 'returns the todos for multiple types' do
+        todos = resolve_todos({ type: %w[Issue MergeRequest] })
 
-        expect(todos).to contain_exactly(todo1)
+        expect(todos).to contain_exactly(todo1, todo3)
       end
 
-      it 'just uses the first author id' do
-        # We need a pending todo for now because of TodosFinder's state query
-        todo4 = create(:todo, user: user, state: :pending, action: Todo::ASSIGNED, author: author2)
+      it 'returns the todos for multiple groups' do
+        group1 = create(:group)
+        group2 = create(:group)
+        group3 = create(:group)
+
+        group1.add_developer(current_user)
+        group2.add_developer(current_user)
 
-        todos = resolve(described_class, obj: user, args: { author_id: [author2.id, author1.id] }, ctx: { current_user: user })
+        todo4 = create(:todo, group: group1, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1)
+        todo5 = create(:todo, group: group2, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1)
+        create(:todo, group: group3, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1)
 
-        expect(todos).to contain_exactly(todo4)
+        todos = resolve_todos({ group_id: [group2.id, group1.id] })
+
+        expect(todos).to contain_exactly(todo4, todo5)
       end
 
-      it 'just uses the first project id' do
-        project1 = create(:project)
-        project2 = create(:project)
+      it 'returns the todos for multiple authors' do
+        author3 = create(:user)
 
-        create(:todo, project: project1, user: user, state: :pending, action: Todo::ASSIGNED, author: author1)
-        todo5 = create(:todo, project: project2, user: user, state: :pending, action: Todo::ASSIGNED, author: author1)
+        todo4 = create(:todo, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author2)
+        create(:todo, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author3)
 
-        todos = resolve(described_class, obj: user, args: { project_id: [project2.id, project1.id] }, ctx: { current_user: user })
+        todos = resolve_todos({ author_id: [author2.id, author1.id] })
 
-        expect(todos).to contain_exactly(todo5)
+        expect(todos).to contain_exactly(todo1, todo3, todo4)
       end
 
-      it 'just uses the first group id' do
-        group1 = create(:group)
-        group2 = create(:group)
+      it 'returns the todos for multiple actions' do
+        create(:todo, user: current_user, state: :pending, action: Todo::DIRECTLY_ADDRESSED, author: author1)
 
-        group1.add_developer(user)
-        group2.add_developer(user)
+        todos = resolve_todos({ action: [Todo::MENTIONED, Todo::ASSIGNED] })
 
-        create(:todo, group: group1, user: user, state: :pending, action: Todo::ASSIGNED, author: author1)
-        todo5 = create(:todo, group: group2, user: user, state: :pending, action: Todo::ASSIGNED, author: author1)
+        expect(todos).to contain_exactly(todo1, todo3)
+      end
 
-        todos = resolve(described_class, obj: user, args: { group_id: [group2.id, group1.id] }, ctx: { current_user: user })
+      it 'returns the todos for multiple projects' do
+        project1 = create(:project)
+        project2 = create(:project)
+        project3 = create(:project)
 
-        expect(todos).to contain_exactly(todo5)
-      end
+        todo4 = create(:todo, project: project1, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1)
+        todo5 = create(:todo, project: project2, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1)
+        create(:todo, project: project3, user: current_user, state: :pending, action: Todo::ASSIGNED, author: author1)
 
-      it 'just uses the first target' do
-        todos = resolve(described_class, obj: user, args: { type: %w[Issue MergeRequest] }, ctx: { current_user: user })
+        todos = resolve_todos({ project_id: [project2.id, project1.id] })
 
-        # Just todo3 because todo2 is in state "done"
-        expect(todos).to contain_exactly(todo3)
+        expect(todos).to contain_exactly(todo4, todo5)
       end
     end
 
@@ -100,7 +102,9 @@ describe Resolvers::TodoResolver do
 
     context 'when provided user is not current user' do
       it 'returns no todos' do
-        todos = resolve(described_class, obj: user, args: {}, ctx: { current_user: current_user })
+        other_user = create(:user)
+
+        todos = resolve(described_class, obj: other_user, args: {}, ctx: { current_user: current_user })
 
         expect(todos).to be_empty
       end