diff --git a/app/services/draft_notes/publish_service.rb b/app/services/draft_notes/publish_service.rb index 5baaf0895f380411706102edc5e7fbfe42385496..28f65540ea7c0490411012b1172a6f0562c986cb 100644 --- a/app/services/draft_notes/publish_service.rb +++ b/app/services/draft_notes/publish_service.rb @@ -49,6 +49,7 @@ def publish_draft_notes(executing_user) keep_around_commits(created_notes) draft_notes.delete_all notification_service.async.new_review(review) + todo_service.new_review(review, current_user) MergeRequests::ResolvedDiscussionNotificationService.new(project: project, current_user: current_user).execute(merge_request) GraphqlTriggers.merge_request_merge_status_updated(merge_request) after_publish diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 7b9092a1a5ff0b613741c70ddbc5364e7c0ff315..6a3452c6330e68009b70c0c789898a40d8d93251 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -187,6 +187,14 @@ def ssh_key_expired(ssh_keys) create_ssh_key_todos(ssh_keys, ::Todo::SSH_KEY_EXPIRED) end + # When a merge request receives a review + # + # * Mark all outstanding todos on this MR for the current user as done + # + def new_review(review, current_user) + resolve_todos_for_target(review.merge_request, current_user) + end + # When user marks a target as todo def mark_todo(target, current_user) project = target.project diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb index b78d66e7db3ec789b36c17a1099c7458026f5781..1d400e3a8d87ca7c26bf1317cd3966b090f95558 100644 --- a/spec/services/draft_notes/publish_service_spec.rb +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -142,6 +142,14 @@ def publish(draft: nil) publish end + it 'resolves todos for the MR' do + expect_any_instance_of(TodoService) do |todo_service| + expect(todo_service).to receive(:new_review).with(kind_of(Review), user) + end + + publish + end + it 'tracks the publish event' do expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) .to receive(:track_publish_review_action) @@ -243,7 +251,7 @@ def publish(draft: nil) recorder = ActiveRecord::QueryRecorder.new(skip_cached: false) { publish } - expect(recorder.count).not_to be > 112 + expect(recorder.count).not_to be > 116 end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 5a385a43f0ed63ba88de9cb94366ed6116567a2b..04e552aa12fb809a12652d2acf00ec3caa83773e 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -1285,6 +1285,32 @@ should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED) end end + + describe '#new_review' do + it 'marks related pending todos to the target MR for the user as done' do + first_todo = create(:todo, :pending, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author) + second_todo = create(:todo, :pending, :review_requested, user: john_doe, project: project, target: mentioned_mr, author: author) + third_todo = create(:todo, :pending, :mentioned, user: john_doe, project: project, target: mentioned_mr, author: author) + + review = Review.new(merge_request: mentioned_mr) + service.new_review(review, john_doe) + + expect(first_todo.reload).to be_done + expect(second_todo.reload).to be_done + expect(third_todo.reload).to be_done + end + + it 'marks related pending todo to the target MR for the user as done when the multiple_todos feature is off' do + stub_feature_flags(multiple_todos: false) + + only_todo = create(:todo, :pending, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author) + + review = Review.new(merge_request: mentioned_mr) + service.new_review(review, john_doe) + + expect(only_todo.reload).to be_done + end + end end describe 'Designs' do