diff --git a/CHANGELOG-EE b/CHANGELOG-EE index df4928920cc1e4b222433d1f6a9794e748dcdd19..1d606dafff6e02d55225c0fc5a84d7d3067d7135 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -1,3 +1,6 @@ +v 7.14 + - Don't send "Added to group" notifications when group is LDAP synched + v 7.13.2 - Fix group web hook @@ -180,4 +183,4 @@ v 6.2.0 - Use omniauth-ldap nickname attribute as GitLab username - Improve group sharing UI for installation with many groups - Fix empty LDAP group raises exception - - Respect LDAP user filter for git access \ No newline at end of file + - Respect LDAP user filter for git access diff --git a/app/models/group.rb b/app/models/group.rb index aab9fb3b0c01c226a0132c41d2e84c1cd2b5b1cf..57eab28f3c9879f5b93d8cbc638893b1d3e7d801 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -64,18 +64,18 @@ class Group < Namespace @owners ||= group_members.owners.map(&:user) end - def add_users(user_ids, access_level, current_user = nil) + def add_users(user_ids, access_level, current_user = nil, skip_notification: false) user_ids.each do |user_id| - Member.add_user(self.group_members, user_id, access_level, current_user) + Member.add_user(self.group_members, user_id, access_level, current_user, skip_notification: skip_notification) end end - def add_user(user, access_level, current_user = nil) - add_users([user], access_level, current_user) + def add_user(user, access_level, current_user = nil, skip_notification: false) + add_users([user], access_level, current_user, skip_notification: skip_notification) end - def add_owner(user, current_user = nil) - self.add_user(user, Gitlab::Access::OWNER, current_user) + def add_owner(user, current_user = nil, skip_notification: false) + self.add_user(user, Gitlab::Access::OWNER, current_user, skip_notification: skip_notification) end def has_owner?(user) diff --git a/app/models/member.rb b/app/models/member.rb index cae8caa23fb2770299ee935b82f48edecd22f39b..c9cd984de7caec785f7eeb0f2f2d7b53b28fe824 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -23,6 +23,7 @@ class Member < ActiveRecord::Base include Gitlab::Access attr_accessor :raw_invite_token + attr_accessor :skip_notification belongs_to :created_by, class_name: "User" belongs_to :user @@ -71,7 +72,7 @@ class Member < ActiveRecord::Base user end - def add_user(members, user_id, access_level, current_user = nil) + def add_user(members, user_id, access_level, current_user = nil, skip_notification: false) user = user_for_id(user_id) # `user` can be either a User object or an email to be invited @@ -85,6 +86,8 @@ class Member < ActiveRecord::Base member.created_by ||= current_user member.access_level = access_level + member.skip_notification = skip_notification + member.save end end diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index fbd00af578461bd412d8f854fa2a96cdfdd23988..a6bae8e9716873681eef3e464df2af722b472e32 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -47,33 +47,33 @@ class GroupMember < Member private def send_invite - notification_service.invite_group_member(self, @raw_invite_token) + notification_service.invite_group_member(self, @raw_invite_token) unless @skip_notification super end def post_create_hook - notification_service.new_group_member(self) + notification_service.new_group_member(self) unless @skip_notification super end def post_update_hook if access_level_changed? - notification_service.update_group_member(self) + notification_service.update_group_member(self) unless @skip_notification end super end def after_accept_invite - notification_service.accept_group_invite(self) + notification_service.accept_group_invite(self) unless @skip_notification super end def after_decline_invite - notification_service.decline_group_invite(self) + notification_service.decline_group_invite(self) unless @skip_notification super end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 1b0c76917aa377d47abb47c2be4716fe5c8a9976..0ed43b60d1dea304ddc1145b387af2fe4d76bac2 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -123,7 +123,7 @@ class ProjectMember < Member private def send_invite - notification_service.invite_project_member(self, @raw_invite_token) + notification_service.invite_project_member(self, @raw_invite_token) unless @skip_notification super end @@ -131,7 +131,7 @@ class ProjectMember < Member def post_create_hook unless owner? event_service.join_project(self.project, self.user) - notification_service.new_project_member(self) + notification_service.new_project_member(self) unless @skip_notification end super @@ -139,7 +139,7 @@ class ProjectMember < Member def post_update_hook if access_level_changed? - notification_service.update_project_member(self) + notification_service.update_project_member(self) unless @skip_notification end super @@ -152,13 +152,13 @@ class ProjectMember < Member end def after_accept_invite - notification_service.accept_project_invite(self) + notification_service.accept_project_invite(self) unless @skip_notification super end def after_decline_invite - notification_service.decline_project_invite(self) + notification_service.decline_project_invite(self) unless @skip_notification super end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index e66e4f597cdee8b1af2da774ea730918b949ba0a..cc8b14cc5babe736edddf273c720abc85e257d45 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -140,7 +140,7 @@ module Gitlab active_group_links = group.ldap_group_links.where(cn: cns_with_access) if active_group_links.any? - group.add_users([user.id], fetch_group_access(group, user, active_group_links)) + group.add_users([user.id], fetch_group_access(group, user, active_group_links), skip_notification: true) elsif group.last_owner?(user) Rails.logger.warn "#{self.class.name}: LDAP group sync cannot remove #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner" else diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index ed092b412543c6851e48d086b4e6ad817cb36512..901f2bf4667a8915c323ce5dc0532503ee8a1ae8 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -258,6 +258,11 @@ objectclass: posixGroup access.update_ldap_group_links expect( gitlab_group_1.has_master?(user) ).to be_truthy end + + it "doesn't send a notification email" do + expect { access.update_ldap_group_links }.not_to \ + change { ActionMailer::Base.deliveries } + end end context "existing access as guest for group-1, allowed via ldap-group1 as DEVELOPER" do @@ -271,6 +276,11 @@ objectclass: posixGroup expect { access.update_ldap_group_links }.to \ change{ gitlab_group_1.has_master?(user) }.from(false).to(true) end + + it "doesn't send a notification email" do + expect { access.update_ldap_group_links }.not_to \ + change { ActionMailer::Base.deliveries } + end end context "existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER" do @@ -284,6 +294,11 @@ objectclass: posixGroup expect { access.update_ldap_group_links }.not_to \ change{ gitlab_group_1.has_master?(user) } end + + it "doesn't send a notification email" do + expect { access.update_ldap_group_links }.not_to \ + change { ActionMailer::Base.deliveries } + end end context "existing access as master for group-1, not allowed" do