Merge branch '28355-multiple-todos-backend' into 'master'

Allow multiple todos behind a feature flag

See merge request gitlab-org/gitlab!47629

app/services/todo_service.rb

@@ -216,7 +216,7 @@ class TodoService
 
   def create_todos(users, attributes)
     Array(users).map do |user|
-      next if pending_todos(user, attributes).exists?
+      next if pending_todos(user, attributes).exists? && Feature.disabled?(:multiple_todos, user)
 
       issue_type = attributes.delete(:issue_type)
       track_todo_creation(user, issue_type)
@@ -278,7 +278,7 @@ class TodoService
     create_todos(directly_addressed_users, attributes)
 
     # Create Todos for mentioned users
-    mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users)
+    mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users + directly_addressed_users)
     attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note)
     create_todos(mentioned_users, attributes)
   end

config/feature_flags/development/multiple_todos.yml

+---
+name: multiple_todos
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47629
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/28355
+milestone: '13.8'
+type: development
+group: group::project management
+default_enabled: false

ee/spec/requests/api/todos_spec.rb

@@ -88,6 +88,8 @@ RSpec.describe API::Todos do
       end
 
       it 'returns 304 there already exist a todo on that epic' do
+        stub_feature_flags(multiple_todos: false)
+
         create(:todo, project: nil, group: group, user: user, target: epic)
 
         subject

ee/spec/services/todo_service_spec.rb

@@ -3,13 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe TodoService do
-  let(:author) { create(:user, username: 'author') }
-  let(:non_member) { create(:user, username: 'non_member') }
-  let(:member) { create(:user, username: 'member') }
-  let(:guest) { create(:user, username: 'guest') }
-  let(:admin) { create(:admin, username: 'administrator') }
-  let(:john_doe) { create(:user, username: 'john_doe') }
-  let(:skipped) { create(:user, username: 'skipped') }
+  let_it_be(:author) { create(:user, username: 'author') }
+  let_it_be(:non_member) { create(:user, username: 'non_member') }
+  let_it_be(:member) { create(:user, username: 'member') }
+  let_it_be(:guest) { create(:user, username: 'guest') }
+  let_it_be(:admin) { create(:admin, username: 'administrator') }
+  let_it_be(:john_doe) { create(:user, username: 'john_doe') }
+  let_it_be(:skipped) { create(:user, username: 'skipped') }
   let(:skip_users) { [skipped] }
   let(:service) { described_class.new }
 
@@ -21,21 +21,23 @@ RSpec.describe TodoService do
     let(:description_mentions) { "- [ ] Task 1\n- [ ] Task 2 FYI: #{mentions}" }
     let(:description_directly_addressed) { "#{mentions}\n- [ ] Task 1\n- [ ] Task 2" }
 
-    let(:group) { create(:group) }
+    let_it_be(:group, reload: true) { create(:group) }
     let(:epic) { create(:epic, group: group, author: author, description: description_mentions) }
 
     let(:todos_for) { [] }
     let(:todos_not_for) { [] }
     let(:target) { epic }
 
-    before do
-      stub_licensed_features(epics: true)
-
+    before_all do
       group.add_guest(guest)
       group.add_developer(author)
       group.add_developer(member)
     end
 
+    before do
+      stub_licensed_features(epics: true)
+    end
+
     shared_examples_for 'todos creation' do
       it 'creates todos for users mentioned' do
         if todos_for.count > 0
@@ -169,19 +171,19 @@ RSpec.describe TodoService do
       end
 
       describe '#new_note' do
-        let!(:first_todo) do
-          create(:todo, :assigned,
-            user: john_doe, project: nil, group: group, target: epic, author: author)
-        end
-
-        let!(:second_todo) do
-          create(:todo, :assigned,
-            user: john_doe, project: nil, group: group, target: epic, author: author)
-        end
-
         let(:note) { create(:note, noteable: epic, project: nil, author: john_doe, note: mentions) }
 
         context 'when a note is created for an epic' do
+          let!(:first_todo) do
+            create(:todo, :assigned,
+              user: john_doe, project: nil, group: group, target: epic, author: author)
+          end
+
+          let!(:second_todo) do
+            create(:todo, :assigned,
+              user: john_doe, project: nil, group: group, target: epic, author: author)
+          end
+
           it 'marks pending epic todos for the note author as done' do
             service.new_note(note, john_doe)
 
@@ -189,7 +191,7 @@ RSpec.describe TodoService do
             expect(second_todo.reload).to be_done
           end
 
-          it 'does not marka pending epic todos for the note author as done for system notes' do
+          it 'does not mark pending epic todos for the note author as done for system notes' do
             system_note = create(:system_note, noteable: epic)
 
             service.new_note(system_note, john_doe)
@@ -208,8 +210,8 @@ RSpec.describe TodoService do
             end
 
             let(:todo_params) { { action: Todo::MENTIONED } }
-            let(:todos_for) { [author, non_member, member, guest, admin, skipped] }
-            let(:todos_not_for) { [john_doe] }
+            let(:todos_for) { users }
+            let(:todos_not_for) { [] }
 
             include_examples 'todos creation'
           end
@@ -220,8 +222,8 @@ RSpec.describe TodoService do
             end
 
             let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
-            let(:todos_for) { [author, non_member, member, guest, admin, skipped] }
-            let(:todos_not_for) { [john_doe] }
+            let(:todos_for) { users }
+            let(:todos_not_for) { [] }
 
             include_examples 'todos creation'
           end
@@ -290,7 +292,6 @@ RSpec.describe TodoService do
 
           # for each valid mentioned user
           should_create_todo(user: john_doe, target: merge_request, action: Todo::MENTIONED)
-          should_not_create_todo(user: approver_1, target: merge_request, action: Todo::MENTIONED)
 
           # skip for code owner
           should_not_create_todo(user: code_owner, target: merge_request, action: Todo::APPROVAL_REQUIRED)

spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb

@@ -42,6 +42,8 @@ RSpec.describe 'Creating a todo for the alert' do
 
   context 'todo already exists' do
     before do
+      stub_feature_flags(multiple_todos: false)
+
       create(:todo, :pending, project: project, user: user, target: alert)
     end
 

spec/requests/api/todos_spec.rb

@@ -302,6 +302,8 @@ RSpec.describe API::Todos do
     end
 
     it 'returns 304 there already exist a todo on that issuable' do
+      stub_feature_flags(multiple_todos: false)
+
       create(:todo, project: project_1, author: author_1, user: john_doe, target: issuable)
 
       post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", john_doe)

spec/services/alert_management/alerts/todo/create_service_spec.rb

@@ -58,6 +58,10 @@ RSpec.describe AlertManagement::Alerts::Todo::CreateService do
             create(:todo, :pending, **todo_params)
           end
 
+          before do
+            stub_feature_flags(multiple_todos: false)
+          end
+
           it 'does not create a todo' do
             expect { result }.not_to change { Todo.count }
           end