Merge branch 'fix/do-not-add-todo-when-build-allowed-to-fail' into 'master' 24710-external-statuses-pipeline

Do not create TODO when build is allowed to fail

## What does this MR do?

Do not create a TODO when build that failed is allowed to fail.

## Does this MR meet the acceptance criteria?

- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- Tests
  - [x] Added for this feature/bug
  - [ ] All builds are passing

## What are the relevant issue numbers?

Closes #22280

See merge request !7618

5 files changed, 47 insertions, 9 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 5d2e7d94190..83c0c64e5fb 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -70,7 +70,11 @@ module Ci
           environment: build.environment,
           status_event: 'enqueue'
         )
-        MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build)
+
+        MergeRequests::AddTodoWhenBuildFailsService
+          .new(build.project, nil)
+          .close(new_build)
+
         build.pipeline.mark_as_processable_after_stage(build.stage_idx)
         new_build
       end
diff --git a/app/services/merge_requests/add_todo_when_build_fails_service.rb b/app/services/merge_requests/add_todo_when_build_fails_service.rb
index d572a928a42..12a8415d9a5 100644
--- a/app/services/merge_requests/add_todo_when_build_fails_service.rb
+++ b/app/services/merge_requests/add_todo_when_build_fails_service.rb
@@ -1,13 +1,18 @@
 module MergeRequests
   class AddTodoWhenBuildFailsService < MergeRequests::BaseService
     # Adds a todo to the parent merge_request when a CI build fails
+    #
     def execute(commit_status)
+      return if commit_status.allow_failure?
+
       commit_status_merge_requests(commit_status) do |merge_request|
         todo_service.merge_request_build_failed(merge_request)
       end
     end
 
-    # Closes any pending build failed todos for the parent MRs when a build is retried
+    # Closes any pending build failed todos for the parent MRs when a
+    # build is retried
+    #
     def close(commit_status)
       commit_status_merge_requests(commit_status) do |merge_request|
         todo_service.merge_request_build_retried(merge_request)
diff --git a/changelogs/unreleased/fix-do-not-add-todo-when-build-allowed-to-fail.yml b/changelogs/unreleased/fix-do-not-add-todo-when-build-allowed-to-fail.yml
new file mode 100644
index 00000000000..6402d0c7ece
--- /dev/null
+++ b/changelogs/unreleased/fix-do-not-add-todo-when-build-allowed-to-fail.yml
@@ -0,0 +1,4 @@
+---
+title: Do not create a new TODO when failed build is allowed to fail
+merge_request: 7618
+author: 
diff --git a/doc/workflow/todos.md b/doc/workflow/todos.md
index a50ba305deb..54e7ae19ea5 100644
--- a/doc/workflow/todos.md
+++ b/doc/workflow/todos.md
@@ -24,9 +24,11 @@ you still have open.
 
 A Todo appears in your Todos dashboard when:
 
-- an issue or merge request is assigned to you
+- an issue or merge request is assigned to you,
 - you are `@mentioned` in an issue or merge request, be it the description of
-  the issue/merge request or in a comment
+  the issue/merge request or in a comment,
+- build in the CI pipeline running for your merge request failed, but this
+  build is not allowed to fail.
 
 >**Note:** Commenting on a commit will _not_ trigger a Todo.
 
diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
index dd656c3bbb7..a44312dd363 100644
--- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
+++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
@@ -1,13 +1,22 @@
 require 'spec_helper'
 
-# Write specs in this file.
 describe MergeRequests::AddTodoWhenBuildFailsService do
   let(:user) { create(:user) }
   let(:merge_request) { create(:merge_request) }
   let(:project) { create(:project) }
   let(:sha) { '1234567890abcdef1234567890abcdef12345678' }
-  let(:pipeline) { create(:ci_pipeline_with_one_job, ref: merge_request.source_branch, project: project, sha: sha) }
-  let(:service) { MergeRequests::AddTodoWhenBuildFailsService.new(project, user, commit_message: 'Awesome message') }
+  let(:ref) { merge_request.source_branch }
+
+  let(:pipeline) do
+    create(:ci_pipeline_with_one_job, ref: ref,
+                                      project: project,
+                                      sha: sha)
+  end
+
+  let(:service) do
+    described_class.new(project, user, commit_message: 'Awesome message')
+  end
+
   let(:todo_service) { TodoService.new }
 
   let(:merge_request) do
@@ -23,7 +32,9 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
 
   describe '#execute' do
     context 'commit status with ref' do
-      let(:commit_status) { create(:generic_commit_status, ref: merge_request.source_branch, pipeline: pipeline) }
+      let(:commit_status) do
+        create(:generic_commit_status, ref: ref, pipeline: pipeline)
+      end
 
       it 'notifies the todo service' do
         expect(todo_service).to receive(:merge_request_build_failed).with(merge_request)
@@ -32,7 +43,7 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
     end
 
     context 'commit status with non-HEAD ref' do
-      let(:commit_status) { create(:generic_commit_status, ref: merge_request.source_branch) }
+      let(:commit_status) { create(:generic_commit_status, ref: ref) }
 
       it 'does not notify the todo service' do
         expect(todo_service).not_to receive(:merge_request_build_failed)
@@ -48,6 +59,18 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
         service.execute(commit_status)
       end
     end
+
+    context 'when commit status is a build allowed to fail' do
+      let(:commit_status) do
+        create(:ci_build, :allowed_to_fail, ref: ref, pipeline: pipeline)
+      end
+
+      it 'does not create todo' do
+        expect(todo_service).not_to receive(:merge_request_build_failed)
+
+        service.execute(commit_status)
+      end
+    end
   end
 
   describe '#close' do