Commit 30030141 authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

Merge branch 'fix-comments-on-confidential-issues-show-activity-feed-for-non-members' into 'master'

Comments on confidential issues doesn't show in activity feed to non-members

Closes #14568

See merge request !3375
parent 62600ec3
...@@ -9,6 +9,7 @@ v 8.6.2 ...@@ -9,6 +9,7 @@ v 8.6.2
- Fix bold text in issuable sidebar. !3358 - Fix bold text in issuable sidebar. !3358
- Fix error with anonymous token in applications settings. !3362 - Fix error with anonymous token in applications settings. !3362
- Fix the milestone 'upcoming' filter. !3364 - Fix the milestone 'upcoming' filter. !3364
- Fix comments on confidential issues showing up in activity feed to non-members. !3375
v 8.6.1 v 8.6.1
- Add option to reload the schema before restoring a database backup. !2807 - Add option to reload the schema before restoring a database backup. !2807
......
...@@ -194,7 +194,7 @@ module EventsHelper ...@@ -194,7 +194,7 @@ module EventsHelper
end end
def event_to_atom(xml, event) def event_to_atom(xml, event)
if event.proper?(current_user) if event.visible_to_user?(current_user)
xml.entry do xml.entry do
event_link = event_feed_url(event) event_link = event_feed_url(event)
event_title = event_feed_title(event) event_title = event_feed_title(event)
......
...@@ -73,15 +73,15 @@ class Event < ActiveRecord::Base ...@@ -73,15 +73,15 @@ class Event < ActiveRecord::Base
end end
end end
def proper?(user = nil) def visible_to_user?(user = nil)
if push? if push?
true true
elsif membership_changed? elsif membership_changed?
true true
elsif created_project? elsif created_project?
true true
elsif issue? elsif issue? || issue_note?
Ability.abilities.allowed?(user, :read_issue, issue) Ability.abilities.allowed?(user, :read_issue, note? ? note_target : target)
else else
((merge_request? || note?) && target) || milestone? ((merge_request? || note?) && target) || milestone?
end end
...@@ -298,6 +298,10 @@ class Event < ActiveRecord::Base ...@@ -298,6 +298,10 @@ class Event < ActiveRecord::Base
target.noteable_type == "Commit" target.noteable_type == "Commit"
end end
def issue_note?
note? && target && target.noteable_type == "Issue"
end
def note_project_snippet? def note_project_snippet?
target.noteable_type == "Snippet" target.noteable_type == "Snippet"
end end
......
- if event.proper?(current_user) - if event.visible_to_user?(current_user)
.event-item{class: "#{event.body? ? "event-block" : "event-inline" }"} .event-item{class: "#{event.body? ? "event-block" : "event-inline" }"}
.event-item-timestamp .event-item-timestamp
#{time_ago_with_tooltip(event.created_at)} #{time_ago_with_tooltip(event.created_at)}
......
...@@ -59,44 +59,70 @@ describe Event, models: true do ...@@ -59,44 +59,70 @@ describe Event, models: true do
end end
it { expect(@event.push?).to be_truthy } it { expect(@event.push?).to be_truthy }
it { expect(@event.proper?).to be_truthy } it { expect(@event.visible_to_user?).to be_truthy }
it { expect(@event.tag?).to be_falsey } it { expect(@event.tag?).to be_falsey }
it { expect(@event.branch_name).to eq("master") } it { expect(@event.branch_name).to eq("master") }
it { expect(@event.author).to eq(@user) } it { expect(@event.author).to eq(@user) }
end end
describe '#proper?' do describe '#visible_to_user?' do
context 'issue event' do
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :public) }
let(:non_member) { create(:user) } let(:non_member) { create(:user) }
let(:member) { create(:user) } let(:member) { create(:user) }
let(:author) { create(:author) } let(:author) { create(:author) }
let(:assignee) { create(:user) } let(:assignee) { create(:user) }
let(:admin) { create(:admin) } let(:admin) { create(:admin) }
let(:event) { Event.new(project: project, action: Event::CREATED, target: issue, author_id: author.id) } let(:issue) { create(:issue, project: project, author: author, assignee: assignee) }
let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) }
let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) }
let(:event) { Event.new(project: project, target: target, author_id: author.id) }
before do before do
project.team << [member, :developer] project.team << [member, :developer]
end end
context 'issue event' do
context 'for non confidential issues' do context 'for non confidential issues' do
let(:issue) { create(:issue, project: project, author: author, assignee: assignee) } let(:target) { issue }
it { expect(event.proper?(non_member)).to eq true } it { expect(event.visible_to_user?(non_member)).to eq true }
it { expect(event.proper?(author)).to eq true } it { expect(event.visible_to_user?(author)).to eq true }
it { expect(event.proper?(assignee)).to eq true } it { expect(event.visible_to_user?(assignee)).to eq true }
it { expect(event.proper?(member)).to eq true } it { expect(event.visible_to_user?(member)).to eq true }
it { expect(event.proper?(admin)).to eq true } it { expect(event.visible_to_user?(admin)).to eq true }
end end
context 'for confidential issues' do context 'for confidential issues' do
let(:issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } let(:target) { confidential_issue }
it { expect(event.visible_to_user?(non_member)).to eq false }
it { expect(event.visible_to_user?(author)).to eq true }
it { expect(event.visible_to_user?(assignee)).to eq true }
it { expect(event.visible_to_user?(member)).to eq true }
it { expect(event.visible_to_user?(admin)).to eq true }
end
end
context 'note event' do
context 'on non confidential issues' do
let(:target) { note_on_issue }
it { expect(event.visible_to_user?(non_member)).to eq true }
it { expect(event.visible_to_user?(author)).to eq true }
it { expect(event.visible_to_user?(assignee)).to eq true }
it { expect(event.visible_to_user?(member)).to eq true }
it { expect(event.visible_to_user?(admin)).to eq true }
end
context 'on confidential issues' do
let(:target) { note_on_confidential_issue }
it { expect(event.proper?(non_member)).to eq false } it { expect(event.visible_to_user?(non_member)).to eq false }
it { expect(event.proper?(author)).to eq true } it { expect(event.visible_to_user?(author)).to eq true }
it { expect(event.proper?(assignee)).to eq true } it { expect(event.visible_to_user?(assignee)).to eq true }
it { expect(event.proper?(member)).to eq true } it { expect(event.visible_to_user?(member)).to eq true }
it { expect(event.proper?(admin)).to eq true } it { expect(event.visible_to_user?(admin)).to eq true }
end end
end end
end end
......
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