From 239560331ab5ef635a8739f5b2b85d0bcd531256 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre <dbalexandre@gmail.com> Date: Thu, 24 Mar 2016 14:20:47 -0300 Subject: [PATCH] Comments on confidential issues doesn't show in activity feed to non-members --- CHANGELOG | 3 +++ app/models/event.rb | 8 ++++-- spec/models/event_spec.rb | 54 +++++++++++++++++++++++++++++---------- 3 files changed, 49 insertions(+), 16 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d9be95defd1..5d9f4961ef5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,9 @@ v 8.7.0 (unreleased) - Make HTTP(s) label consistent on clone bar (Stan Hu) - Fix avatar stretching by providing a cropping feature +v 8.6.2 (unreleased) + - Comments on confidential issues don't show up in activity feed to non-members + v 8.6.1 - Add option to reload the schema before restoring a database backup. !2807 - Display navigation controls on mobile. !3214 diff --git a/app/models/event.rb b/app/models/event.rb index a5cfeaf388e..2b963d4f2f6 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -80,8 +80,8 @@ class Event < ActiveRecord::Base true elsif created_project? true - elsif issue? - Ability.abilities.allowed?(user, :read_issue, issue) + elsif issue? || issue_note? + Ability.abilities.allowed?(user, :read_issue, note? ? note_target : target) else ((merge_request? || note?) && target) || milestone? end @@ -298,6 +298,10 @@ class Event < ActiveRecord::Base target.noteable_type == "Commit" end + def issue_note? + note? && target && target.noteable_type == "Issue" + end + def note_project_snippet? target.noteable_type == "Snippet" end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 5fe44246738..697a34022ce 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -66,21 +66,25 @@ describe Event, models: true do end describe '#proper?' do - context 'issue event' do - let(:project) { create(:empty_project, :public) } - let(:non_member) { create(:user) } - let(:member) { create(:user) } - let(:author) { create(:author) } - let(:assignee) { create(:user) } - let(:admin) { create(:admin) } - let(:event) { Event.new(project: project, action: Event::CREATED, target: issue, author_id: author.id) } - - before do - project.team << [member, :developer] - end + let(:project) { create(:empty_project, :public) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:author) { create(:author) } + let(:assignee) { create(:user) } + let(:admin) { create(:admin) } + 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 + project.team << [member, :developer] + end + context 'issue event' 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.proper?(author)).to eq true } @@ -90,7 +94,29 @@ describe Event, models: true do end context 'for confidential issues' do - let(:issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:target) { confidential_issue } + + it { expect(event.proper?(non_member)).to eq false } + it { expect(event.proper?(author)).to eq true } + it { expect(event.proper?(assignee)).to eq true } + it { expect(event.proper?(member)).to eq true } + it { expect(event.proper?(admin)).to eq true } + end + end + + context 'note event' do + context 'on non confidential issues' do + let(:target) { note_on_issue } + + it { expect(event.proper?(non_member)).to eq true } + it { expect(event.proper?(author)).to eq true } + it { expect(event.proper?(assignee)).to eq true } + it { expect(event.proper?(member)).to eq true } + it { expect(event.proper?(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.proper?(author)).to eq true } -- 2.30.9