Merge branch 'fix-todos' into 'master'

Does not create a Todo when commenting on commit or project snippet

Closes #13733

See merge request !2964

4 files changed, 15 insertions, 3 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f3370f8c3bc..cb95cdef042 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -22,6 +22,9 @@ v 8.5.1
   - Update sentry-raven gem to 0.15.6
   - Add build coverage in project's builds page (Steffen Köhler)
 
+v 8.5.2
+  - Fix error 500 when commenting on a commit
+
 v 8.5.0
   - Fix duplicate "me" in tooltip of the "thumbsup" awards Emoji (Stan Hu)
   - Cache various Repository methods to improve performance (Yorick Peterse)
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index b970439b921..2bb312bb252 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -13,6 +13,5 @@ module Notes
 
       note
     end
-
   end
 end
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index dc270602ebc..4392e2d17fe 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -130,8 +130,8 @@ class TodoService
   end
 
   def handle_note(note, author)
-    # Skip system notes, like status changes and cross-references
-    return if note.system
+    # Skip system notes, notes on commit, and notes on project snippet
+    return if note.system? || ['Commit', 'Snippet'].include?(note.noteable_type)
 
     project = note.project
     target  = note.noteable
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index df3aa955f24..96420acb31d 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -110,6 +110,8 @@ describe TodoService, services: true do
       let!(:first_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) }
       let!(:second_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) }
       let(:note) { create(:note, project: project, noteable: issue, author: john_doe, note: mentions) }
+      let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) }
+      let(:note_on_project_snippet) { create(:note_on_project_snippet, project: project, author: john_doe, note: mentions) }
       let(:award_note) { create(:note, :award, project: project, noteable: issue, author: john_doe, note: 'thumbsup') }
       let(:system_note) { create(:system_note, project: project, noteable: issue) }
 
@@ -145,6 +147,14 @@ describe TodoService, services: true do
         should_not_create_todo(user: john_doe, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_not_create_todo(user: stranger, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
       end
+
+      it 'does not create todo when leaving a note on commit' do
+        should_not_create_any_todo { service.new_note(note_on_commit, john_doe) }
+      end
+
+      it 'does not create todo when leaving a note on snippet' do
+        should_not_create_any_todo { service.new_note(note_on_project_snippet, john_doe) }
+      end
     end
   end