Commit 93212370 authored by Jonathan Schafer's avatar Jonathan Schafer

Use user_notes/discussions_count resolvers for MRs

Modify user_notes_count_resolver.rb and
user_discussions_count_resolver.rb to be generic
Use resolvers for MergeRequestType
parent 3318b9dc
...@@ -9,8 +9,8 @@ module Resolvers ...@@ -9,8 +9,8 @@ module Resolvers
def resolve def resolve
authorize!(object) authorize!(object)
BatchLoader::GraphQL.for(object.id).batch(key: :issue_user_discussions_count) do |ids, loader, args| BatchLoader::GraphQL.for(object.id).batch do |ids, loader, args|
counts = Note.count_for_collection(ids, 'Issue', 'COUNT(DISTINCT discussion_id) as count').index_by(&:noteable_id) counts = Note.count_for_collection(ids, object.class.name, 'COUNT(DISTINCT discussion_id) as count').index_by(&:noteable_id)
ids.each do |id| ids.each do |id|
loader.call(id, counts[id]&.count || 0) loader.call(id, counts[id]&.count || 0)
...@@ -19,7 +19,8 @@ module Resolvers ...@@ -19,7 +19,8 @@ module Resolvers
end end
def authorized_resource?(object) def authorized_resource?(object)
context[:current_user].present? && Ability.allowed?(context[:current_user], :read_issue, object) ability = "read_#{object.class.name.underscore}".to_sym
context[:current_user].present? && Ability.allowed?(context[:current_user], ability, object)
end end
end end
end end
...@@ -9,8 +9,8 @@ module Resolvers ...@@ -9,8 +9,8 @@ module Resolvers
def resolve def resolve
authorize!(object) authorize!(object)
BatchLoader::GraphQL.for(object.id).batch(key: :issue_user_notes_count) do |ids, loader, args| BatchLoader::GraphQL.for(object.id).batch(key: :user_notes_count) do |ids, loader, args|
counts = Note.count_for_collection(ids, 'Issue').index_by(&:noteable_id) counts = Note.count_for_collection(ids, object.class.name).index_by(&:noteable_id)
ids.each do |id| ids.each do |id|
loader.call(id, counts[id]&.count || 0) loader.call(id, counts[id]&.count || 0)
...@@ -19,7 +19,8 @@ module Resolvers ...@@ -19,7 +19,8 @@ module Resolvers
end end
def authorized_resource?(object) def authorized_resource?(object)
context[:current_user].present? && Ability.allowed?(context[:current_user], :read_issue, object) ability = "read_#{object.class.name.underscore}".to_sym
context[:current_user].present? && Ability.allowed?(context[:current_user], ability, object)
end end
end end
end end
...@@ -69,9 +69,11 @@ module Types ...@@ -69,9 +69,11 @@ module Types
field :merge_commit_sha, GraphQL::STRING_TYPE, null: true, field :merge_commit_sha, GraphQL::STRING_TYPE, null: true,
description: 'SHA of the merge request commit (set once merged)' description: 'SHA of the merge request commit (set once merged)'
field :user_notes_count, GraphQL::INT_TYPE, null: true, field :user_notes_count, GraphQL::INT_TYPE, null: true,
description: 'User notes count of the merge request' description: 'User notes count of the merge request',
resolver: Resolvers::UserNotesCountResolver
field :user_discussions_count, GraphQL::INT_TYPE, null: true, field :user_discussions_count, GraphQL::INT_TYPE, null: true,
description: 'Number of user discussions in the merge request' description: 'Number of user discussions in the merge request',
resolver: Resolvers::UserDiscussionsCountResolver
field :should_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :should_remove_source_branch?, null: true, field :should_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :should_remove_source_branch?, null: true,
description: 'Indicates if the source branch of the merge request will be deleted after merge' description: 'Indicates if the source branch of the merge request will be deleted after merge'
field :force_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :force_remove_source_branch?, null: true, field :force_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :force_remove_source_branch?, null: true,
......
...@@ -7,43 +7,82 @@ RSpec.describe Resolvers::UserNotesCountResolver do ...@@ -7,43 +7,82 @@ RSpec.describe Resolvers::UserNotesCountResolver do
describe '#resolve' do describe '#resolve' do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public) } let_it_be(:project) { create(:project, :repository, :public) }
let_it_be(:private_project) { create(:project, :private) } let_it_be(:private_project) { create(:project, :repository, :private) }
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:private_issue) { create(:issue, project: private_project) }
let_it_be(:public_notes) { create_list(:note, 2, noteable: issue, project: project) }
let_it_be(:system_note) { create(:note, system: true, noteable: issue, project: project) }
let_it_be(:private_notes) { create_list(:note, 3, noteable: private_issue, project: private_project) }
specify do specify do
expect(described_class).to have_nullable_graphql_type(GraphQL::INT_TYPE) expect(described_class).to have_nullable_graphql_type(GraphQL::INT_TYPE)
end end
context 'when counting notes from a public issue' do context 'when counting notes from an issue' do
subject { batch_sync { resolve_user_notes_count(issue) } } let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:private_issue) { create(:issue, project: private_project) }
let_it_be(:public_notes) { create_list(:note, 2, noteable: issue, project: project) }
let_it_be(:system_note) { create(:note, system: true, noteable: issue, project: project) }
let_it_be(:private_notes) { create_list(:note, 3, noteable: private_issue, project: private_project) }
it 'returns the number of non-system notes for the issue' do context 'when counting notes from a public issue' do
expect(subject).to eq(2) subject { batch_sync { resolve_user_notes_count(issue) } }
it 'returns the number of non-system notes for the issue' do
expect(subject).to eq(2)
end
end end
end
context 'when a user has permission to view notes' do context 'when a user has permission to view notes' do
before do before do
private_project.add_developer(user) private_project.add_developer(user)
end
subject { batch_sync { resolve_user_notes_count(private_issue) } }
it 'returns the number of notes for the issue' do
expect(subject).to eq(3)
end
end end
subject { batch_sync { resolve_user_notes_count(private_issue) } } context 'when a user does not have permission to view discussions' do
subject { batch_sync { resolve_user_notes_count(private_issue) } }
it 'returns the number of notes for the issue' do it 'returns no notes' do
expect(subject).to eq(3) expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end end
end end
context 'when a user does not have permission to view discussions' do context 'when counting notes from a merge request' do
subject { batch_sync { resolve_user_notes_count(private_issue) } } let_it_be(:merge_request) { create(:merge_request, source_project: project) }
let_it_be(:private_merge_request) { create(:merge_request, source_project: private_project) }
let_it_be(:public_notes) { create_list(:note, 2, noteable: merge_request, project: project) }
let_it_be(:system_note) { create(:note, system: true, noteable: merge_request, project: project) }
let_it_be(:private_notes) { create_list(:note, 3, noteable: private_merge_request, project: private_project) }
context 'when counting notes from a public merge request' do
subject { batch_sync { resolve_user_notes_count(merge_request) } }
it 'returns the number of non-system notes for the merge request' do
expect(subject).to eq(2)
end
end
context 'when a user has permission to view notes' do
before do
private_project.add_developer(user)
end
subject { batch_sync { resolve_user_notes_count(private_merge_request) } }
it 'returns the number of notes for the merge request' do
expect(subject).to eq(3)
end
end
context 'when a user does not have permission to view discussions' do
subject { batch_sync { resolve_user_notes_count(private_merge_request) } }
it 'returns no notes' do it 'returns no notes' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
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