Commit 4834e2e6 authored by Rémy Coutable's avatar Rémy Coutable

Fix diff comments not showing up in activity feed

Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent f60b48bd
...@@ -41,6 +41,7 @@ v 8.9.5 (unreleased) ...@@ -41,6 +41,7 @@ v 8.9.5 (unreleased)
- Downgrade to Redis 3.2.2 due to massive memory leak with Sidekiq - Downgrade to Redis 3.2.2 due to massive memory leak with Sidekiq
- Fixes issues importing events in Import/Export. Import/Export version bumped to 0.1.1 - Fixes issues importing events in Import/Export. Import/Export version bumped to 0.1.1
- Fix import button disabled when import process fail due to the namespace already been taken. - Fix import button disabled when import process fail due to the namespace already been taken.
- Fix diff comments not showing up in activity feed. !5069
- Security: Update RedCloth to 4.3.2 (Takuya Noguchi) - Security: Update RedCloth to 4.3.2 (Takuya Noguchi)
v 8.9.4 v 8.9.4
......
...@@ -67,7 +67,7 @@ class Event < ActiveRecord::Base ...@@ -67,7 +67,7 @@ class Event < ActiveRecord::Base
elsif issue? || issue_note? elsif issue? || issue_note?
Ability.abilities.allowed?(user, :read_issue, note? ? note_target : target) Ability.abilities.allowed?(user, :read_issue, note? ? note_target : target)
else else
((merge_request? || note?) && target) || milestone? ((merge_request? || note?) && target.present?) || milestone?
end end
end end
...@@ -136,7 +136,7 @@ class Event < ActiveRecord::Base ...@@ -136,7 +136,7 @@ class Event < ActiveRecord::Base
end end
def note? def note?
target_type == "Note" %w[Note LegacyDiffNote].include?(target_type)
end end
def issue? def issue?
......
...@@ -46,6 +46,22 @@ describe Event, models: true do ...@@ -46,6 +46,22 @@ describe Event, models: true do
it { expect(@event.author).to eq(@user) } it { expect(@event.author).to eq(@user) }
end end
describe '#note?' do
subject { Event.new(project: target.project, target: target) }
context 'issue note event' do
let(:target) { create(:note_on_issue) }
it { is_expected.to be_note }
end
context 'merge request note event' do
let(:target) { create(:note_on_merge_request) }
it { is_expected.to be_note }
end
end
describe '#visible_to_user?' do describe '#visible_to_user?' do
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :public) }
let(:non_member) { create(:user) } let(:non_member) { create(:user) }
...@@ -89,7 +105,7 @@ describe Event, models: true do ...@@ -89,7 +105,7 @@ describe Event, models: true do
end end
end end
context 'note event' do context 'issue note event' do
context 'on non confidential issues' do context 'on non confidential issues' do
let(:target) { note_on_issue } let(:target) { note_on_issue }
...@@ -112,6 +128,20 @@ describe Event, models: true do ...@@ -112,6 +128,20 @@ describe Event, models: true do
it { expect(event.visible_to_user?(admin)).to eq true } it { expect(event.visible_to_user?(admin)).to eq true }
end end
end end
context 'merge request note event' do
let(:project) { create(:project, :public) }
let(:merge_request) { create(:merge_request, source_project: project, author: author, assignee: assignee) }
let(:note_on_merge_request) { create(:note_on_merge_request, noteable: merge_request, project: project) }
let(:target) { note_on_merge_request }
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?(guest)).to eq true }
it { expect(event.visible_to_user?(admin)).to eq true }
end
end end
describe '.limit_recent' do describe '.limit_recent' 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