Commit b5d3c3ca authored by Robert Speicher's avatar Robert Speicher

Merge branch '23824-activity-page-does-not-show-commits-comments' into 'master'

Allow commit note to be visible if repo is visible

## What does this MR do?

It enforces the `:download_code` permission in `Event#visible_to_user?` for commit notes.

Closes #23824

See merge request !7504
parents a65f83c6 d47fca53
...@@ -86,7 +86,7 @@ module EventsHelper ...@@ -86,7 +86,7 @@ module EventsHelper
elsif event.merge_request? elsif event.merge_request?
namespace_project_merge_request_url(event.project.namespace, namespace_project_merge_request_url(event.project.namespace,
event.project, event.merge_request) event.project, event.merge_request)
elsif event.note? && event.commit_note? elsif event.commit_note?
namespace_project_commit_url(event.project.namespace, event.project, namespace_project_commit_url(event.project.namespace, event.project,
event.note_target) event.note_target)
elsif event.note? elsif event.note?
...@@ -127,7 +127,7 @@ module EventsHelper ...@@ -127,7 +127,7 @@ module EventsHelper
end end
def event_note_target_path(event) def event_note_target_path(event)
if event.note? && event.commit_note? if event.commit_note?
namespace_project_commit_path(event.project.namespace, namespace_project_commit_path(event.project.namespace,
event.project, event.project,
event.note_target, event.note_target,
......
...@@ -62,7 +62,7 @@ class Event < ActiveRecord::Base ...@@ -62,7 +62,7 @@ class Event < ActiveRecord::Base
end end
def visible_to_user?(user = nil) def visible_to_user?(user = nil)
if push? if push? || commit_note?
Ability.allowed?(user, :download_code, project) Ability.allowed?(user, :download_code, project)
elsif membership_changed? elsif membership_changed?
true true
...@@ -283,7 +283,7 @@ class Event < ActiveRecord::Base ...@@ -283,7 +283,7 @@ class Event < ActiveRecord::Base
end end
def commit_note? def commit_note?
target.for_commit? note? && target && target.for_commit?
end end
def issue_note? def issue_note?
...@@ -295,7 +295,7 @@ class Event < ActiveRecord::Base ...@@ -295,7 +295,7 @@ class Event < ActiveRecord::Base
end end
def project_snippet_note? def project_snippet_note?
target.for_snippet? note? && target && target.for_snippet?
end end
def note_target def note_target
......
---
title: Allow commit note to be visible if repo is visible
merge_request:
author:
...@@ -94,6 +94,7 @@ describe Event, models: true do ...@@ -94,6 +94,7 @@ describe Event, models: true do
let(:admin) { create(:admin) } let(:admin) { create(:admin) }
let(:issue) { create(:issue, project: project, author: author, assignee: assignee) } let(:issue) { create(:issue, project: project, author: author, assignee: assignee) }
let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
let(:note_on_commit) { create(:note_on_commit, project: project) }
let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) } 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(: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) } let(:event) { Event.new(project: project, target: target, author_id: author.id) }
...@@ -103,6 +104,32 @@ describe Event, models: true do ...@@ -103,6 +104,32 @@ describe Event, models: true do
project.team << [guest, :guest] project.team << [guest, :guest]
end end
context 'commit note event' do
let(:target) { note_on_commit }
it do
aggregate_failures do
expect(event.visible_to_user?(non_member)).to eq true
expect(event.visible_to_user?(member)).to eq true
expect(event.visible_to_user?(guest)).to eq true
expect(event.visible_to_user?(admin)).to eq true
end
end
context 'private project' do
let(:project) { create(:empty_project, :private) }
it do
aggregate_failures do
expect(event.visible_to_user?(non_member)).to eq false
expect(event.visible_to_user?(member)).to eq true
expect(event.visible_to_user?(guest)).to eq false
expect(event.visible_to_user?(admin)).to eq true
end
end
end
end
context 'issue event' do context 'issue event' do
context 'for non confidential issues' do context 'for non confidential issues' do
let(:target) { issue } let(:target) { issue }
......
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