Merge branch '42481-remove-notification-settings-left-projects' into 'master'

Resolve "Remove notification settings for groups and projects you were previously a member of"

Closes #42481

See merge request gitlab-org/gitlab-ce!16906

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 = {})

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

changelogs/unreleased/42481-remove-notification-settings-left-projects.yml

+---
+title: Remove user notification settings for groups and projects when user leaves
+merge_request: 16906
+author: Jacopo Beschi @jacopo-beschi
+type: fixed

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
+    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
+    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