From 1f3849b00fcfad54c4b0fe1e9a185d7bddda8f4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=F0=9F=99=88=20=20jacopo=20beschi=20=F0=9F=99=89?=
 <intrip@gmail.com>
Date: Wed, 14 Feb 2018 12:01:12 +0000
Subject: [PATCH] Resolve "Remove notification settings for groups and projects
 you were previously a member of"

---
 app/models/member.rb                          |  2 +-
 .../members/authorized_destroy_service.rb     |  1 +
 ...ve-notification-settings-left-projects.yml |  5 ++
 .../authorized_destroy_service_spec.rb        | 60 ++++++++++++++++---
 4 files changed, 59 insertions(+), 9 deletions(-)
 create mode 100644 changelogs/unreleased/42481-remove-notification-settings-left-projects.yml

diff --git a/app/models/member.rb b/app/models/member.rb
index c47145667b5..2d17795e62d 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -314,7 +314,7 @@ class Member < ActiveRecord::Base
   end
 
   def notification_setting
-    @notification_setting ||= user.notification_settings_for(source)
+    @notification_setting ||= user&.notification_settings_for(source)
   end
 
   def notifiable?(type, opts = {})
diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb
index de3a252d6c6..2e89f00dad8 100644
--- a/app/services/members/authorized_destroy_service.rb
+++ b/app/services/members/authorized_destroy_service.rb
@@ -11,6 +11,7 @@ module Members
 
       Member.transaction do
         unassign_issues_and_merge_requests(member) unless member.invite?
+        member.notification_setting&.destroy
 
         member.destroy
       end
diff --git a/changelogs/unreleased/42481-remove-notification-settings-left-projects.yml b/changelogs/unreleased/42481-remove-notification-settings-left-projects.yml
new file mode 100644
index 00000000000..ea99649131b
--- /dev/null
+++ b/changelogs/unreleased/42481-remove-notification-settings-left-projects.yml
@@ -0,0 +1,5 @@
+---
+title: Remove user notification settings for groups and projects when user leaves
+merge_request: 16906
+author: Jacopo Beschi @jacopo-beschi
+type: fixed
diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb
index 757c45708b9..9cf6f64a078 100644
--- a/spec/services/members/authorized_destroy_service_spec.rb
+++ b/spec/services/members/authorized_destroy_service_spec.rb
@@ -21,6 +21,15 @@ describe Members::AuthorizedDestroyService do
         .to change { Member.count }.from(3).to(2)
     end
 
+    it "doesn't destroy invited project member notification_settings" do
+      project.add_developer(member_user)
+
+      member = create :project_member, :invited, project: project
+
+      expect { described_class.new(member, member_user).execute }
+        .not_to change { NotificationSetting.count }
+    end
+
     it 'destroys invited group member' do
       group.add_developer(member_user)
 
@@ -29,38 +38,73 @@ describe Members::AuthorizedDestroyService do
       expect { described_class.new(member, member_user).execute }
         .to change { Member.count }.from(2).to(1)
     end
+
+    it "doesn't destroy invited group member notification_settings" do
+      group.add_developer(member_user)
+
+      member = create :group_member, :invited, group: group
+
+      expect { described_class.new(member, member_user).execute }
+        .not_to change { NotificationSetting.count }
+    end
+  end
+
+  context 'Requested user' do
+    it "doesn't destroy member notification_settings" do
+      member = create(:project_member, user: member_user, requested_at: Time.now)
+
+      expect { described_class.new(member, member_user).execute }
+        .not_to change { NotificationSetting.count }
+    end
   end
 
   context 'Group member' do
-    it "unassigns issues and merge requests" do
-      group.add_developer(member_user)
+    let(:member) { group.members.find_by(user_id: member_user.id) }
 
+    before do
+      group.add_developer(member_user)
+    end
+
+    it "unassigns issues and merge requests" do
       issue = create :issue, project: group_project, assignees: [member_user]
       create :issue, assignees: [member_user]
       merge_request = create :merge_request, target_project: group_project, source_project: group_project, assignee: member_user
       create :merge_request, target_project: project, source_project: project, assignee: member_user
 
-      member = group.members.find_by(user_id: member_user.id)
-
       expect { described_class.new(member, member_user).execute }
         .to change { number_of_assigned_issuables(member_user) }.from(4).to(2)
 
       expect(issue.reload.assignee_ids).to be_empty
       expect(merge_request.reload.assignee_id).to be_nil
     end
+
+    it 'destroys member notification_settings' do
+      group.add_developer(member_user)
+      member = group.members.find_by(user_id: member_user.id)
+
+      expect { described_class.new(member, member_user).execute }
+        .to change { member_user.notification_settings.count }.by(-1)
+    end
   end
 
   context 'Project member' do
-    it "unassigns issues and merge requests" do
-      project.add_developer(member_user)
+    let(:member) { project.members.find_by(user_id: member_user.id) }
 
+    before do
+      project.add_developer(member_user)
+    end
+
+    it "unassigns issues and merge requests" do
       create :issue, project: project, assignees: [member_user]
       create :merge_request, target_project: project, source_project: project, assignee: member_user
 
-      member = project.members.find_by(user_id: member_user.id)
-
       expect { described_class.new(member, member_user).execute }
         .to change { number_of_assigned_issuables(member_user) }.from(2).to(0)
     end
+
+    it 'destroys member notification_settings' do
+      expect { described_class.new(member, member_user).execute }
+        .to change { member_user.notification_settings.count }.by(-1)
+    end
   end
 end