diff --git a/app/models/group.rb b/app/models/group.rb index fa9217543a3f004e53abe7509b8b2fc9ae988c81..6c1231c54f72f6c79c929f84103dbb6c56b112c5 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -268,13 +268,13 @@ def members_with_parents end GroupMember - .active_without_invites + .active_without_invites_and_requests .where(source_id: source_ids) end def members_with_descendants GroupMember - .active_without_invites + .active_without_invites_and_requests .where(source_id: self_and_descendants.reorder(nil).select(:id)) end diff --git a/app/models/member.rb b/app/models/member.rb index 01a1affd74f2b93caeb72e30585a124d919c284c..ecfa689021611a2934c0ff4d09d79c922b699a76 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -53,7 +53,7 @@ class Member < ActiveRecord::Base end # Like active, but without invites. For when a User is required. - scope :active_without_invites, -> do + scope :active_without_invites_and_requests, -> do left_join_users .where(users: { state: 'active' }) .non_request diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 76f656a48bdda373ea56e41074c226c2d60eb663..9e955935d6e96606af8070386149e6ca69f7651d 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -231,9 +231,9 @@ def send_new_note_notifications(note) def new_access_request(member) return true unless member.notifiable?(:subscription) - recipients = member.source.members.active_without_invites.owners_and_masters + recipients = member.source.members.active_without_invites_and_requests.owners_and_masters if fallback_to_group_owners_masters?(recipients, member) - recipients = member.source.group.members.active_without_invites.owners_and_masters + recipients = member.source.group.members.active_without_invites_and_requests.owners_and_masters end recipients.each { |recipient| deliver_access_request_email(recipient, member) } diff --git a/ee/app/services/ee/notification_service.rb b/ee/app/services/ee/notification_service.rb index c04ec1343af3473a0dcf39dfdb32b3eb7514b4f4..0eac5afc6e09502d15d80045044fd21f5f46c4f0 100644 --- a/ee/app/services/ee/notification_service.rb +++ b/ee/app/services/ee/notification_service.rb @@ -24,10 +24,10 @@ def send_service_desk_notification(note) end def mirror_was_hard_failed(project) - recipients = project.members.active_without_invites.owners_and_masters + recipients = project.members.active_without_invites_and_requests.owners_and_masters - unless recipients.present? - recipients = project.group.members.active_without_invites.owners_and_masters + if recipients.empty? && project.group + recipients = project.group.members.active_without_invites_and_requests.owners_and_masters end recipients.each do |recipient| diff --git a/ee/changelogs/unreleased/5358-hard-failing-mirror-fails-for-personal-project-with-blocked-owners.yml b/ee/changelogs/unreleased/5358-hard-failing-mirror-fails-for-personal-project-with-blocked-owners.yml new file mode 100644 index 0000000000000000000000000000000000000000..9efd2dd96567e932b5d185641a474132fb48a528 --- /dev/null +++ b/ee/changelogs/unreleased/5358-hard-failing-mirror-fails-for-personal-project-with-blocked-owners.yml @@ -0,0 +1,5 @@ +--- +title: Hard failing a mirror no longer fails for a blocked user's personal project +merge_request: 5063 +author: +type: fixed diff --git a/ee/spec/services/ee/notification_service_spec.rb b/ee/spec/services/ee/notification_service_spec.rb index 947218db10990c43e912f11c6d7581470bb6b674..bd50e8bfb4a9e7cdbe70255efb0de2a4c7c12d1e 100644 --- a/ee/spec/services/ee/notification_service_spec.rb +++ b/ee/spec/services/ee/notification_service_spec.rb @@ -142,6 +142,34 @@ def self.it_should_not_email! subject.mirror_was_hard_failed(project) end + + context 'when owner is blocked' do + it 'does not send email' do + project = create(:project, :mirror, :import_hard_failed) + project.owner.block! + + expect(Notify).not_to receive(:mirror_was_hard_failed_email) + + subject.mirror_was_hard_failed(project) + end + + context 'when project belongs to group' do + it 'does not send email to the blocked owner' do + blocked_user = create(:user, :blocked) + + group = create(:group, :public) + group.add_owner(blocked_user) + group.add_owner(user) + + project = create(:project, :mirror, :import_hard_failed, namespace: group) + + expect(Notify).not_to receive(:mirror_was_hard_failed_email).with(project.id, blocked_user.id).and_call_original + expect(Notify).to receive(:mirror_was_hard_failed_email).with(project.id, user.id).and_call_original + + subject.mirror_was_hard_failed(project) + end + end + end end context 'when user is master' do