Remove todos when destroy project member and specs

3 files changed, 47 insertions, 2 deletions

diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index 8dae3bb2ef2..0ced9c31581 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -3,9 +3,9 @@ class ProjectMember < Member
 
   include Gitlab::ShellAdapter
 
+  has_many :todos, through: :user
   belongs_to :project, class_name: 'Project', foreign_key: 'source_id'
 
-
   # Make sure project member points only to project as it source
   default_value_for :source_type, SOURCE_TYPE
   validates_format_of :source_type, with: /\AProject\z/
@@ -15,6 +15,8 @@ class ProjectMember < Member
   scope :in_projects, ->(projects) { where(source_id: projects.pluck(:id)) }
   scope :with_user, ->(user) { where(user_id: user.id) }
 
+  before_destroy { todos.each(&:destroy) }
+
   class << self
 
     # Add users to project teams with passed access option
diff --git a/app/models/user.rb b/app/models/user.rb
index 70a966491eb..6a09b78455b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -77,7 +77,7 @@ class User < ActiveRecord::Base
   has_one  :abuse_report,             dependent: :destroy
   has_many :spam_logs,                dependent: :destroy
   has_many :builds,                   dependent: :nullify, class_name: 'Ci::Build'
-  has_many :todos, -> { joins("join members on members.source_id = todos.project_id and members.user_id = todos.user_id") }, dependent: :destroy
+  has_many :todos,                    dependent: :destroy
   has_many :notification_settings,    dependent: :destroy
 
   #
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index 9f26d9eb5ce..2d2fa91e001 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -20,6 +20,49 @@
 require 'spec_helper'
 
 describe ProjectMember, models: true do
+  describe 'associations' do
+    it { is_expected.to have_many(:todos).through(:user) }
+    it { is_expected.to belong_to(:project).class_name('Project').with_foreign_key(:source_id) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to allow_value('Project').for(:source_type) }
+    it { is_expected.not_to allow_value('project').for(:source_type) }
+  end
+
+  describe 'modules' do
+    it { is_expected.to include_module(Gitlab::ShellAdapter) }
+  end
+
+  describe "#destroy" do
+    let(:owner)   { create(:project_member, access_level: ProjectMember::OWNER) }
+    let(:project) { owner.project }
+    let(:master)  { create(:project_member, project: project) }
+
+    let(:owner_todos)  { (0...2).map { create(:todo, user: owner.user, project: project) } }
+    let(:master_todos) { (0...3).map { create(:todo, user: master.user, project: project) } }
+
+    before do
+      owner_todos
+      master_todos
+    end
+
+    it "destroy itself and delete associated todos" do
+      expect(owner.todos.size).to eq(2)
+      expect(master.todos.size).to eq(3)
+      expect(Todo.count).to eq(5)
+
+      master_todo_ids = master_todos.map(&:id)
+      master.destroy
+
+      expect(owner.todos.size).to eq(2)
+      expect(Todo.count).to eq(2)
+      master_todo_ids.each do |id|
+        expect(Todo.exists?(id)).to eq(false)
+      end
+    end
+  end
+
   describe :import_team do
     before do
       @abilities = Six.new