From 0c40b42a43644fd2df7c25f247c82343b251424f Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Tue, 1 Aug 2023 16:27:55 +0530 Subject: [PATCH 01/19] Add a GraphQL query to get organization groups The query supports multiple sort options for sorting the groups. Everything is experimental therefore all fields/arguements have been marked as alpha. Changelog: added --- .../organizations/organization_resolver.rb | 22 +++++++++++++++ .../types/organizations/organization_type.rb | 27 +++++++++++++++++++ app/graphql/types/query_type.rb | 6 +++++ 3 files changed, 55 insertions(+) create mode 100644 app/graphql/resolvers/organizations/organization_resolver.rb create mode 100644 app/graphql/types/organizations/organization_type.rb diff --git a/app/graphql/resolvers/organizations/organization_resolver.rb b/app/graphql/resolvers/organizations/organization_resolver.rb new file mode 100644 index 00000000000000..9194d9a32c5dd9 --- /dev/null +++ b/app/graphql/resolvers/organizations/organization_resolver.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Resolvers + module Organizations + class OrganizationResolver < BaseResolver + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorize :read_organization + + type Types::Organizations::OrganizationType, null: true + + argument :id, + Types::GlobalIDType[::Organizations::Organization], + required: true, + description: 'ID of the organization.' + + def resolve(id:) + authorized_find!(id: id) + end + end + end +end diff --git a/app/graphql/types/organizations/organization_type.rb b/app/graphql/types/organizations/organization_type.rb new file mode 100644 index 00000000000000..fb95f5b5b0f88c --- /dev/null +++ b/app/graphql/types/organizations/organization_type.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Types + module Organizations + class OrganizationType < BaseObject + graphql_name 'Organization' + + authorize :read_organization + + field :id, + GraphQL::Types::ID, + null: false, + description: 'ID of the organization.', + alpha: { milestone: '16.3' } + field :name, + GraphQL::Types::String, + null: false, + description: 'Name of the organization.', + alpha: { milestone: '16.3' } + field :path, + GraphQL::Types::String, + null: false, + description: 'Path of the organization.', + alpha: { milestone: '16.3' } + end + end +end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 38b8973034d3b8..c29b1b8fb75f16 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -96,6 +96,12 @@ class QueryType < ::Types::BaseObject required: true, description: 'Global ID of the note.' end + field :organization, + Types::Organizations::OrganizationType, + null: true, + resolver: Resolvers::Organizations::OrganizationResolver, + description: "Find an organization.", + alpha: { milestone: '16.3' } field :package, description: 'Find a package. This field can only be resolved for one query in any single request. Returns `null` if a package has no `default` status.', resolver: Resolvers::PackageDetailsResolver -- GitLab From 1235c47f0b1b6e2010cd0b42f63372e6fe1b6c7a Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 7 Aug 2023 12:32:31 +0530 Subject: [PATCH 02/19] Add Organizations::GroupsFinder to find groups belonging to organization The finder supports searching and sorting. --- app/finders/organizations/groups_finder.rb | 63 ++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 app/finders/organizations/groups_finder.rb diff --git a/app/finders/organizations/groups_finder.rb b/app/finders/organizations/groups_finder.rb new file mode 100644 index 00000000000000..ea88bb5a2bedd5 --- /dev/null +++ b/app/finders/organizations/groups_finder.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +# Organizations::GroupsFinder +# +# Used to find Groups within an Organization +module Organizations + class GroupsFinder + # @param organization [Organizations::Organization] + # @param current_user [User] + # @param params [{ sort: [String], search: [String] }] + def initialize(organization:, current_user:, params: {}) + @organization = organization + @current_user = current_user + @params = params + end + + def execute + return Group.none if organization.nil? || !authorized? + + filter_groups(all_accessible_groups) + .then { |groups| sort(groups) } + .then(&:with_route) + end + + private + + attr_reader :organization, :params, :current_user + + def all_accessible_groups + organization.groups.public_or_visible_to_user(current_user) + end + + def filter_groups(groups) + by_search(groups) + end + + def by_search(groups) + return groups unless params[:search].present? + + groups.search(params[:search]) + end + + def sort(groups) + return default_sort_order(groups) if params[:sort].blank? + + if params[:sort] == :similarity + return groups.sorted_by_similarity_and_parent_id_desc(params[:search]) if params[:search].present? + + return default_sort_order(groups) + end + + groups.sort_by_attribute(params[:sort]) + end + + def default_sort_order(groups) + groups.sort_by_attribute('name_asc') + end + + def authorized? + Ability.allowed?(current_user, :read_organization, organization) + end + end +end -- GitLab From f810ebd6a3f8038d68fbeb64d2c235034c8f739d Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 7 Aug 2023 12:37:48 +0530 Subject: [PATCH 03/19] Add groups connection type to organization query By default there's a limit of 100 nodes in the GraphQL schema. To request more groups pagination can be used. --- app/finders/organizations/groups_finder.rb | 10 ++----- .../organizations/groups_resolver.rb | 28 +++++++++++++++++++ .../types/organizations/organization_type.rb | 6 ++++ 3 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 app/graphql/resolvers/organizations/groups_resolver.rb diff --git a/app/finders/organizations/groups_finder.rb b/app/finders/organizations/groups_finder.rb index ea88bb5a2bedd5..8ab9afc5d17f14 100644 --- a/app/finders/organizations/groups_finder.rb +++ b/app/finders/organizations/groups_finder.rb @@ -43,13 +43,9 @@ def by_search(groups) def sort(groups) return default_sort_order(groups) if params[:sort].blank? - if params[:sort] == :similarity - return groups.sorted_by_similarity_and_parent_id_desc(params[:search]) if params[:search].present? - - return default_sort_order(groups) - end - - groups.sort_by_attribute(params[:sort]) + field = params[:sort][:field] + direction = params[:sort][:direction] + groups.reorder(field => direction) # rubocop: disable CodeReuse/ActiveRecord end def default_sort_order(groups) diff --git a/app/graphql/resolvers/organizations/groups_resolver.rb b/app/graphql/resolvers/organizations/groups_resolver.rb new file mode 100644 index 00000000000000..23907d80d3c3af --- /dev/null +++ b/app/graphql/resolvers/organizations/groups_resolver.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Resolvers + module Organizations + class GroupsResolver < BaseResolver + include Gitlab::Graphql::Authorize::AuthorizeResource + include ResolvesGroups + + type Types::GroupType.connection_type, null: true + + authorize :read_group + + argument :search, + GraphQL::Types::String, + required: false, + description: 'Search query for group name or full path.', + alpha: { milestone: '16.3' } + + private + + def resolve_groups(**args) + ::Organizations::GroupsFinder + .new(organization: object, current_user: context[:current_user], params: args) + .execute + end + end + end +end diff --git a/app/graphql/types/organizations/organization_type.rb b/app/graphql/types/organizations/organization_type.rb index fb95f5b5b0f88c..938ab310b2da51 100644 --- a/app/graphql/types/organizations/organization_type.rb +++ b/app/graphql/types/organizations/organization_type.rb @@ -7,6 +7,12 @@ class OrganizationType < BaseObject authorize :read_organization + field :groups, + Types::GroupType.connection_type, + null: false, + description: 'Groups within this organization where the user has access.', + alpha: { milestone: '16.3' }, + resolver: ::Resolvers::Organizations::GroupsResolver field :id, GraphQL::Types::ID, null: false, -- GitLab From bb57370a97c8e2814ce86f6691d0a14cbd05fde6 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 7 Aug 2023 14:55:09 +0530 Subject: [PATCH 04/19] Add sorting to Organizations::GroupsResolver To support sorting Organizations::GroupSortEnum is added. --- .../organizations/groups_resolver.rb | 7 ++++++ .../types/organizations/group_sort_enum.rb | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 app/graphql/types/organizations/group_sort_enum.rb diff --git a/app/graphql/resolvers/organizations/groups_resolver.rb b/app/graphql/resolvers/organizations/groups_resolver.rb index 23907d80d3c3af..2e1ffb3bd86da9 100644 --- a/app/graphql/resolvers/organizations/groups_resolver.rb +++ b/app/graphql/resolvers/organizations/groups_resolver.rb @@ -16,6 +16,13 @@ class GroupsResolver < BaseResolver description: 'Search query for group name or full path.', alpha: { milestone: '16.3' } + argument :sort, + Types::Organizations::GroupSortEnum, + description: 'Criteria to sort organization groups by.', + required: false, + default_value: { field: 'name', direction: :asc }, + alpha: { milestone: '16.3' } + private def resolve_groups(**args) diff --git a/app/graphql/types/organizations/group_sort_enum.rb b/app/graphql/types/organizations/group_sort_enum.rb new file mode 100644 index 00000000000000..236c929b5705de --- /dev/null +++ b/app/graphql/types/organizations/group_sort_enum.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Types + module Organizations + class GroupSortEnum < BaseEnum + graphql_name 'OrganizationGroupSort' + description 'Values for sorting organization groups' + + sortable_fields = ['Id', 'Name', 'Path', 'Updated at', 'Created at'] + + sortable_fields.each do |field| + value "#{field.upcase.tr(' ', '_')}_ASC", + value: { field: field.downcase.tr(' ', '_'), direction: :asc }, + description: "#{field} in ascending order.", + alpha: { milestone: '16.3' } + + value "#{field.upcase.tr(' ', '_')}_DESC", + value: { field: field.downcase.tr(' ', '_'), direction: :desc }, + description: "#{field} in descending order.", + alpha: { milestone: '16.3' } + end + end + end +end -- GitLab From f89464f732f6a702d8b751301a5aac0859ab15d1 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 7 Aug 2023 15:35:00 +0530 Subject: [PATCH 05/19] Simplify query to check if organization user exists The `organization_users` table already contain the `user_id` column so we don't need to join on the `users` table to check the existence of the user. --- app/models/organizations/organization.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/organizations/organization.rb b/app/models/organizations/organization.rb index 9f2119949fba21..489fd6e0da7c19 100644 --- a/app/models/organizations/organization.rb +++ b/app/models/organizations/organization.rb @@ -38,7 +38,7 @@ def to_param end def user?(user) - users.exists?(user.id) + organization_users.exists?(user: user) end private -- GitLab From f9e3faa0b24c832ea8476ccd51fdc1748658f520 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 7 Aug 2023 15:50:20 +0530 Subject: [PATCH 06/19] Update GraphQL documentation after adding organization query All the arguments and fields have been marked as alpha as they are experimental. --- doc/api/graphql/reference/index.md | 72 ++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index aa679dad425e02..c87e1857e14569 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -532,6 +532,22 @@ Returns [`Note`](#note). | ---- | ---- | ----------- | | `id` | [`NoteID!`](#noteid) | Global ID of the note. | +### `Query.organization` + +Find an organization. + +WARNING: +**Introduced** in 16.3. +This feature is an Experiment. It can be changed or removed at any time. + +Returns [`Organization`](#organization). + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `id` | [`OrganizationsOrganizationID!`](#organizationsorganizationid) | ID of the organization. | + ### `Query.package` Find a package. This field can only be resolved for one query in any single request. Returns `null` if a package has no `default` status. @@ -20221,6 +20237,39 @@ Active period time range for on-call rotation. | `endTime` | [`String`](#string) | End of the rotation active period. | | `startTime` | [`String`](#string) | Start of the rotation active period. | +### `Organization` + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `id` **{warning-solid}** | [`ID!`](#id) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. ID of the organization. | +| `name` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Name of the organization. | +| `path` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Path of the organization. | + +#### Fields with arguments + +##### `Organization.groups` + +Groups within this organization where the user has access. + +WARNING: +**Introduced** in 16.3. +This feature is an Experiment. It can be changed or removed at any time. + +Returns [`GroupConnection!`](#groupconnection). + +This field returns a [connection](#connections). It accepts the +four standard [pagination arguments](#connection-pagination-arguments): +`before: String`, `after: String`, `first: Int`, `last: Int`. + +###### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `search` **{warning-solid}** | [`String`](#string) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Search query for group name or full path. | +| `sort` **{warning-solid}** | [`OrganizationGroupSort`](#organizationgroupsort) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Criteria to sort organization groups by. | + ### `OrganizationStateCounts` Represents the total number of organizations for the represented states. @@ -27215,6 +27264,23 @@ Rotation length unit of an on-call rotation. | `HOURS` | Hours. | | `WEEKS` | Weeks. | +### `OrganizationGroupSort` + +Values for sorting organization groups. + +| Value | Description | +| ----- | ----------- | +| `CREATED_AT_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Created at in ascending order. | +| `CREATED_AT_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Created at in descending order. | +| `ID_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Id in ascending order. | +| `ID_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Id in descending order. | +| `NAME_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Name in ascending order. | +| `NAME_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Name in descending order. | +| `PATH_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Path in ascending order. | +| `PATH_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Path in descending order. | +| `UPDATED_AT_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Updated at in ascending order. | +| `UPDATED_AT_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Updated at in descending order. | + ### `OrganizationSort` Values for sorting organizations. @@ -28688,6 +28754,12 @@ A `NoteableID` is a global ID. It is encoded as a string. An example `NoteableID` is: `"gid://gitlab/Noteable/1"`. +### `OrganizationsOrganizationID` + +A `OrganizationsOrganizationID` is a global ID. It is encoded as a string. + +An example `OrganizationsOrganizationID` is: `"gid://gitlab/Organizations::Organization/1"`. + ### `PackagesConanFileMetadatumID` A `PackagesConanFileMetadatumID` is a global ID. It is encoded as a string. -- GitLab From b8fae68403fd26e4272e87a71b32f288ceadcc6a Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 7 Aug 2023 16:32:54 +0530 Subject: [PATCH 07/19] Add specs for organization query Add specs for OrganizationType, GroupSortEnum and resolvers as well. Add organization_user association to organization factory. --- spec/factories/organizations/organizations.rb | 1 + .../organizations/groups_resolver_spec.rb | 51 +++++++++++++++++++ .../organization_resolver_spec.rb | 34 +++++++++++++ .../organizations/group_sort_enum_spec.rb | 26 ++++++++++ .../organizations/organization_type_spec.rb | 11 ++++ spec/graphql/types/query_type_spec.rb | 10 ++++ 6 files changed, 133 insertions(+) create mode 100644 spec/graphql/resolvers/organizations/groups_resolver_spec.rb create mode 100644 spec/graphql/resolvers/organizations/organization_resolver_spec.rb create mode 100644 spec/graphql/types/organizations/group_sort_enum_spec.rb create mode 100644 spec/graphql/types/organizations/organization_type_spec.rb diff --git a/spec/factories/organizations/organizations.rb b/spec/factories/organizations/organizations.rb index f88ef0462486a2..e6e2eece0e2503 100644 --- a/spec/factories/organizations/organizations.rb +++ b/spec/factories/organizations/organizations.rb @@ -6,6 +6,7 @@ factory :organization, class: 'Organizations::Organization' do sequence(:name) { |n| "Organization ##{n}" } path { name.parameterize } + organization_users { [association(:organization_user, organization: instance)] } trait :default do id { Organizations::Organization::DEFAULT_ORGANIZATION_ID } diff --git a/spec/graphql/resolvers/organizations/groups_resolver_spec.rb b/spec/graphql/resolvers/organizations/groups_resolver_spec.rb new file mode 100644 index 00000000000000..d31d91cab83201 --- /dev/null +++ b/spec/graphql/resolvers/organizations/groups_resolver_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Resolvers::Organizations::GroupsResolver, feature_category: :cell do + include GraphqlHelpers + + describe '#resolve_groups' do + let_it_be(:organization) { create(:organization) } + let_it_be(:user) { organization.users.first } + let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } + let_it_be(:private_group) do + create(:group, :private, name: 'private-group', organization: organization) + end + + let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } + let_it_be(:group_outside_organization) { create(:group) } + + let(:params) { {} } + + subject(:resolved_groups) do + resolve(described_class, obj: organization, args: params, ctx: { current_user: user }) + end + + before_all do + private_group.add_developer(user) + public_group.add_developer(user) + other_group.add_developer(user) + end + + it 'includes accessible groups' do + expect(subject).to contain_exactly(public_group, private_group, other_group) + end + + context 'with `search` argument' do + let(:params) { { search: 'oth' } } + + it 'filters groups by name' do + expect(subject).to contain_exactly(other_group) + end + end + + context 'with `sort` argument' do + let(:params) { { sort: { field: "name", direction: :desc } } } + + it 'orders by name ascending' do + expect(subject.map(&:name)).to eq(%w[other-group private-group public-group]) + end + end + end +end diff --git a/spec/graphql/resolvers/organizations/organization_resolver_spec.rb b/spec/graphql/resolvers/organizations/organization_resolver_spec.rb new file mode 100644 index 00000000000000..b280a7773e897f --- /dev/null +++ b/spec/graphql/resolvers/organizations/organization_resolver_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Resolvers::Organizations::OrganizationResolver, feature_category: :cell do + include GraphqlHelpers + + let_it_be(:organization) { create(:organization) } + + let(:params) { { id: global_id_of(organization) } } + let(:current_user) { organization.users.first } + + subject(:resolved_organization) { resolve_organization } + + context 'when the user can read the organization' do + it { is_expected.to eq(organization) } + end + + context 'when the user cannot read the organization' do + let(:current_user) { create(:user) } + + it 'raises a resource not available error' do + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do + resolve_organization + end + end + end + + private + + def resolve_organization + resolve(described_class, args: params, ctx: { current_user: current_user }) + end +end diff --git a/spec/graphql/types/organizations/group_sort_enum_spec.rb b/spec/graphql/types/organizations/group_sort_enum_spec.rb new file mode 100644 index 00000000000000..57915d95c45e16 --- /dev/null +++ b/spec/graphql/types/organizations/group_sort_enum_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['OrganizationGroupSort'], feature_category: :cell do + let(:sort_values) do + %w[ + ID_ASC + ID_DESC + NAME_ASC + NAME_DESC + PATH_ASC + PATH_DESC + UPDATED_AT_ASC + UPDATED_AT_DESC + CREATED_AT_ASC + CREATED_AT_DESC + ] + end + + specify { expect(described_class.graphql_name).to eq('OrganizationGroupSort') } + + it 'exposes all the organization groups sort values' do + expect(described_class.values.keys).to include(*sort_values) + end +end diff --git a/spec/graphql/types/organizations/organization_type_spec.rb b/spec/graphql/types/organizations/organization_type_spec.rb new file mode 100644 index 00000000000000..78c5dea73088e5 --- /dev/null +++ b/spec/graphql/types/organizations/organization_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['Organization'], feature_category: :cell do + let(:expected_fields) { %w[groups id name path] } + + specify { expect(described_class.graphql_name).to eq('Organization') } + specify { expect(described_class).to require_graphql_authorizations(:read_organization) } + specify { expect(described_class).to have_graphql_fields(*expected_fields) } +end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index 100ecc94f355ae..4471b9e1dda952 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -23,6 +23,16 @@ end end + describe 'organization field' do + subject { described_class.fields['organization'] } + + it 'finds projects by full path' do + is_expected.to have_graphql_arguments(:id) + is_expected.to have_graphql_type(Types::Organizations::OrganizationType) + is_expected.to have_graphql_resolver(Resolvers::Organizations::OrganizationResolver) + end + end + describe 'project field' do subject { described_class.fields['project'] } -- GitLab From bb557b227e6373f86f9c4220fb4647175161b40d Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Tue, 8 Aug 2023 14:35:26 +0530 Subject: [PATCH 08/19] Add GraphQL request specs for organization query Also, remove the OrganizationResolver and GroupsResolver unit tests as adding unit tests for resolvers has been deprecated. --- .../organizations/groups_resolver_spec.rb | 51 ---------- .../organization_resolver_spec.rb | 34 ------- .../organizations/organization_query_spec.rb | 97 +++++++++++++++++++ 3 files changed, 97 insertions(+), 85 deletions(-) delete mode 100644 spec/graphql/resolvers/organizations/groups_resolver_spec.rb delete mode 100644 spec/graphql/resolvers/organizations/organization_resolver_spec.rb create mode 100644 spec/requests/api/graphql/organizations/organization_query_spec.rb diff --git a/spec/graphql/resolvers/organizations/groups_resolver_spec.rb b/spec/graphql/resolvers/organizations/groups_resolver_spec.rb deleted file mode 100644 index d31d91cab83201..00000000000000 --- a/spec/graphql/resolvers/organizations/groups_resolver_spec.rb +++ /dev/null @@ -1,51 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Resolvers::Organizations::GroupsResolver, feature_category: :cell do - include GraphqlHelpers - - describe '#resolve_groups' do - let_it_be(:organization) { create(:organization) } - let_it_be(:user) { organization.users.first } - let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } - let_it_be(:private_group) do - create(:group, :private, name: 'private-group', organization: organization) - end - - let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } - let_it_be(:group_outside_organization) { create(:group) } - - let(:params) { {} } - - subject(:resolved_groups) do - resolve(described_class, obj: organization, args: params, ctx: { current_user: user }) - end - - before_all do - private_group.add_developer(user) - public_group.add_developer(user) - other_group.add_developer(user) - end - - it 'includes accessible groups' do - expect(subject).to contain_exactly(public_group, private_group, other_group) - end - - context 'with `search` argument' do - let(:params) { { search: 'oth' } } - - it 'filters groups by name' do - expect(subject).to contain_exactly(other_group) - end - end - - context 'with `sort` argument' do - let(:params) { { sort: { field: "name", direction: :desc } } } - - it 'orders by name ascending' do - expect(subject.map(&:name)).to eq(%w[other-group private-group public-group]) - end - end - end -end diff --git a/spec/graphql/resolvers/organizations/organization_resolver_spec.rb b/spec/graphql/resolvers/organizations/organization_resolver_spec.rb deleted file mode 100644 index b280a7773e897f..00000000000000 --- a/spec/graphql/resolvers/organizations/organization_resolver_spec.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Resolvers::Organizations::OrganizationResolver, feature_category: :cell do - include GraphqlHelpers - - let_it_be(:organization) { create(:organization) } - - let(:params) { { id: global_id_of(organization) } } - let(:current_user) { organization.users.first } - - subject(:resolved_organization) { resolve_organization } - - context 'when the user can read the organization' do - it { is_expected.to eq(organization) } - end - - context 'when the user cannot read the organization' do - let(:current_user) { create(:user) } - - it 'raises a resource not available error' do - expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do - resolve_organization - end - end - end - - private - - def resolve_organization - resolve(described_class, args: params, ctx: { current_user: current_user }) - end -end diff --git a/spec/requests/api/graphql/organizations/organization_query_spec.rb b/spec/requests/api/graphql/organizations/organization_query_spec.rb new file mode 100644 index 00000000000000..b9a31cd5089a56 --- /dev/null +++ b/spec/requests/api/graphql/organizations/organization_query_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting organization information', feature_category: :cell do + include GraphqlHelpers + + let(:organization_fields) { all_graphql_fields_for('Organization', max_depth: 1) } + let(:query) { graphql_query_for(:organization, { id: organization.to_global_id }, organization_fields) } + let(:current_user) { user } + let(:groups) { graphql_data_at(:organization, :groups, :edges, :node) } + + let_it_be(:organization) { create(:organization) } + let_it_be(:user) { organization.users.first } + let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } + let_it_be(:private_group) do + create(:group, :private, name: 'private-group', organization: organization) + end + + let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } + let_it_be(:group_outside_organization) { create(:group) } + + before_all do + private_group.add_developer(user) + public_group.add_developer(user) + other_group.add_developer(user) + group_outside_organization.add_developer(user) + end + + subject(:request_organization) { post_graphql(query, current_user: current_user) } + + context 'when the user does not have access to the organization' do + let(:current_user) { create(:user) } + + it 'returns an empty field' do + request_organization + + expect(graphql_data['organization']).to be_nil + end + end + + context 'when user has access to the organization' do + it_behaves_like 'a working graphql query' do + before do + request_organization + end + end + + context 'with `search` argument' do + let(:search) { 'oth' } + let(:organization_fields) do + <<~FIELDS + id + path + groups(search: "#{search}") { + edges { + node { + id + name + } + } + } + FIELDS + end + + it 'filters groups by name' do + request_organization + + expect(groups).to contain_exactly(a_graphql_entity_for(other_group)) + end + end + + context 'with `sort` argument' do + let(:sort) { 'NAME_DESC' } + let(:organization_fields) do + <<~FIELDS + id + path + groups(sort: #{sort}) { + edges { + node { + id + name + } + } + } + FIELDS + end + + it 'orders by name descending' do + request_organization + + expect(groups.pluck('name')).to eq(%w[public-group private-group other-group]) + end + end + end +end -- GitLab From 3ae3c25276bffceda02bd6125f1459b9d014cd88 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Tue, 8 Aug 2023 15:23:25 +0530 Subject: [PATCH 09/19] Add specs for Organizations::GroupsFinder For sorting, searching and authorization. --- .../organizations/groups_finder_spec.rb | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 spec/finders/organizations/groups_finder_spec.rb diff --git a/spec/finders/organizations/groups_finder_spec.rb b/spec/finders/organizations/groups_finder_spec.rb new file mode 100644 index 00000000000000..8306e1e47f59a9 --- /dev/null +++ b/spec/finders/organizations/groups_finder_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Organizations::GroupsFinder, feature_category: :cell do + include AdminModeHelper + + let(:current_user) { user } + let(:params) { {} } + let(:finder) { described_class.new(organization: organization, current_user: current_user, params: params) } + + let_it_be(:organization) { create(:organization) } + let_it_be(:user) { organization.users.first } + let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } + let_it_be(:private_group) do + create(:group, :private, name: 'private-group', organization: organization) + end + + let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } + let_it_be(:group_outside_organization) { create(:group) } + + before_all do + private_group.add_developer(user) + public_group.add_developer(user) + other_group.add_developer(user) + group_outside_organization.add_developer(user) + end + + subject(:result) { finder.execute.to_a } + + describe '#execute' do + context 'when user is not authorized to read the organization' do + let(:current_user) { create(:user) } + + it { is_expected.to be_empty } + end + + context 'when organization is nil' do + let(:finder) { described_class.new(organization: nil, current_user: current_user, params: params) } + + it { is_expected.to be_empty } + end + + context 'when user is authorized to read the organization' do + it 'return all accessible groups' do + expect(result).to contain_exactly(public_group, private_group, other_group) + end + + context 'when search param is passed' do + let(:params) { { search: 'the' } } + + it 'filters the groups by search' do + expect(result).to contain_exactly(other_group) + end + end + + context 'when sort param is not passed' do + it 'return groups sorted by name in ascending order by default' do + expect(result).to eq([other_group, private_group, public_group]) + end + end + + context 'when sort param is passed' do + using RSpec::Parameterized::TableSyntax + + where(:field, :direction, :sorted_groups) do + 'name' | 'asc' | lazy { [other_group, private_group, public_group] } + 'name' | 'desc' | lazy { [public_group, private_group, other_group] } + 'path' | 'asc' | lazy { [other_group, private_group, public_group] } + 'path' | 'desc' | lazy { [public_group, private_group, other_group] } + end + + with_them do + let(:params) { { sort: { field: field, direction: direction } } } + it 'sorts the groups' do + expect(result).to eq(sorted_groups) + end + end + end + end + end +end -- GitLab From 9f39f229d85dc48a8b0e238497992e99b4ba5757 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Tue, 8 Aug 2023 17:06:42 +0530 Subject: [PATCH 10/19] Return only authorized groups --- app/finders/organizations/groups_finder.rb | 2 +- app/models/namespace.rb | 1 + spec/finders/organizations/groups_finder_spec.rb | 7 +++++-- .../api/graphql/organizations/organization_query_spec.rb | 8 ++++++-- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/finders/organizations/groups_finder.rb b/app/finders/organizations/groups_finder.rb index 8ab9afc5d17f14..cddccb5eebe108 100644 --- a/app/finders/organizations/groups_finder.rb +++ b/app/finders/organizations/groups_finder.rb @@ -27,7 +27,7 @@ def execute attr_reader :organization, :params, :current_user def all_accessible_groups - organization.groups.public_or_visible_to_user(current_user) + current_user.authorized_groups.in_organization(organization) end def filter_groups(groups) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index abf4ffaead4924..a81d0f047ba1a2 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -166,6 +166,7 @@ class Namespace < ApplicationRecord scope :include_route, -> { includes(:route) } scope :by_parent, -> (parent) { where(parent_id: parent) } scope :filter_by_path, -> (query) { where('lower(path) = :query', query: query.downcase) } + scope :in_organization, -> (organization) { where(organization: organization) } scope :with_statistics, -> do joins('LEFT JOIN project_statistics ps ON ps.namespace_id = namespaces.id') diff --git a/spec/finders/organizations/groups_finder_spec.rb b/spec/finders/organizations/groups_finder_spec.rb index 8306e1e47f59a9..1a779639a1d430 100644 --- a/spec/finders/organizations/groups_finder_spec.rb +++ b/spec/finders/organizations/groups_finder_spec.rb @@ -12,12 +12,15 @@ let_it_be(:organization) { create(:organization) } let_it_be(:user) { organization.users.first } let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } + let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } + let_it_be(:group_outside_organization) { create(:group) } let_it_be(:private_group) do create(:group, :private, name: 'private-group', organization: organization) end - let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } - let_it_be(:group_outside_organization) { create(:group) } + let_it_be(:no_access_group_in_org) do + create(:group, :private, name: 'no-access', organization: organization) + end before_all do private_group.add_developer(user) diff --git a/spec/requests/api/graphql/organizations/organization_query_spec.rb b/spec/requests/api/graphql/organizations/organization_query_spec.rb index b9a31cd5089a56..10d9e7f98c9e40 100644 --- a/spec/requests/api/graphql/organizations/organization_query_spec.rb +++ b/spec/requests/api/graphql/organizations/organization_query_spec.rb @@ -13,12 +13,16 @@ let_it_be(:organization) { create(:organization) } let_it_be(:user) { organization.users.first } let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } + let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } + let_it_be(:group_outside_organization) { create(:group) } + let_it_be(:private_group) do create(:group, :private, name: 'private-group', organization: organization) end - let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } - let_it_be(:group_outside_organization) { create(:group) } + let_it_be(:no_access_group_in_org) do + create(:group, :private, name: 'no-access', organization: organization) + end before_all do private_group.add_developer(user) -- GitLab From ceddae122e428759c33e653377cdc4f79ed8d9a6 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Wed, 9 Aug 2023 10:03:53 +0530 Subject: [PATCH 11/19] Remove inline association to fix organization_spec.rb --- spec/factories/organizations/organizations.rb | 1 - spec/finders/organizations/groups_finder_spec.rb | 5 +++-- .../api/graphql/organizations/organization_query_spec.rb | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/factories/organizations/organizations.rb b/spec/factories/organizations/organizations.rb index e6e2eece0e2503..f88ef0462486a2 100644 --- a/spec/factories/organizations/organizations.rb +++ b/spec/factories/organizations/organizations.rb @@ -6,7 +6,6 @@ factory :organization, class: 'Organizations::Organization' do sequence(:name) { |n| "Organization ##{n}" } path { name.parameterize } - organization_users { [association(:organization_user, organization: instance)] } trait :default do id { Organizations::Organization::DEFAULT_ORGANIZATION_ID } diff --git a/spec/finders/organizations/groups_finder_spec.rb b/spec/finders/organizations/groups_finder_spec.rb index 1a779639a1d430..a7286639150542 100644 --- a/spec/finders/organizations/groups_finder_spec.rb +++ b/spec/finders/organizations/groups_finder_spec.rb @@ -9,8 +9,9 @@ let(:params) { {} } let(:finder) { described_class.new(organization: organization, current_user: current_user, params: params) } - let_it_be(:organization) { create(:organization) } - let_it_be(:user) { organization.users.first } + let_it_be(:organization_user) { create(:organization_user) } + let_it_be(:organization) { organization_user.organization } + let_it_be(:user) { organization_user.user } let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } let_it_be(:group_outside_organization) { create(:group) } diff --git a/spec/requests/api/graphql/organizations/organization_query_spec.rb b/spec/requests/api/graphql/organizations/organization_query_spec.rb index 10d9e7f98c9e40..389c4a869b2153 100644 --- a/spec/requests/api/graphql/organizations/organization_query_spec.rb +++ b/spec/requests/api/graphql/organizations/organization_query_spec.rb @@ -10,8 +10,9 @@ let(:current_user) { user } let(:groups) { graphql_data_at(:organization, :groups, :edges, :node) } - let_it_be(:organization) { create(:organization) } - let_it_be(:user) { organization.users.first } + let_it_be(:organization_user) { create(:organization_user) } + let_it_be(:organization) { organization_user.organization } + let_it_be(:user) { organization_user.user } let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } let_it_be(:group_outside_organization) { create(:group) } -- GitLab From f510f1a311fec4e81ed16aba12daa1e3035abb6d Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Wed, 9 Aug 2023 11:10:50 +0530 Subject: [PATCH 12/19] Add more specs for sort options Rename a variable. --- .../organizations/groups_finder_spec.rb | 4 +-- .../organizations/organization_query_spec.rb | 36 +++++++++++++------ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/spec/finders/organizations/groups_finder_spec.rb b/spec/finders/organizations/groups_finder_spec.rb index a7286639150542..972d80ab036789 100644 --- a/spec/finders/organizations/groups_finder_spec.rb +++ b/spec/finders/organizations/groups_finder_spec.rb @@ -14,7 +14,7 @@ let_it_be(:user) { organization_user.user } let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } - let_it_be(:group_outside_organization) { create(:group) } + let_it_be(:outside_organization_group) { create(:group) } let_it_be(:private_group) do create(:group, :private, name: 'private-group', organization: organization) end @@ -27,7 +27,7 @@ private_group.add_developer(user) public_group.add_developer(user) other_group.add_developer(user) - group_outside_organization.add_developer(user) + outside_organization_group.add_developer(user) end subject(:result) { finder.execute.to_a } diff --git a/spec/requests/api/graphql/organizations/organization_query_spec.rb b/spec/requests/api/graphql/organizations/organization_query_spec.rb index 389c4a869b2153..63c91f6f752dd1 100644 --- a/spec/requests/api/graphql/organizations/organization_query_spec.rb +++ b/spec/requests/api/graphql/organizations/organization_query_spec.rb @@ -15,7 +15,7 @@ let_it_be(:user) { organization_user.user } let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } - let_it_be(:group_outside_organization) { create(:group) } + let_it_be(:outside_organization_group) { create(:group) } let_it_be(:private_group) do create(:group, :private, name: 'private-group', organization: organization) @@ -29,7 +29,7 @@ private_group.add_developer(user) public_group.add_developer(user) other_group.add_developer(user) - group_outside_organization.add_developer(user) + outside_organization_group.add_developer(user) end subject(:request_organization) { post_graphql(query, current_user: current_user) } @@ -76,26 +76,40 @@ end context 'with `sort` argument' do - let(:sort) { 'NAME_DESC' } - let(:organization_fields) do - <<~FIELDS + using RSpec::Parameterized::TableSyntax + + let(:authorized_groups) { [public_group, private_group, other_group] } + + where(:field, :direction, :sorted_groups) do + 'id' | 'asc' | lazy { authorized_groups.sort_by(&:id) } + 'id' | 'desc' | lazy { authorized_groups.sort_by(&:id).reverse } + 'name' | 'asc' | lazy { authorized_groups.sort_by(&:name) } + 'name' | 'desc' | lazy { authorized_groups.sort_by(&:name).reverse } + 'path' | 'asc' | lazy { authorized_groups.sort_by(&:path) } + 'path' | 'desc' | lazy { authorized_groups.sort_by(&:path).reverse } + end + + with_them do + let(:sort) { "#{field}_#{direction}".upcase } + let(:organization_fields) do + <<~FIELDS id path groups(sort: #{sort}) { edges { node { id - name } } } - FIELDS - end + FIELDS + end - it 'orders by name descending' do - request_organization + it 'sorts the groups' do + request_organization - expect(groups.pluck('name')).to eq(%w[public-group private-group other-group]) + expect(groups.pluck('id')).to eq(sorted_groups.map(&:to_global_id).map(&:to_s)) + end end end end -- GitLab From 8a74eee9fdf2661277f5b7eccc6410c6c114371e Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Fri, 11 Aug 2023 14:40:56 +0530 Subject: [PATCH 13/19] Add resolve_organization_groups feature flag In case authorized_groups in Organizations::GroupsFinder performs poorly. The feature flag is enabled by default. --- .../organizations/groups_resolver.rb | 2 + .../resolve_organization_groups.yml | 8 ++++ .../organizations/organization_query_spec.rb | 41 +++++++++++++++---- 3 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 config/feature_flags/development/resolve_organization_groups.yml diff --git a/app/graphql/resolvers/organizations/groups_resolver.rb b/app/graphql/resolvers/organizations/groups_resolver.rb index 2e1ffb3bd86da9..7516f12d2ecef0 100644 --- a/app/graphql/resolvers/organizations/groups_resolver.rb +++ b/app/graphql/resolvers/organizations/groups_resolver.rb @@ -26,6 +26,8 @@ class GroupsResolver < BaseResolver private def resolve_groups(**args) + return Group.none if Feature.disabled?(:resolve_organization_groups, context[:current_user]) + ::Organizations::GroupsFinder .new(organization: object, current_user: context[:current_user], params: args) .execute diff --git a/config/feature_flags/development/resolve_organization_groups.yml b/config/feature_flags/development/resolve_organization_groups.yml new file mode 100644 index 00000000000000..7a70c8568a6211 --- /dev/null +++ b/config/feature_flags/development/resolve_organization_groups.yml @@ -0,0 +1,8 @@ +--- +name: resolve_organization_groups +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/128733 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/421673 +milestone: '16.3' +type: development +group: group::tenant scale +default_enabled: true diff --git a/spec/requests/api/graphql/organizations/organization_query_spec.rb b/spec/requests/api/graphql/organizations/organization_query_spec.rb index 63c91f6f752dd1..1a933eea45b5d7 100644 --- a/spec/requests/api/graphql/organizations/organization_query_spec.rb +++ b/spec/requests/api/graphql/organizations/organization_query_spec.rb @@ -5,10 +5,22 @@ RSpec.describe 'getting organization information', feature_category: :cell do include GraphqlHelpers - let(:organization_fields) { all_graphql_fields_for('Organization', max_depth: 1) } let(:query) { graphql_query_for(:organization, { id: organization.to_global_id }, organization_fields) } let(:current_user) { user } let(:groups) { graphql_data_at(:organization, :groups, :edges, :node) } + let(:organization_fields) do + <<~FIELDS + id + path + groups { + edges { + node { + id + } + } + } + FIELDS + end let_it_be(:organization_user) { create(:organization_user) } let_it_be(:organization) { organization_user.organization } @@ -51,6 +63,19 @@ end end + context 'when resolve_organization_groups feature flag is disabled' do + before do + stub_feature_flags(resolve_organization_groups: false) + end + + it 'returns no groups' do + request_organization + + expect(graphql_data['organization']).not_to be_nil + expect(graphql_data['organization']['groups']['edges']).to be_empty + end + end + context 'with `search` argument' do let(:search) { 'oth' } let(:organization_fields) do @@ -93,15 +118,15 @@ let(:sort) { "#{field}_#{direction}".upcase } let(:organization_fields) do <<~FIELDS - id - path - groups(sort: #{sort}) { - edges { - node { - id + id + path + groups(sort: #{sort}) { + edges { + node { + id + } } } - } FIELDS end -- GitLab From fd05d47aa78d7e1c0fb8a84e8a26805ffaab75ba Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Fri, 11 Aug 2023 15:48:26 +0530 Subject: [PATCH 14/19] Update Types::QueryType spec --- ee/spec/graphql/types/query_type_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/ee/spec/graphql/types/query_type_spec.rb b/ee/spec/graphql/types/query_type_spec.rb index 0ac790cdc8be63..623a4d3289d7b0 100644 --- a/ee/spec/graphql/types/query_type_spec.rb +++ b/ee/spec/graphql/types/query_type_spec.rb @@ -19,6 +19,7 @@ :instance_security_dashboard, :iteration, :license_history_entries, + :organization, :subscription_future_entries, :vulnerabilities, :vulnerabilities_count_by_day, -- GitLab From 82075f445093426dcd2b2b77141a57e94a6789fa Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 14 Aug 2023 13:59:09 +0530 Subject: [PATCH 15/19] Correct example description --- spec/graphql/types/query_type_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index 4471b9e1dda952..8bda738751d66d 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -26,7 +26,7 @@ describe 'organization field' do subject { described_class.fields['organization'] } - it 'finds projects by full path' do + it 'finds organization by path' do is_expected.to have_graphql_arguments(:id) is_expected.to have_graphql_type(Types::Organizations::OrganizationType) is_expected.to have_graphql_resolver(Resolvers::Organizations::OrganizationResolver) -- GitLab From c2c502a1094d10be2706631b7e59b04b66292216 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 14 Aug 2023 14:52:39 +0530 Subject: [PATCH 16/19] Fix organization request spec As all organizations are currently public. --- .../api/graphql/organizations/organization_query_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/graphql/organizations/organization_query_spec.rb b/spec/requests/api/graphql/organizations/organization_query_spec.rb index 1a933eea45b5d7..2d09a53527934d 100644 --- a/spec/requests/api/graphql/organizations/organization_query_spec.rb +++ b/spec/requests/api/graphql/organizations/organization_query_spec.rb @@ -49,10 +49,10 @@ context 'when the user does not have access to the organization' do let(:current_user) { create(:user) } - it 'returns an empty field' do + it 'returns the organization as all organizations are public' do request_organization - expect(graphql_data['organization']).to be_nil + expect(graphql_data['organization']['id']).to eq(organization.to_global_id.to_s) end end -- GitLab From 538f59fa20813f368955e49edce2d3d6806935d3 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Wed, 16 Aug 2023 21:50:27 +0530 Subject: [PATCH 17/19] Update documentation with review suggestions --- app/graphql/types/organizations/group_sort_enum.rb | 2 +- app/graphql/types/organizations/organization_type.rb | 2 +- doc/api/graphql/reference/index.md | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/graphql/types/organizations/group_sort_enum.rb b/app/graphql/types/organizations/group_sort_enum.rb index 236c929b5705de..57018ddd6ea2ac 100644 --- a/app/graphql/types/organizations/group_sort_enum.rb +++ b/app/graphql/types/organizations/group_sort_enum.rb @@ -6,7 +6,7 @@ class GroupSortEnum < BaseEnum graphql_name 'OrganizationGroupSort' description 'Values for sorting organization groups' - sortable_fields = ['Id', 'Name', 'Path', 'Updated at', 'Created at'] + sortable_fields = ['ID', 'Name', 'Path', 'Updated at', 'Created at'] sortable_fields.each do |field| value "#{field.upcase.tr(' ', '_')}_ASC", diff --git a/app/graphql/types/organizations/organization_type.rb b/app/graphql/types/organizations/organization_type.rb index 938ab310b2da51..8ba7957f58a11f 100644 --- a/app/graphql/types/organizations/organization_type.rb +++ b/app/graphql/types/organizations/organization_type.rb @@ -10,7 +10,7 @@ class OrganizationType < BaseObject field :groups, Types::GroupType.connection_type, null: false, - description: 'Groups within this organization where the user has access.', + description: 'Groups within this organization that the user has access to.', alpha: { milestone: '16.3' }, resolver: ::Resolvers::Organizations::GroupsResolver field :id, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index c87e1857e14569..afa7a87ec1d965 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -20251,7 +20251,7 @@ Active period time range for on-call rotation. ##### `Organization.groups` -Groups within this organization where the user has access. +Groups within this organization that the user has access to. WARNING: **Introduced** in 16.3. @@ -27272,8 +27272,8 @@ Values for sorting organization groups. | ----- | ----------- | | `CREATED_AT_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Created at in ascending order. | | `CREATED_AT_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Created at in descending order. | -| `ID_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Id in ascending order. | -| `ID_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Id in descending order. | +| `ID_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. ID in ascending order. | +| `ID_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. ID in descending order. | | `NAME_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Name in ascending order. | | `NAME_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Name in descending order. | | `PATH_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Path in ascending order. | -- GitLab From 7b1504527d1b6e4b8812ea7ffec22a57743b3704 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Thu, 17 Aug 2023 16:26:02 +0530 Subject: [PATCH 18/19] Fix comment with correct type --- app/finders/organizations/groups_finder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/finders/organizations/groups_finder.rb b/app/finders/organizations/groups_finder.rb index cddccb5eebe108..2b59a3106a3a62 100644 --- a/app/finders/organizations/groups_finder.rb +++ b/app/finders/organizations/groups_finder.rb @@ -7,7 +7,7 @@ module Organizations class GroupsFinder # @param organization [Organizations::Organization] # @param current_user [User] - # @param params [{ sort: [String], search: [String] }] + # @param params [{ sort: { field: [String], direction: [String] }, search: [String] }] def initialize(organization:, current_user:, params: {}) @organization = organization @current_user = current_user -- GitLab From f100b2f29d231a41f22f3876618d30f847ae5a7e Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Fri, 18 Aug 2023 12:33:35 +0530 Subject: [PATCH 19/19] Update milestone number Also update the GraphQL docs. --- .../organizations/groups_resolver.rb | 4 +-- .../types/organizations/group_sort_enum.rb | 4 +-- .../types/organizations/organization_type.rb | 8 ++--- app/graphql/types/query_type.rb | 2 +- doc/api/graphql/reference/index.md | 34 +++++++++---------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/app/graphql/resolvers/organizations/groups_resolver.rb b/app/graphql/resolvers/organizations/groups_resolver.rb index 7516f12d2ecef0..0f50713b9b439a 100644 --- a/app/graphql/resolvers/organizations/groups_resolver.rb +++ b/app/graphql/resolvers/organizations/groups_resolver.rb @@ -14,14 +14,14 @@ class GroupsResolver < BaseResolver GraphQL::Types::String, required: false, description: 'Search query for group name or full path.', - alpha: { milestone: '16.3' } + alpha: { milestone: '16.4' } argument :sort, Types::Organizations::GroupSortEnum, description: 'Criteria to sort organization groups by.', required: false, default_value: { field: 'name', direction: :asc }, - alpha: { milestone: '16.3' } + alpha: { milestone: '16.4' } private diff --git a/app/graphql/types/organizations/group_sort_enum.rb b/app/graphql/types/organizations/group_sort_enum.rb index 57018ddd6ea2ac..8fb2f553539a2d 100644 --- a/app/graphql/types/organizations/group_sort_enum.rb +++ b/app/graphql/types/organizations/group_sort_enum.rb @@ -12,12 +12,12 @@ class GroupSortEnum < BaseEnum value "#{field.upcase.tr(' ', '_')}_ASC", value: { field: field.downcase.tr(' ', '_'), direction: :asc }, description: "#{field} in ascending order.", - alpha: { milestone: '16.3' } + alpha: { milestone: '16.4' } value "#{field.upcase.tr(' ', '_')}_DESC", value: { field: field.downcase.tr(' ', '_'), direction: :desc }, description: "#{field} in descending order.", - alpha: { milestone: '16.3' } + alpha: { milestone: '16.4' } end end end diff --git a/app/graphql/types/organizations/organization_type.rb b/app/graphql/types/organizations/organization_type.rb index 8ba7957f58a11f..791fddc52667df 100644 --- a/app/graphql/types/organizations/organization_type.rb +++ b/app/graphql/types/organizations/organization_type.rb @@ -11,23 +11,23 @@ class OrganizationType < BaseObject Types::GroupType.connection_type, null: false, description: 'Groups within this organization that the user has access to.', - alpha: { milestone: '16.3' }, + alpha: { milestone: '16.4' }, resolver: ::Resolvers::Organizations::GroupsResolver field :id, GraphQL::Types::ID, null: false, description: 'ID of the organization.', - alpha: { milestone: '16.3' } + alpha: { milestone: '16.4' } field :name, GraphQL::Types::String, null: false, description: 'Name of the organization.', - alpha: { milestone: '16.3' } + alpha: { milestone: '16.4' } field :path, GraphQL::Types::String, null: false, description: 'Path of the organization.', - alpha: { milestone: '16.3' } + alpha: { milestone: '16.4' } end end end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index c29b1b8fb75f16..e3dd02110291d7 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -101,7 +101,7 @@ class QueryType < ::Types::BaseObject null: true, resolver: Resolvers::Organizations::OrganizationResolver, description: "Find an organization.", - alpha: { milestone: '16.3' } + alpha: { milestone: '16.4' } field :package, description: 'Find a package. This field can only be resolved for one query in any single request. Returns `null` if a package has no `default` status.', resolver: Resolvers::PackageDetailsResolver diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index afa7a87ec1d965..ff5cdee4009d42 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -537,7 +537,7 @@ Returns [`Note`](#note). Find an organization. WARNING: -**Introduced** in 16.3. +**Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Returns [`Organization`](#organization). @@ -20243,9 +20243,9 @@ Active period time range for on-call rotation. | Name | Type | Description | | ---- | ---- | ----------- | -| `id` **{warning-solid}** | [`ID!`](#id) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. ID of the organization. | -| `name` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Name of the organization. | -| `path` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Path of the organization. | +| `id` **{warning-solid}** | [`ID!`](#id) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. ID of the organization. | +| `name` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Name of the organization. | +| `path` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Path of the organization. | #### Fields with arguments @@ -20254,7 +20254,7 @@ Active period time range for on-call rotation. Groups within this organization that the user has access to. WARNING: -**Introduced** in 16.3. +**Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Returns [`GroupConnection!`](#groupconnection). @@ -20267,8 +20267,8 @@ four standard [pagination arguments](#connection-pagination-arguments): | Name | Type | Description | | ---- | ---- | ----------- | -| `search` **{warning-solid}** | [`String`](#string) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Search query for group name or full path. | -| `sort` **{warning-solid}** | [`OrganizationGroupSort`](#organizationgroupsort) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Criteria to sort organization groups by. | +| `search` **{warning-solid}** | [`String`](#string) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Search query for group name or full path. | +| `sort` **{warning-solid}** | [`OrganizationGroupSort`](#organizationgroupsort) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Criteria to sort organization groups by. | ### `OrganizationStateCounts` @@ -27270,16 +27270,16 @@ Values for sorting organization groups. | Value | Description | | ----- | ----------- | -| `CREATED_AT_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Created at in ascending order. | -| `CREATED_AT_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Created at in descending order. | -| `ID_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. ID in ascending order. | -| `ID_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. ID in descending order. | -| `NAME_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Name in ascending order. | -| `NAME_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Name in descending order. | -| `PATH_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Path in ascending order. | -| `PATH_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Path in descending order. | -| `UPDATED_AT_ASC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Updated at in ascending order. | -| `UPDATED_AT_DESC` **{warning-solid}** | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Updated at in descending order. | +| `CREATED_AT_ASC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Created at in ascending order. | +| `CREATED_AT_DESC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Created at in descending order. | +| `ID_ASC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. ID in ascending order. | +| `ID_DESC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. ID in descending order. | +| `NAME_ASC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Name in ascending order. | +| `NAME_DESC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Name in descending order. | +| `PATH_ASC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Path in ascending order. | +| `PATH_DESC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Path in descending order. | +| `UPDATED_AT_ASC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Updated at in ascending order. | +| `UPDATED_AT_DESC` **{warning-solid}** | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Updated at in descending order. | ### `OrganizationSort` -- GitLab