From fe4d3abead763d627379bc348797dae81dfd5c17 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Wed, 9 Mar 2016 19:22:43 +0000
Subject: [PATCH] Merge branch 'fix-todos' into 'master'

Fix error 500 in Todos

Closes #14095

Closes #14075

Closes #14109

Closes #14151

See merge request !3141
---
 CHANGELOG                                 |  3 ++-
 app/helpers/application_helper.rb         |  2 +-
 app/models/project.rb                     |  1 +
 app/views/dashboard/todos/_todo.html.haml |  9 ++++++---
 db/migrate/20160309140734_fix_todos.rb    | 16 ++++++++++++++++
 db/schema.rb                              |  2 +-
 spec/models/project_spec.rb               |  1 +
 7 files changed, 28 insertions(+), 6 deletions(-)
 create mode 100644 db/migrate/20160309140734_fix_todos.rb

diff --git a/CHANGELOG b/CHANGELOG
index f14c613192..64ed97e0aa 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,7 +14,8 @@ v 8.6.0 (unreleased)
   - Increase the notes polling timeout over time (Roberto Dip)
 
 v 8.5.5
-  - No changes
+  - Ensure removing a project removes associated Todo entries.
+  - Prevent a 500 error in Todos when author was removed.
 
 v 8.5.4
   - Do not cache requests for badges (including builds badge)
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index f0aa2b5712..368969c647 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -72,7 +72,7 @@ module ApplicationHelper
     if user_or_email.is_a?(User)
       user = user_or_email
     else
-      user = User.find_by(email: user_or_email.downcase)
+      user = User.find_by(email: user_or_email.try(:downcase))
     end
 
     if user
diff --git a/app/models/project.rb b/app/models/project.rb
index 13679cbd9b..8d98ccdf47 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -151,6 +151,7 @@ class Project < ActiveRecord::Base
   has_many :releases, dependent: :destroy
   has_many :lfs_objects_projects, dependent: :destroy
   has_many :lfs_objects, through: :lfs_objects_projects
+  has_many :todos, dependent: :destroy
 
   has_one :import_data, dependent: :destroy, class_name: "ProjectImportData"
 
diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml
index 6975f6ed0d..f38caf1826 100644
--- a/app/views/dashboard/todos/_todo.html.haml
+++ b/app/views/dashboard/todos/_todo.html.haml
@@ -3,9 +3,12 @@
     = image_tag avatar_icon(todo.author_email, 40), class: 'avatar s40', alt:''
 
     .todo-title
-      %span.author_name
-        = link_to_author todo
-      %span.todo_label
+      %span.author-name
+        - if todo.author
+          = link_to_author(todo)
+        - else
+          (removed)
+      %span.todo-label
         = todo_action_name(todo)
         = todo_target_link(todo)
 
diff --git a/db/migrate/20160309140734_fix_todos.rb b/db/migrate/20160309140734_fix_todos.rb
new file mode 100644
index 0000000000..ebe0fc8230
--- /dev/null
+++ b/db/migrate/20160309140734_fix_todos.rb
@@ -0,0 +1,16 @@
+class FixTodos < ActiveRecord::Migration
+ def up
+    execute <<-SQL
+      DELETE FROM todos
+      WHERE todos.target_type IN ('Commit', 'ProjectSnippet')
+         OR NOT EXISTS (
+              SELECT *
+              FROM projects
+              WHERE projects.id = todos.project_id
+            )
+    SQL
+ end
+
+ def down
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 53a941d30d..3c839db8db 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: 20160222153918) do
+ActiveRecord::Schema.define(version: 20160309140734) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f9842d23af..012be3e2df 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -68,6 +68,7 @@ describe Project, models: true do
     it { is_expected.to have_many(:runners) }
     it { is_expected.to have_many(:variables) }
     it { is_expected.to have_many(:triggers) }
+    it { is_expected.to have_many(:todos).dependent(:destroy) }
   end
 
   describe 'modules' do