Merge branch 'fix-todos' into 'master'

Destroy related todos when an Issue/MR is deleted

Closes #14550 

Closes #14598

See merge request !3376

7 files changed, 45 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 313a584d1d0..f95a243b0a1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,10 @@ v 8.7.0 (unreleased)
   - Gracefully handle notes on deleted commits in merge requests (Stan Hu)
   - Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla)
 
+v 8.6.3 (unreleased)
+  - Destroy related todos when an Issue/MR is deleted. !3376
+  - Fix error 500 when target is nil on todo list. !3376
+
 v 8.6.2
   - Fix dropdown alignment. !3298
   - Fix issuable sidebar overlaps on tablet. !3299
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index cf5b2c71675..47ac22995ab 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -19,6 +19,7 @@ module Issuable
     has_many :notes, as: :noteable, dependent: :destroy
     has_many :label_links, as: :target, dependent: :destroy
     has_many :labels, through: :label_links
+    has_many :todos, as: :target, dependent: :destroy
 
     validates :author, presence: true
     validates :title, presence: true, length: { within: 0..255 }
diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml
index 25d2b64e381..aa0aff86d4d 100644
--- a/app/views/dashboard/todos/_todo.html.haml
+++ b/app/views/dashboard/todos/_todo.html.haml
@@ -10,7 +10,10 @@
           (removed)
       %span.todo-label
         = todo_action_name(todo)
-        = todo_target_link(todo)
+        - if todo.target
+          = todo_target_link(todo)
+        - else
+          (removed)
 
       · #{time_ago_with_tooltip(todo.created_at)}
 
diff --git a/db/migrate/20160324020319_remove_todos_for_deleted_issues.rb b/db/migrate/20160324020319_remove_todos_for_deleted_issues.rb
new file mode 100644
index 00000000000..1fff9759d1e
--- /dev/null
+++ b/db/migrate/20160324020319_remove_todos_for_deleted_issues.rb
@@ -0,0 +1,17 @@
+class RemoveTodosForDeletedIssues < ActiveRecord::Migration
+  def up
+    execute <<-SQL
+      DELETE FROM todos
+      WHERE todos.target_type = 'Issue'
+        AND NOT EXISTS (
+              SELECT *
+              FROM issues
+              WHERE issues.id = todos.target_id
+                AND issues.deleted_at IS NULL
+            )
+    SQL
+  end
+
+  def down
+  end
+end
diff --git a/db/migrate/20160331133914_remove_todos_for_deleted_merge_requests.rb b/db/migrate/20160331133914_remove_todos_for_deleted_merge_requests.rb
new file mode 100644
index 00000000000..54cea964ff2
--- /dev/null
+++ b/db/migrate/20160331133914_remove_todos_for_deleted_merge_requests.rb
@@ -0,0 +1,17 @@
+class RemoveTodosForDeletedMergeRequests < ActiveRecord::Migration
+  def up
+    execute <<-SQL
+      DELETE FROM todos
+      WHERE todos.target_type = 'MergeRequest'
+        AND NOT EXISTS (
+              SELECT *
+              FROM merge_requests
+              WHERE merge_requests.id = todos.target_id
+                AND merge_requests.deleted_at IS NULL
+            )
+    SQL
+  end
+
+  def down
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index dce2bfe62ca..90eaff1a4dd 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160320204112) do
+ActiveRecord::Schema.define(version: 20160331133914) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index be29b6d66ff..b16ccc6e305 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -9,6 +9,7 @@ describe Issue, "Issuable" do
     it { is_expected.to belong_to(:author) }
     it { is_expected.to belong_to(:assignee) }
     it { is_expected.to have_many(:notes).dependent(:destroy) }
+    it { is_expected.to have_many(:todos).dependent(:destroy) }
   end
 
   describe "Validation" do