diff options
| -rw-r--r-- | app/services/notification_service.rb | 12 | ||||
| -rw-r--r-- | spec/services/issues/update_service_spec.rb | 7 | ||||
| -rw-r--r-- | spec/services/notification_service_spec.rb | 88 |
3 files changed, 105 insertions, 2 deletions
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 3bdf00a8291..eff0d96f93d 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -162,6 +162,7 @@ class NotificationService recipients = add_subscribed_users(recipients, note.noteable) recipients = reject_unsubscribed_users(recipients, note.noteable) + recipients = reject_users_without_access(recipients, note.noteable) recipients.delete(note.author) recipients = recipients.uniq @@ -376,6 +377,14 @@ class NotificationService end end + def reject_users_without_access(recipients, target) + return recipients unless target.is_a?(Issue) + + recipients.select do |user| + user.can?(:read_issue, target) + end + end + def add_subscribed_users(recipients, target) return recipients unless target.respond_to? :subscribers @@ -464,15 +473,16 @@ class NotificationService end recipients = reject_unsubscribed_users(recipients, target) + recipients = reject_users_without_access(recipients, target) recipients.delete(current_user) - recipients.uniq end def build_relabeled_recipients(target, current_user, labels:) recipients = add_labels_subscribers([], target, labels: labels) recipients = reject_unsubscribed_users(recipients, target) + recipients = reject_users_without_access(recipients, target) recipients.delete(current_user) recipients.uniq end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 4ffe753fef5..6b214a0d96b 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -151,7 +151,12 @@ describe Issues::UpdateService, services: true do context 'when the issue is relabeled' do let!(:non_subscriber) { create(:user) } - let!(:subscriber) { create(:user).tap { |u| label.toggle_subscription(u) } } + let!(:subscriber) do + create(:user).tap do |u| + label.toggle_subscription(u) + project.team << [u, :developer] + end + end it 'sends notifications for subscribers of newly added labels' do opts = { label_ids: [label.id] } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index b5407397c1d..0f2aa3ae73c 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -111,6 +111,33 @@ describe NotificationService, services: true do end end + context 'confidential issue note' do + let(:project) { create(:empty_project, :public) } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") } + + it 'filters out users that can not read the issue' do + project.team << [member, :developer] + + expect(SentNotification).to receive(:record).with(confidential_issue, any_args).exactly(4).times + + ActionMailer::Base.deliveries.clear + + notification.new_note(note) + + should_not_email(non_member) + should_email(author) + should_email(assignee) + should_email(member) + should_email(admin) + end + end + context 'issue note mention' do let(:project) { create(:empty_project, :public) } let(:issue) { create(:issue, project: project, assignee: create(:user)) } @@ -233,6 +260,36 @@ describe NotificationService, services: true do should_email(subscriber) end + + context 'confidential issues' do + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) } + + it "emails subscribers of the issue's labels that can read the issue" do + project.team << [member, :developer] + + label = create(:label, issues: [confidential_issue]) + label.toggle_subscription(non_member) + label.toggle_subscription(author) + label.toggle_subscription(assignee) + label.toggle_subscription(member) + label.toggle_subscription(admin) + + ActionMailer::Base.deliveries.clear + + notification.new_issue(confidential_issue, @u_disabled) + + should_not_email(non_member) + should_not_email(author) + should_email(assignee) + should_email(member) + should_email(admin) + end + end end describe :reassigned_issue do @@ -332,6 +389,37 @@ describe NotificationService, services: true do should_not_email(subscriber_to_label) should_email(subscriber_to_label2) end + + context 'confidential issues' do + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:admin) { create(:admin) } + let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) } + let!(:label_1) { create(:label, issues: [confidential_issue]) } + let!(:label_2) { create(:label) } + + it "emails subscribers of the issue's labels that can read the issue" do + project.team << [member, :developer] + + label_2.toggle_subscription(non_member) + label_2.toggle_subscription(author) + label_2.toggle_subscription(assignee) + label_2.toggle_subscription(member) + label_2.toggle_subscription(admin) + + ActionMailer::Base.deliveries.clear + + notification.relabeled_issue(confidential_issue, [label_2], @u_disabled) + + should_not_email(non_member) + should_email(author) + should_email(assignee) + should_email(member) + should_email(admin) + end + end end describe :close_issue do |