From f5c7c3b9ae8b91662830f46b595ce1512050be89 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Wed, 17 Apr 2019 18:43:19 -0500 Subject: [PATCH 1/3] Basic GraphQL for a group Add new query for Groups, with new GroupType and NamespaceType --- app/graphql/resolvers/full_path_resolver.rb | 6 +- app/graphql/resolvers/group_resolver.rb | 13 ++ app/graphql/types/group_type.rb | 21 ++ app/graphql/types/namespace_type.rb | 19 ++ app/graphql/types/permission_types/group.rb | 11 + app/graphql/types/project_type.rb | 3 + app/graphql/types/query_type.rb | 5 + .../unreleased/bw-add-graphql-groups.yml | 5 + spec/graphql/types/group_type_spec.rb | 11 + spec/graphql/types/namespace_type.rb | 7 + spec/graphql/types/query_type_spec.rb | 2 +- spec/requests/api/graphql/group_query_spec.rb | 197 ++++++++++++++++++ 12 files changed, 296 insertions(+), 4 deletions(-) create mode 100644 app/graphql/resolvers/group_resolver.rb create mode 100644 app/graphql/types/group_type.rb create mode 100644 app/graphql/types/namespace_type.rb create mode 100644 app/graphql/types/permission_types/group.rb create mode 100644 changelogs/unreleased/bw-add-graphql-groups.yml create mode 100644 spec/graphql/types/group_type_spec.rb create mode 100644 spec/graphql/types/namespace_type.rb create mode 100644 spec/requests/api/graphql/group_query_spec.rb diff --git a/app/graphql/resolvers/full_path_resolver.rb b/app/graphql/resolvers/full_path_resolver.rb index 0f1a64b6c58..972f318c806 100644 --- a/app/graphql/resolvers/full_path_resolver.rb +++ b/app/graphql/resolvers/full_path_resolver.rb @@ -7,14 +7,14 @@ module Resolvers prepended do argument :full_path, GraphQL::ID_TYPE, required: true, - description: 'The full path of the project or namespace, e.g., "gitlab-org/gitlab-ce"' + description: 'The full path of the project, group or namespace, e.g., "gitlab-org/gitlab-ce"' end def model_by_full_path(model, full_path) BatchLoader.for(full_path).batch(key: model) do |full_paths, loader, args| # `with_route` avoids an N+1 calculating full_path - args[:key].where_full_path_in(full_paths).with_route.each do |project| - loader.call(project.full_path, project) + args[:key].where_full_path_in(full_paths).with_route.each do |model_instance| + loader.call(model_instance.full_path, model_instance) end end end diff --git a/app/graphql/resolvers/group_resolver.rb b/app/graphql/resolvers/group_resolver.rb new file mode 100644 index 00000000000..4260e18829e --- /dev/null +++ b/app/graphql/resolvers/group_resolver.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Resolvers + class GroupResolver < BaseResolver + prepend FullPathResolver + + type Types::GroupType, null: true + + def resolve(full_path:) + model_by_full_path(Group, full_path) + end + end +end diff --git a/app/graphql/types/group_type.rb b/app/graphql/types/group_type.rb new file mode 100644 index 00000000000..a2d615ee732 --- /dev/null +++ b/app/graphql/types/group_type.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Types + class GroupType < NamespaceType + graphql_name 'Group' + + authorize :read_group + + expose_permissions Types::PermissionTypes::Group + + field :web_url, GraphQL::STRING_TYPE, null: true + + field :avatar_url, GraphQL::STRING_TYPE, null: true, resolve: -> (group, args, ctx) do + group.avatar_url(only_path: false) + end + + if ::Group.supports_nested_objects? + field :parent, GroupType, null: true + end + end +end diff --git a/app/graphql/types/namespace_type.rb b/app/graphql/types/namespace_type.rb new file mode 100644 index 00000000000..b1c5da50aa5 --- /dev/null +++ b/app/graphql/types/namespace_type.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Types + class NamespaceType < BaseObject + graphql_name 'Namespace' + + field :id, GraphQL::ID_TYPE, null: false + field :name, GraphQL::STRING_TYPE, null: false + + field :path, GraphQL::STRING_TYPE, null: false + field :description, GraphQL::STRING_TYPE, null: true + field :visibility, GraphQL::STRING_TYPE, null: true + field :lfs_enabled, GraphQL::BOOLEAN_TYPE, null: true, method: :lfs_enabled? + + field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true + field :full_path, GraphQL::ID_TYPE, null: false + field :full_name, GraphQL::STRING_TYPE, null: false + end +end diff --git a/app/graphql/types/permission_types/group.rb b/app/graphql/types/permission_types/group.rb new file mode 100644 index 00000000000..29833993ce6 --- /dev/null +++ b/app/graphql/types/permission_types/group.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Types + module PermissionTypes + class Group < BasePermissionType + graphql_name 'GroupPermissions' + + abilities :read_group + end + end +end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index fbb4eddd13c..baea6658e05 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -66,6 +66,9 @@ module Types field :only_allow_merge_if_all_discussions_are_resolved, GraphQL::BOOLEAN_TYPE, null: true field :printing_merge_request_link_enabled, GraphQL::BOOLEAN_TYPE, null: true + field :namespace, Types::NamespaceType, null: false + field :group, Types::GroupType, null: true + field :merge_requests, Types::MergeRequestType.connection_type, null: true, diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 0f655ab9d03..40d7de1a49a 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -9,6 +9,11 @@ module Types resolver: Resolvers::ProjectResolver, description: "Find a project" + field :group, Types::GroupType, + null: true, + resolver: Resolvers::GroupResolver, + description: "Find a group" + field :metadata, Types::MetadataType, null: true, resolver: Resolvers::MetadataResolver, diff --git a/changelogs/unreleased/bw-add-graphql-groups.yml b/changelogs/unreleased/bw-add-graphql-groups.yml new file mode 100644 index 00000000000..f72ee1cf2b7 --- /dev/null +++ b/changelogs/unreleased/bw-add-graphql-groups.yml @@ -0,0 +1,5 @@ +--- +title: Add initial GraphQL query for Groups +merge_request: 27492 +author: +type: added diff --git a/spec/graphql/types/group_type_spec.rb b/spec/graphql/types/group_type_spec.rb new file mode 100644 index 00000000000..3dd5b602aa2 --- /dev/null +++ b/spec/graphql/types/group_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['Group'] do + it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::Group) } + + it { expect(described_class.graphql_name).to eq('Group') } + + it { expect(described_class).to require_graphql_authorizations(:read_group) } +end diff --git a/spec/graphql/types/namespace_type.rb b/spec/graphql/types/namespace_type.rb new file mode 100644 index 00000000000..7cd6a79ae5d --- /dev/null +++ b/spec/graphql/types/namespace_type.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['Namespace'] do + it { expect(described_class.graphql_name).to eq('Namespace') } +end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index 69e3ea8a4a9..b4626955816 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -5,7 +5,7 @@ describe GitlabSchema.types['Query'] do expect(described_class.graphql_name).to eq('Query') end - it { is_expected.to have_graphql_fields(:project, :echo, :metadata) } + it { is_expected.to have_graphql_fields(:project, :group, :echo, :metadata) } describe 'project field' do subject { described_class.fields['project'] } diff --git a/spec/requests/api/graphql/group_query_spec.rb b/spec/requests/api/graphql/group_query_spec.rb new file mode 100644 index 00000000000..e4f559e9f10 --- /dev/null +++ b/spec/requests/api/graphql/group_query_spec.rb @@ -0,0 +1,197 @@ +require 'spec_helper' + +# Based on spec/requests/api/groups_spec.rb +# Should follow closely in order to ensure all situations are covered +describe 'getting group information' do + include GraphqlHelpers + include UploadHelpers + + let(:user1) { create(:user, can_create_group: false) } + let(:user2) { create(:user) } + let(:admin) { create(:admin) } + let!(:group1) { create(:group, avatar: File.open(uploaded_image_temp_path)) } + let!(:group2) { create(:group, :private) } + # let!(:project1) { create(:project, namespace: group1) } + # let!(:project2) { create(:project, namespace: group2) } + # let!(:project3) { create(:project, namespace: group1, path: 'test', visibility_level: Gitlab::VisibilityLevel::PRIVATE) } + + before do + group1.add_owner(user1) + group2.add_owner(user2) + end + + # similar to the API "GET /groups/:id" + describe "Query group(fullPath)" do + # Given a group, create one project for each visibility level + # + # group - Group to add projects to + # share_with - If provided, each project will be shared with this Group + # + # Returns a Hash of visibility_level => Project pairs + def add_projects_to_group(group, share_with: nil) + projects = { + public: create(:project, :public, namespace: group), + internal: create(:project, :internal, namespace: group), + private: create(:project, :private, namespace: group) + } + + if share_with + create(:project_group_link, project: projects[:public], group: share_with) + create(:project_group_link, project: projects[:internal], group: share_with) + create(:project_group_link, project: projects[:private], group: share_with) + end + + projects + end + + def response_project_ids(json_response, key) + json_response[key].map do |project| + project['id'].to_i + end + end + + def group_query(group) + graphql_query_for('group', 'fullPath' => group.full_path) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(group_query(group1)) + end + end + + context 'when unauthenticated' do + it 'returns nil for a private group' do + post_graphql(group_query(group2)) + + expect(graphql_data['group']).to be_nil + end + + it 'returns a public group' do + post_graphql(group_query(group1)) + + expect(graphql_data['group']).not_to be_nil + end + + # it 'returns only public projects in the group' do + # public_group = create(:group, :public) + # projects = add_projects_to_group(public_group) + # + # get api("/groups/#{public_group.id}") + # + # expect(response_project_ids(json_response, 'projects')) + # .to contain_exactly(projects[:public].id) + # end + + # it 'returns only public projects shared with the group' do + # public_group = create(:group, :public) + # projects = add_projects_to_group(public_group, share_with: group1) + # + # get api("/groups/#{group1.id}") + # + # expect(response_project_ids(json_response, 'shared_projects')) + # .to contain_exactly(projects[:public].id) + # end + end + + context "when authenticated as user" do + it "returns one of user1's groups" do + project = create(:project, namespace: group2, path: 'Foo') + create(:project_group_link, project: project, group: group1) + + post_graphql(group_query(group1), current_user: user1) + + expect(response).to have_gitlab_http_status(200) + expect(graphql_data['group']['id']).to eq(group1.id.to_s) + expect(graphql_data['group']['name']).to eq(group1.name) + expect(graphql_data['group']['path']).to eq(group1.path) + expect(graphql_data['group']['description']).to eq(group1.description) + expect(graphql_data['group']['visibility']).to eq(Gitlab::VisibilityLevel.string_level(group1.visibility_level)) + expect(graphql_data['group']['avatarUrl']).to eq(group1.avatar_url(only_path: false)) + expect(graphql_data['group']['webUrl']).to eq(group1.web_url) + expect(graphql_data['group']['requestAccessEnabled']).to eq(group1.request_access_enabled) + expect(graphql_data['group']['fullName']).to eq(group1.full_name) + expect(graphql_data['group']['fullPath']).to eq(group1.full_path) + expect(graphql_data['group']['parentId']).to eq(group1.parent_id) + # expect(graphql_data['group']['projects']).to be_an Array + # expect(graphql_data['group']['projects'].length).to eq(2) + # expect(graphql_data['group']['sharedProjects']).to be_an Array + # expect(graphql_data['group']['sharedProjects'].length).to eq(1) + # expect(graphql_data['group']['sharedProjects'][0]['id']).to eq(project.id) + end + + # it "returns one of user1's groups without projects when with_projects option is set to false" do + # project = create(:project, namespace: group2, path: 'Foo') + # create(:project_group_link, project: project, group: group1) + # + # get api("/groups/#{group1.id}", user1), params: { with_projects: false } + # + # expect(response).to have_gitlab_http_status(200) + # expect(json_response['projects']).to be_nil + # expect(json_response['shared_projects']).to be_nil + # end + + it "does not return a non existing group" do + query = graphql_query_for('group', 'fullPath' => '1328') + post_graphql(query, current_user: user1) + + expect(graphql_data['group']).to be_nil + end + + it "does not return a group not attached to user1" do + post_graphql(group_query(group2), current_user: user1) + + expect(graphql_data['group']).to be_nil + end + + # it 'returns only public and internal projects in the group' do + # public_group = create(:group, :public) + # projects = add_projects_to_group(public_group) + # + # get api("/groups/#{public_group.id}", user2) + # + # expect(response_project_ids(json_response, 'projects')) + # .to contain_exactly(projects[:public].id, projects[:internal].id) + # end + + # it 'returns only public and internal projects shared with the group' do + # public_group = create(:group, :public) + # projects = add_projects_to_group(public_group, share_with: group1) + # + # get api("/groups/#{group1.id}", user2) + # + # expect(response_project_ids(json_response, 'shared_projects')) + # .to contain_exactly(projects[:public].id, projects[:internal].id) + # end + + it 'avoids N+1 queries' do + post_graphql(group_query(group1), current_user: admin) + + control_count = ActiveRecord::QueryRecorder.new do + post_graphql(group_query(group1), current_user: admin) + end.count + + create(:project, namespace: group1) + + expect do + post_graphql(group_query(group1), current_user: admin) + end.not_to exceed_query_limit(control_count) + end + end + + context "when authenticated as admin" do + it "returns any existing group" do + post_graphql(group_query(group2), current_user: admin) + + expect(graphql_data['group']['name']).to eq(group2.name) + end + + it "does not return a non existing group" do + query = graphql_query_for('group', 'fullPath' => '1328') + post_graphql(query, current_user: admin) + + expect(graphql_data['group']).to be_nil + end + end + end +end From d693c3e5cac0fbe5c5e28e0ade43aa7455ba4876 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Mon, 22 Apr 2019 14:46:13 -0500 Subject: [PATCH 2/3] Refactor group query spec and removing unnecessary code --- app/graphql/types/namespace_type.rb | 8 +- spec/requests/api/graphql/group_query_spec.rb | 127 ++++-------------- 2 files changed, 28 insertions(+), 107 deletions(-) diff --git a/app/graphql/types/namespace_type.rb b/app/graphql/types/namespace_type.rb index b1c5da50aa5..36d8ee8c878 100644 --- a/app/graphql/types/namespace_type.rb +++ b/app/graphql/types/namespace_type.rb @@ -5,15 +5,15 @@ module Types graphql_name 'Namespace' field :id, GraphQL::ID_TYPE, null: false - field :name, GraphQL::STRING_TYPE, null: false + field :name, GraphQL::STRING_TYPE, null: false field :path, GraphQL::STRING_TYPE, null: false + field :full_name, GraphQL::STRING_TYPE, null: false + field :full_path, GraphQL::ID_TYPE, null: false + field :description, GraphQL::STRING_TYPE, null: true field :visibility, GraphQL::STRING_TYPE, null: true field :lfs_enabled, GraphQL::BOOLEAN_TYPE, null: true, method: :lfs_enabled? - field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true - field :full_path, GraphQL::ID_TYPE, null: false - field :full_name, GraphQL::STRING_TYPE, null: false end end diff --git a/spec/requests/api/graphql/group_query_spec.rb b/spec/requests/api/graphql/group_query_spec.rb index e4f559e9f10..8ff95cc9af2 100644 --- a/spec/requests/api/graphql/group_query_spec.rb +++ b/spec/requests/api/graphql/group_query_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' # Based on spec/requests/api/groups_spec.rb @@ -6,95 +8,47 @@ describe 'getting group information' do include GraphqlHelpers include UploadHelpers - let(:user1) { create(:user, can_create_group: false) } - let(:user2) { create(:user) } - let(:admin) { create(:admin) } - let!(:group1) { create(:group, avatar: File.open(uploaded_image_temp_path)) } - let!(:group2) { create(:group, :private) } - # let!(:project1) { create(:project, namespace: group1) } - # let!(:project2) { create(:project, namespace: group2) } - # let!(:project3) { create(:project, namespace: group1, path: 'test', visibility_level: Gitlab::VisibilityLevel::PRIVATE) } - - before do - group1.add_owner(user1) - group2.add_owner(user2) - end + let(:user1) { create(:user, can_create_group: false) } + let(:user2) { create(:user) } + let(:admin) { create(:admin) } + let(:public_group) { create(:group, :public) } + let(:private_group) { create(:group, :private) } # similar to the API "GET /groups/:id" describe "Query group(fullPath)" do - # Given a group, create one project for each visibility level - # - # group - Group to add projects to - # share_with - If provided, each project will be shared with this Group - # - # Returns a Hash of visibility_level => Project pairs - def add_projects_to_group(group, share_with: nil) - projects = { - public: create(:project, :public, namespace: group), - internal: create(:project, :internal, namespace: group), - private: create(:project, :private, namespace: group) - } - - if share_with - create(:project_group_link, project: projects[:public], group: share_with) - create(:project_group_link, project: projects[:internal], group: share_with) - create(:project_group_link, project: projects[:private], group: share_with) - end - - projects - end - - def response_project_ids(json_response, key) - json_response[key].map do |project| - project['id'].to_i - end - end - def group_query(group) graphql_query_for('group', 'fullPath' => group.full_path) end it_behaves_like 'a working graphql query' do before do - post_graphql(group_query(group1)) + post_graphql(group_query(public_group)) end end context 'when unauthenticated' do it 'returns nil for a private group' do - post_graphql(group_query(group2)) + post_graphql(group_query(private_group)) expect(graphql_data['group']).to be_nil end it 'returns a public group' do - post_graphql(group_query(group1)) + post_graphql(group_query(public_group)) expect(graphql_data['group']).not_to be_nil end - - # it 'returns only public projects in the group' do - # public_group = create(:group, :public) - # projects = add_projects_to_group(public_group) - # - # get api("/groups/#{public_group.id}") - # - # expect(response_project_ids(json_response, 'projects')) - # .to contain_exactly(projects[:public].id) - # end - - # it 'returns only public projects shared with the group' do - # public_group = create(:group, :public) - # projects = add_projects_to_group(public_group, share_with: group1) - # - # get api("/groups/#{group1.id}") - # - # expect(response_project_ids(json_response, 'shared_projects')) - # .to contain_exactly(projects[:public].id) - # end end context "when authenticated as user" do + let!(:group1) { create(:group, avatar: File.open(uploaded_image_temp_path)) } + let!(:group2) { create(:group, :private) } + + before do + group1.add_owner(user1) + group2.add_owner(user2) + end + it "returns one of user1's groups" do project = create(:project, namespace: group2, path: 'Foo') create(:project_group_link, project: project, group: group1) @@ -113,57 +67,24 @@ describe 'getting group information' do expect(graphql_data['group']['fullName']).to eq(group1.full_name) expect(graphql_data['group']['fullPath']).to eq(group1.full_path) expect(graphql_data['group']['parentId']).to eq(group1.parent_id) - # expect(graphql_data['group']['projects']).to be_an Array - # expect(graphql_data['group']['projects'].length).to eq(2) - # expect(graphql_data['group']['sharedProjects']).to be_an Array - # expect(graphql_data['group']['sharedProjects'].length).to eq(1) - # expect(graphql_data['group']['sharedProjects'][0]['id']).to eq(project.id) end - # it "returns one of user1's groups without projects when with_projects option is set to false" do - # project = create(:project, namespace: group2, path: 'Foo') - # create(:project_group_link, project: project, group: group1) - # - # get api("/groups/#{group1.id}", user1), params: { with_projects: false } - # - # expect(response).to have_gitlab_http_status(200) - # expect(json_response['projects']).to be_nil - # expect(json_response['shared_projects']).to be_nil - # end - it "does not return a non existing group" do query = graphql_query_for('group', 'fullPath' => '1328') + post_graphql(query, current_user: user1) expect(graphql_data['group']).to be_nil end it "does not return a group not attached to user1" do - post_graphql(group_query(group2), current_user: user1) + private_group.add_owner(user2) + + post_graphql(group_query(private_group), current_user: user1) expect(graphql_data['group']).to be_nil end - # it 'returns only public and internal projects in the group' do - # public_group = create(:group, :public) - # projects = add_projects_to_group(public_group) - # - # get api("/groups/#{public_group.id}", user2) - # - # expect(response_project_ids(json_response, 'projects')) - # .to contain_exactly(projects[:public].id, projects[:internal].id) - # end - - # it 'returns only public and internal projects shared with the group' do - # public_group = create(:group, :public) - # projects = add_projects_to_group(public_group, share_with: group1) - # - # get api("/groups/#{group1.id}", user2) - # - # expect(response_project_ids(json_response, 'shared_projects')) - # .to contain_exactly(projects[:public].id, projects[:internal].id) - # end - it 'avoids N+1 queries' do post_graphql(group_query(group1), current_user: admin) @@ -181,9 +102,9 @@ describe 'getting group information' do context "when authenticated as admin" do it "returns any existing group" do - post_graphql(group_query(group2), current_user: admin) + post_graphql(group_query(private_group), current_user: admin) - expect(graphql_data['group']['name']).to eq(group2.name) + expect(graphql_data['group']['name']).to eq(private_group.name) end it "does not return a non existing group" do From ff2727fc7efa92228b022ea46bace698caceced1 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Tue, 23 Apr 2019 12:24:48 -0500 Subject: [PATCH 3/3] Mention group query in GraphQL documentation --- doc/api/graphql/index.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md index ec48bf4940b..cf02bbd9c92 100644 --- a/doc/api/graphql/index.md +++ b/doc/api/graphql/index.md @@ -29,7 +29,11 @@ curl --data "value=100" --header "PRIVATE-TOKEN: " https://gi ## Available queries -A first iteration of a GraphQL API includes a query for: `project`. Within a project it is also possible to fetch a `mergeRequest` by IID. +A first iteration of a GraphQL API includes the following queries + +1. `project` : Within a project it is also possible to fetch a `mergeRequest` by IID. + +1. `group` : Only basic group information is currently supported. ## GraphiQL