Commit 3b39ce32 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'notifications-for-subscribers-confidential-issue-labels' into 'master'

Restrict notifications for confidential issues

Closes #14468 

/cc @rymai 

See merge request !3334
parents 0c0f9253 8ca5b331
...@@ -97,12 +97,12 @@ class Label < ActiveRecord::Base ...@@ -97,12 +97,12 @@ class Label < ActiveRecord::Base
end end
end end
def open_issues_count def open_issues_count(user = nil)
issues.opened.count issues.visible_to_user(user).opened.count
end end
def closed_issues_count def closed_issues_count(user = nil)
issues.closed.count issues.visible_to_user(user).closed.count
end end
def open_merge_requests_count def open_merge_requests_count
......
...@@ -162,6 +162,7 @@ class NotificationService ...@@ -162,6 +162,7 @@ class NotificationService
recipients = add_subscribed_users(recipients, note.noteable) recipients = add_subscribed_users(recipients, note.noteable)
recipients = reject_unsubscribed_users(recipients, note.noteable) recipients = reject_unsubscribed_users(recipients, note.noteable)
recipients = reject_users_without_access(recipients, note.noteable)
recipients.delete(note.author) recipients.delete(note.author)
recipients = recipients.uniq recipients = recipients.uniq
...@@ -376,6 +377,14 @@ class NotificationService ...@@ -376,6 +377,14 @@ class NotificationService
end end
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) def add_subscribed_users(recipients, target)
return recipients unless target.respond_to? :subscribers return recipients unless target.respond_to? :subscribers
...@@ -464,15 +473,16 @@ class NotificationService ...@@ -464,15 +473,16 @@ class NotificationService
end end
recipients = reject_unsubscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target)
recipients = reject_users_without_access(recipients, target)
recipients.delete(current_user) recipients.delete(current_user)
recipients.uniq recipients.uniq
end end
def build_relabeled_recipients(target, current_user, labels:) def build_relabeled_recipients(target, current_user, labels:)
recipients = add_labels_subscribers([], target, labels: labels) recipients = add_labels_subscribers([], target, labels: labels)
recipients = reject_unsubscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target)
recipients = reject_users_without_access(recipients, target)
recipients.delete(current_user) recipients.delete(current_user)
recipients.uniq recipients.uniq
end end
......
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
%strong.append-right-20 %strong.append-right-20
= link_to_label(label) do = link_to_label(label) do
= pluralize label.open_issues_count, 'open issue' = pluralize label.open_issues_count(current_user), 'open issue'
- if current_user - if current_user
.label-subscription{data: {url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label)}} .label-subscription{data: {url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label)}}
......
...@@ -151,7 +151,12 @@ describe Issues::UpdateService, services: true do ...@@ -151,7 +151,12 @@ describe Issues::UpdateService, services: true do
context 'when the issue is relabeled' do context 'when the issue is relabeled' do
let!(:non_subscriber) { create(:user) } 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 it 'sends notifications for subscribers of newly added labels' do
opts = { label_ids: [label.id] } opts = { label_ids: [label.id] }
......
...@@ -111,6 +111,33 @@ describe NotificationService, services: true do ...@@ -111,6 +111,33 @@ describe NotificationService, services: true do
end end
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 context 'issue note mention' do
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :public) }
let(:issue) { create(:issue, project: project, assignee: create(:user)) } let(:issue) { create(:issue, project: project, assignee: create(:user)) }
...@@ -233,6 +260,36 @@ describe NotificationService, services: true do ...@@ -233,6 +260,36 @@ describe NotificationService, services: true do
should_email(subscriber) should_email(subscriber)
end 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 end
describe :reassigned_issue do describe :reassigned_issue do
...@@ -332,6 +389,37 @@ describe NotificationService, services: true do ...@@ -332,6 +389,37 @@ describe NotificationService, services: true do
should_not_email(subscriber_to_label) should_not_email(subscriber_to_label)
should_email(subscriber_to_label2) should_email(subscriber_to_label2)
end 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 end
describe :close_issue do describe :close_issue do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment