summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2016-07-04 16:29:28 +0200
committerRémy Coutable <remy@rymai.me>2016-07-04 16:29:28 +0200
commit4834e2e60936d9f59f3dd315d6d9d4ef82eecd66 (patch)
treeb5c6cc56019b187a5493f8ea2d7faae02969bd5c
parentf60b48bdcb4762451618fe03bca98581282a0904 (diff)
downloadgitlab-ce-4834e2e60936d9f59f3dd315d6d9d4ef82eecd66.tar.gz
Fix diff comments not showing up in activity feed
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--CHANGELOG1
-rw-r--r--app/models/event.rb4
-rw-r--r--spec/models/event_spec.rb32
3 files changed, 34 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 3b89fa05801..6cc62793a57 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -41,6 +41,7 @@ v 8.9.5 (unreleased)
- 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
- 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)
v 8.9.4
diff --git a/app/models/event.rb b/app/models/event.rb
index d7d23c7ae6d..e0c52fed6fb 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -67,7 +67,7 @@ class Event < ActiveRecord::Base
elsif issue? || issue_note?
Ability.abilities.allowed?(user, :read_issue, note? ? note_target : target)
else
- ((merge_request? || note?) && target) || milestone?
+ ((merge_request? || note?) && target.present?) || milestone?
end
end
@@ -136,7 +136,7 @@ class Event < ActiveRecord::Base
end
def note?
- target_type == "Note"
+ %w[Note LegacyDiffNote].include?(target_type)
end
def issue?
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index 166a1dc4ddb..00925591a5e 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -46,6 +46,22 @@ describe Event, models: true do
it { expect(@event.author).to eq(@user) }
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
let(:project) { create(:empty_project, :public) }
let(:non_member) { create(:user) }
@@ -89,7 +105,7 @@ describe Event, models: true do
end
end
- context 'note event' do
+ context 'issue note event' do
context 'on non confidential issues' do
let(:target) { note_on_issue }
@@ -112,6 +128,20 @@ describe Event, models: true do
it { expect(event.visible_to_user?(admin)).to eq true }
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
describe '.limit_recent' do