From fa67b5490dd97784f7377051cea6c32183ce64da Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 8 Mar 2024 12:06:56 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../components/achievements_app.vue | 33 ++- .../components/achievements_form.vue | 214 ++++++++++++++++++ .../achievement_fields.fragment.graphql | 5 + .../create_achievement.mutation.graphql | 10 + .../get_group_achievements.query.graphql | 5 +- app/assets/javascripts/achievements/routes.js | 2 + app/assets/javascripts/members/constants.js | 7 +- .../pages/groups/achievements/index.js | 2 + .../reviewers/uncollapsed_reviewer_list.vue | 9 +- .../concerns/membership_actions.rb | 16 +- .../groups/achievements_controller.rb | 4 + .../groups/group_members_controller.rb | 1 + .../projects/project_members_controller.rb | 4 + app/models/member.rb | 3 + ...ed_private_group_accessibility_assigner.rb | 68 ++++++ .../members/members/members_with_parents.rb | 2 +- ...er_max_access_level_in_groups_preloader.rb | 2 +- app/serializers/member_entity.rb | 9 +- app/serializers/member_serializer.rb | 2 + .../gitlab_google_cloud_integration/index.md | 33 +-- doc/user/group/manage.md | 2 +- .../dependency_proxy/index.md | 6 +- .../members/share_project_with_groups.md | 2 +- locale/gitlab.pot | 33 +++ .../groups/group_members_controller_spec.rb | 14 +- .../project_members_controller_spec.rb | 24 +- .../achievements/achievements_spec.rb | 71 ++++++ .../groups/members/filter_members_spec.rb | 21 +- ...ctive_access_level_per_user_finder_spec.rb | 8 +- .../achievements/achievements_app_spec.js | 26 ++- .../achievements/achievements_form_spec.js | 120 ++++++++++ spec/frontend/fixtures/achievements.rb | 73 ++++-- .../uncollapsed_reviewer_list_spec.js | 8 +- .../projects/project_members_helper_spec.rb | 3 + ...ivate_group_accessibility_assigner_spec.rb | 180 +++++++++++++++ .../groups/achievements_controller_spec.rb | 6 + spec/scripts/internal_events/cli_spec.rb | 12 +- spec/serializers/member_entity_spec.rb | 48 +++- spec/serializers/member_serializer_spec.rb | 95 +++++++- .../move_quick_action_shared_examples.rb | 4 + 40 files changed, 1090 insertions(+), 97 deletions(-) create mode 100644 app/assets/javascripts/achievements/components/achievements_form.vue create mode 100644 app/assets/javascripts/achievements/components/graphql/achievement_fields.fragment.graphql create mode 100644 app/assets/javascripts/achievements/components/graphql/create_achievement.mutation.graphql create mode 100644 app/models/members/members/invited_private_group_accessibility_assigner.rb create mode 100644 spec/features/achievements/achievements_spec.rb create mode 100644 spec/frontend/achievements/achievements_form_spec.js create mode 100644 spec/models/members/members/invited_private_group_accessibility_assigner_spec.rb diff --git a/app/assets/javascripts/achievements/components/achievements_app.vue b/app/assets/javascripts/achievements/components/achievements_app.vue index 1f64c5060d3..1ad14886971 100644 --- a/app/assets/javascripts/achievements/components/achievements_app.vue +++ b/app/assets/javascripts/achievements/components/achievements_app.vue @@ -1,19 +1,25 @@ diff --git a/app/assets/javascripts/achievements/components/achievements_form.vue b/app/assets/javascripts/achievements/components/achievements_form.vue new file mode 100644 index 00000000000..ad9f6d9ff89 --- /dev/null +++ b/app/assets/javascripts/achievements/components/achievements_form.vue @@ -0,0 +1,214 @@ + + + diff --git a/app/assets/javascripts/achievements/components/graphql/achievement_fields.fragment.graphql b/app/assets/javascripts/achievements/components/graphql/achievement_fields.fragment.graphql new file mode 100644 index 00000000000..c7954c2d1e6 --- /dev/null +++ b/app/assets/javascripts/achievements/components/graphql/achievement_fields.fragment.graphql @@ -0,0 +1,5 @@ +fragment AchievementFragment on Achievement { + id + name + description +} diff --git a/app/assets/javascripts/achievements/components/graphql/create_achievement.mutation.graphql b/app/assets/javascripts/achievements/components/graphql/create_achievement.mutation.graphql new file mode 100644 index 00000000000..8170e46cb68 --- /dev/null +++ b/app/assets/javascripts/achievements/components/graphql/create_achievement.mutation.graphql @@ -0,0 +1,10 @@ +#import "./achievement_fields.fragment.graphql" + +mutation createAchievement($input: AchievementsCreateInput!) { + achievementsCreate(input: $input) { + achievement { + ...AchievementFragment + } + errors + } +} diff --git a/app/assets/javascripts/achievements/components/graphql/get_group_achievements.query.graphql b/app/assets/javascripts/achievements/components/graphql/get_group_achievements.query.graphql index 8d1bf0ec677..996c2f9af75 100644 --- a/app/assets/javascripts/achievements/components/graphql/get_group_achievements.query.graphql +++ b/app/assets/javascripts/achievements/components/graphql/get_group_achievements.query.graphql @@ -1,4 +1,5 @@ #import "~/graphql_shared/fragments/page_info.fragment.graphql" +#import "./achievement_fields.fragment.graphql" query getGroupAchievements( $groupFullPath: ID! @@ -11,9 +12,7 @@ query getGroupAchievements( id achievements(first: $first, last: $last, after: $after, before: $before) { nodes { - id - name - description + ...AchievementFragment } pageInfo { ...PageInfo diff --git a/app/assets/javascripts/achievements/routes.js b/app/assets/javascripts/achievements/routes.js index 12aa17d73b6..12739fd33d0 100644 --- a/app/assets/javascripts/achievements/routes.js +++ b/app/assets/javascripts/achievements/routes.js @@ -1,4 +1,5 @@ import { INDEX_ROUTE_NAME, NEW_ROUTE_NAME, EDIT_ROUTE_NAME } from './constants'; +import AchievementsForm from './components/achievements_form.vue'; export default [ { @@ -8,6 +9,7 @@ export default [ { name: NEW_ROUTE_NAME, path: '/new', + component: AchievementsForm, }, { name: EDIT_ROUTE_NAME, diff --git a/app/assets/javascripts/members/constants.js b/app/assets/javascripts/members/constants.js index e1f7e81d831..1c32542e299 100644 --- a/app/assets/javascripts/members/constants.js +++ b/app/assets/javascripts/members/constants.js @@ -141,7 +141,12 @@ export const FILTERED_SEARCH_TOKEN_WITH_INHERITED_PERMISSIONS = { operators: OPERATORS_IS, options: [ { value: 'exclude', title: s__('Members|Direct') }, - { value: 'only', title: s__('Members|Inherited') }, + { + value: 'only', + title: gon.features?.webuiMembersInheritedUsers + ? s__('Members|Indirect') + : s__('Members|Inherited'), + }, ], }; diff --git a/app/assets/javascripts/pages/groups/achievements/index.js b/app/assets/javascripts/pages/groups/achievements/index.js index 9da5f6b6435..b9e6a48e9fe 100644 --- a/app/assets/javascripts/pages/groups/achievements/index.js +++ b/app/assets/javascripts/pages/groups/achievements/index.js @@ -1,3 +1,4 @@ +import { GlToast } from '@gitlab/ui'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; import VueRouter from 'vue-router'; @@ -8,6 +9,7 @@ import routes from '~/achievements/routes'; Vue.use(VueApollo); Vue.use(VueRouter); +Vue.use(GlToast); const init = () => { const el = document.getElementById('js-achievements-app'); diff --git a/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue b/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue index d2b5fa92a1a..34996335004 100644 --- a/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue +++ b/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue @@ -10,19 +10,18 @@ const JUST_APPROVED = 'approved'; const REVIEW_STATE_ICONS = { APPROVED: { - name: 'status-success', + name: 'check-circle', class: 'gl-text-green-500', title: __('Reviewer approved changes'), }, REQUESTED_CHANGES: { - name: 'status-alert', + name: 'error', class: 'gl-text-red-500', title: __('Reviewer requested changes'), }, REVIEWED: { - name: 'comment', - class: 'gl-bg-blue-500 gl-text-white gl-icon s16 gl-rounded-full gl--flex-center', - size: 8, + name: 'comment-lines', + class: 'gl-text-blue-500', title: __('Reviewer commented'), }, UNREVIEWED: { diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 3053efa4191..231adfefc19 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -150,14 +150,9 @@ module MembershipActions when 'exclude' [:direct] when 'only' - [:inherited] + [:inherited].concat(shared_members_relations) else - if Feature.enabled?(:webui_members_inherited_users, current_user) - project_relations = [:invited_groups, :shared_into_ancestors] - [:inherited, :direct, :shared_from_groups, *(project_relations if params[:project_id])] - else - [:inherited, :direct] - end + [:inherited, :direct].concat(shared_members_relations) end end @@ -188,6 +183,13 @@ module MembershipActions {} end end + + def shared_members_relations + return [] unless Feature.enabled?(:webui_members_inherited_users, current_user) + + project_relations = [:invited_groups, :shared_into_ancestors] + [:shared_from_groups, *(project_relations if params[:project_id])] + end end MembershipActions.prepend_mod_with('MembershipActions') diff --git a/app/controllers/groups/achievements_controller.rb b/app/controllers/groups/achievements_controller.rb index 52d63761819..7ff6871bf3d 100644 --- a/app/controllers/groups/achievements_controller.rb +++ b/app/controllers/groups/achievements_controller.rb @@ -7,6 +7,10 @@ module Groups before_action :authorize_read_achievement! + def new + render action: "index" + end + private def authorize_read_achievement! diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index de47c5fb5e3..fa6d293ee94 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -18,6 +18,7 @@ class Groups::GroupMembersController < Groups::ApplicationController before_action only: [:index] do push_frontend_feature_flag(:service_accounts_crud, @group) + push_frontend_feature_flag(:webui_members_inherited_users, current_user) end skip_before_action :check_two_factor_requirement, only: :leave diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 5390df449e8..223e8ade22e 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -8,6 +8,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController # Authorize before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access] + before_action only: [:index] do + push_frontend_feature_flag(:webui_members_inherited_users, current_user) + end + feature_category :groups_and_projects urgency :low diff --git a/app/models/member.rb b/app/models/member.rb index 1aef1fe21b6..751ba4ea97f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -293,6 +293,9 @@ class Member < ApplicationRecord end attribute :notification_level, default: -> { NotificationSetting.levels[:global] } + # Only false when the current user is a member of the shared group or project but not of the invited private group + # so the current user can't see the source of the membership. + attribute :is_source_accessible_to_current_user, default: true class << self def search(query) diff --git a/app/models/members/members/invited_private_group_accessibility_assigner.rb b/app/models/members/members/invited_private_group_accessibility_assigner.rb new file mode 100644 index 00000000000..cd92ed79772 --- /dev/null +++ b/app/models/members/members/invited_private_group_accessibility_assigner.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module Members + # We allow the current user to see the invited private group when the current user is a member of the shared group to + # allow better collaboration between the two groups even though the current user is not a member of the invited group. + # We don't allow the current user to see the source of membership i.e. the group name, path, and other group info as + # it's sensitive information if the current user is not an owner of the group or at least maintainer of the project. + # This class deals with setting `is_source_accessible_to_current_user` which is used to hide or show the source of + # memberships as per the above cases. + class InvitedPrivateGroupAccessibilityAssigner + include Gitlab::Allowable + include Gitlab::Utils::StrongMemoize + + def initialize(members, source:, current_user:) + if !members.is_a?(Array) && !members.is_a?(MembersPresenter) + raise ArgumentError, "members should be an instance of Array or MembersPresenter" + end + + @members = members + @source = source + @current_user = current_user + end + + def execute + # We don't need to calculate the access level of the current user in the invited groups if: + # + # 1. The current user can admin members then the user should be able to see the source of all memberships + # to enable management of group/project memberships. + # 2. There are no members invited from a private group. + return if can_admin_members? || private_invited_group_members.nil? + + private_invited_group_members.each do |member| + member.is_source_accessible_to_current_user = authorized_group_ids.include?(member.source_id) + end + end + + private + + attr_reader :members, :source, :current_user + + def authorized_group_ids + return [] if current_user.nil? + + private_invited_group_ids = private_invited_group_members.map(&:source_id).uniq + current_user.authorized_groups.id_in(private_invited_group_ids).map(&:id) + end + strong_memoize_attr(:authorized_group_ids) + + def private_invited_group_members + members.select do |member| + # The user can see those members where: + # - The source is public. + # - The member is direct or inherited. ProjectMember type is always direct. + member.is_a?(GroupMember) && + member.source.visibility_level != Gitlab::VisibilityLevel::PUBLIC && + member.source_id != source.id && # Exclude direct member + member.source.traversal_ids.exclude?(source.id) # Exclude inherited member + end + end + strong_memoize_attr(:private_invited_group_members) + + def can_admin_members? + return can?(current_user, :admin_project_member, source) if source.is_a?(Project) + + can?(current_user, :admin_group_member, source) + end + end +end diff --git a/app/models/members/members/members_with_parents.rb b/app/models/members/members/members_with_parents.rb index 61ce99e1f3e..3598a146e8d 100644 --- a/app/models/members/members/members_with_parents.rb +++ b/app/models/members/members/members_with_parents.rb @@ -75,7 +75,7 @@ module Members # Instead of members.access_level, we need to maximize that access_level at # the respective group_group_links.group_access. - member_columns = GroupMember.attribute_names.map do |column_name| + member_columns = GroupMember.column_names.map do |column_name| if column_name == 'access_level' smallest_value_arel([cte_alias[:group_access], group_member_table[:access_level]], 'access_level') else diff --git a/app/models/preloaders/user_max_access_level_in_groups_preloader.rb b/app/models/preloaders/user_max_access_level_in_groups_preloader.rb index aaa54e0228b..4615b22cfba 100644 --- a/app/models/preloaders/user_max_access_level_in_groups_preloader.rb +++ b/app/models/preloaders/user_max_access_level_in_groups_preloader.rb @@ -48,7 +48,7 @@ module Preloaders group_group_link_table = GroupGroupLink.arel_table group_member_table = GroupMember.arel_table - altered_columns = GroupMember.attribute_names.map do |column_name| + altered_columns = GroupMember.column_names.map do |column_name| case column_name when 'access_level' # Consider the limiting effect of group share's access level diff --git a/app/serializers/member_entity.rb b/app/serializers/member_entity.rb index f259cba65a6..450937d9cb6 100644 --- a/app/serializers/member_entity.rb +++ b/app/serializers/member_entity.rb @@ -11,7 +11,8 @@ class MemberEntity < Grape::Entity end expose :requested_at - expose :created_by, if: -> (member) { member.created_by.present? } do |member| + expose :created_by, + if: -> (member) { member.created_by.present? && member.is_source_accessible_to_current_user } do |member| UserEntity.represent(member.created_by, only: [:name, :web_url]) end @@ -38,10 +39,14 @@ class MemberEntity < Grape::Entity expose :custom_permissions - expose :source do |member| + expose :source, if: -> (member) { member.is_source_accessible_to_current_user } do |member| GroupEntity.represent(member.source, only: [:id, :full_name, :web_url]) end + expose :is_shared_with_group_private do |member| + !member.is_source_accessible_to_current_user + end + expose :type expose :valid_level_roles, as: :valid_roles diff --git a/app/serializers/member_serializer.rb b/app/serializers/member_serializer.rb index ad258b0ef1e..aa79a6ebb80 100644 --- a/app/serializers/member_serializer.rb +++ b/app/serializers/member_serializer.rb @@ -5,6 +5,8 @@ class MemberSerializer < BaseSerializer def represent(members, opts = {}) LastGroupOwnerAssigner.new(opts[:group], members).execute unless opts[:source].is_a?(Project) + Members::InvitedPrivateGroupAccessibilityAssigner + .new(members, source: opts[:source], current_user: opts[:current_user]).execute super(members, opts) end diff --git a/doc/ci/gitlab_google_cloud_integration/index.md b/doc/ci/gitlab_google_cloud_integration/index.md index 9d220c4e648..e0541b44461 100644 --- a/doc/ci/gitlab_google_cloud_integration/index.md +++ b/doc/ci/gitlab_google_cloud_integration/index.md @@ -1,20 +1,27 @@ --- -stage: none -group: unassigned +stage: Package +group: Container Registry info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://handbook.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# GitLab and Google Cloud +# GitLab and Google Cloud integration -DISCLAIMER: -This page contains information related to upcoming products, features, and functionality. -It is important to note that the information presented is for informational purposes only. -Please do not rely on this information for purchasing or planning purposes. -As with all projects, the items mentioned on this page are subject to change or delay. -The development, release, and timing of any products, features, or functionality remain at the -sole discretion of GitLab Inc. +DETAILS: +**Tier:** Premium, Ultimate +**Offering:** GitLab.com +**Status:** Beta -The GitLab and Google Cloud integration combines GitLab source code management, planning, CI/CD workflow, advanced security, and compliance capabilities -with the unified data plane in Google’s Cloud console and Artifact Registry. +FLAG: +On GitLab.com, this feature is available for a subset of users. +On GitLab Dedicated, this feature is not available. -For more information, see [Better together with GitLab and Google Cloud](https://about.gitlab.com/blog/2023/08/29/gitlab-google-partnership-s3c/). +This feature is in [Beta](../../policy/experiment-beta-support.md). +To test this feature, join the [waitlist](https://forms.gle/XdxdTxC7DXj4NSaz9). + +Use the Google Cloud integration to use Google Cloud resources in GitLab projects. + +To get started, see [Set up the Google Cloud integration](../../tutorials/set_up_gitlab_google_integration/index.md). + +- [Google Cloud workload identity federation and IAM](../../integration/google_cloud_iam.md) +- [Google Artifact Registry](../../user/project/integrations/google_artifact_registry.md) +- [Provisioning runners to Google Cloud](../runners/provision_runners_google_cloud.md) diff --git a/doc/user/group/manage.md b/doc/user/group/manage.md index 98f633a74a1..3e5cb780cba 100644 --- a/doc/user/group/manage.md +++ b/doc/user/group/manage.md @@ -119,7 +119,7 @@ For more information about sharing conditions and behavior, see [Sharing project Prerequisites: -- You must be a member of the inviting group. +- You must be a member of the invited and inviting groups. To invite a group to your group: diff --git a/doc/user/packages/package_registry/dependency_proxy/index.md b/doc/user/packages/package_registry/dependency_proxy/index.md index 7e172c37fc0..060ff3461e9 100644 --- a/doc/user/packages/package_registry/dependency_proxy/index.md +++ b/doc/user/packages/package_registry/dependency_proxy/index.md @@ -108,7 +108,7 @@ For Maven packages, [all clients supported by the package registry](../../maven_ - `gradle` - `sbt` -For authentication, the Maven dependency proxy access all authentication methods accepted by the [Maven package registry](../../maven_repository/index.md#edit-the-client-configuration). +For authentication, you can use all methods accepted by the [Maven package registry](../../maven_repository/index.md#edit-the-client-configuration). You should use the [Basic HTTP authentication](../../maven_repository/index.md#basic-http-authentication) method as it is less complex. To configure the client: @@ -123,6 +123,10 @@ To configure the client: :::TabTitle mvn +[Basic HTTP authentication](../../maven_repository/index.md#basic-http-authentication) is accepted. +However, you should use the [custom HTTP header authentication](../../maven_repository/index.md#custom-http-header), +so that `mvn` uses fewer network requests. + In the `pom.xml` file add a `repository` element: ```xml diff --git a/doc/user/project/members/share_project_with_groups.md b/doc/user/project/members/share_project_with_groups.md index 9e13b376d36..2a774dfb868 100644 --- a/doc/user/project/members/share_project_with_groups.md +++ b/doc/user/project/members/share_project_with_groups.md @@ -23,7 +23,7 @@ Prerequisites: - Explicitly defined as a [member](index.md) of the project. - Explicitly defined as a member of a group or subgroup that has access to the project. - An administrator. -- You must be a member of the inviting group or subgroup. +- You must be a member of the invited group or subgroup. To invite a group to a project: diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9b46c8cdd81..48712d18c15 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2700,6 +2700,18 @@ msgstr "" msgid "Achievements|%{namespace_link} awarded you the %{bold_start}%{achievement_name}%{bold_end} achievement!" msgstr "" +msgid "Achievements|Achievement description cannot be longer than %{length} characters." +msgstr "" + +msgid "Achievements|Achievement has been added." +msgstr "" + +msgid "Achievements|Achievement name cannot be longer than %{length} characters." +msgstr "" + +msgid "Achievements|Achievement name is required." +msgstr "" + msgid "Achievements|Achievements" msgstr "" @@ -2709,6 +2721,15 @@ msgstr "" msgid "Achievements|Awarded %{timeAgo} by a private namespace" msgstr "" +msgid "Achievements|Description" +msgstr "" + +msgid "Achievements|Name" +msgstr "" + +msgid "Achievements|New achievement" +msgstr "" + msgid "Achievements|There are currently no achievements." msgstr "" @@ -4416,6 +4437,9 @@ msgstr "" msgid "Admin|Your instance has reached its user cap" msgstr "" +msgid "Adopted" +msgstr "" + msgid "Advanced" msgstr "" @@ -31032,6 +31056,9 @@ msgstr "" msgid "Members|Filter members" msgstr "" +msgid "Members|Indirect" +msgstr "" + msgid "Members|Inherited" msgstr "" @@ -33496,6 +33523,9 @@ msgstr "" msgid "Normal view" msgstr "" +msgid "Not adopted" +msgstr "" + msgid "Not all browsers support WebAuthn. Therefore, we require that you set up a two-factor authentication app first. That way you'll always be able to sign in, even from an unsupported browser." msgstr "" @@ -59659,6 +59689,9 @@ msgstr "" msgid "failed to revoke token" msgstr "" +msgid "features adopted" +msgstr "" + msgid "file" msgid_plural "files" msgstr[0] "" diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index f806563928e..60b7d8e5eff 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Groups::GroupMembersController do +RSpec.describe Groups::GroupMembersController, feature_category: :groups_and_projects do include ExternalAuthorizationServiceHelpers let_it_be(:user) { create(:user) } @@ -101,17 +101,21 @@ RSpec.describe Groups::GroupMembersController do context 'when user has owner access to subgroup' do let_it_be(:nested_group) { create(:group, parent: group) } let_it_be(:nested_group_user) { create(:user) } + let_it_be(:shared_group) { create(:group, parent: group) } + let_it_be(:shared_group_user) { create(:user) } before do group.add_owner(user) nested_group.add_owner(nested_group_user) + shared_group.add_owner(shared_group_user) + create(:group_group_link, shared_group: nested_group, shared_with_group: shared_group) sign_in(user) end - it 'lists inherited group members by default' do + it 'lists all group members including members from shared group by default' do get :index, params: { group_id: nested_group } - expect(assigns(:members).map(&:user_id)).to contain_exactly(user.id, nested_group_user.id) + expect(assigns(:members).map(&:user_id)).to contain_exactly(user.id, nested_group_user.id, shared_group_user.id) end it 'lists direct group members only' do @@ -120,10 +124,10 @@ RSpec.describe Groups::GroupMembersController do expect(assigns(:members).map(&:user_id)).to contain_exactly(nested_group_user.id) end - it 'lists inherited group members only' do + it 'lists inherited and shared group members only' do get :index, params: { group_id: nested_group, with_inherited_permissions: 'only' } - expect(assigns(:members).map(&:user_id)).to contain_exactly(user.id) + expect(assigns(:members).map(&:user_id)).to contain_exactly(user.id, shared_group_user.id) end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index c20f92cd2f0..f0c59f4d53d 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -2,11 +2,13 @@ require('spec_helper') -RSpec.describe Projects::ProjectMembersController do +RSpec.describe Projects::ProjectMembersController, feature_category: :groups_and_projects do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group, :public) } let_it_be(:sub_group) { create(:group, parent: group) } let_it_be(:project, reload: true) { create(:project, :public) } + let_it_be(:shared_group) { create(:group, parent: group) } + let_it_be(:shared_group_user) { create(:user) } shared_examples_for 'controller actions' do before do @@ -32,13 +34,15 @@ RSpec.describe Projects::ProjectMembersController do before do group.add_owner(user_in_group) project_in_group.add_maintainer(user) + shared_group.add_owner(shared_group_user) + create(:group_group_link, shared_group: group, shared_with_group: shared_group) sign_in(user) end - it 'lists inherited project members by default' do + it 'lists all project members including members from shared group by default' do get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id) + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id, shared_group_user.id) end it 'lists direct project members only' do @@ -47,10 +51,10 @@ RSpec.describe Projects::ProjectMembersController do expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id) end - it 'lists inherited project members only' do + it 'lists inherited project members and shared group members only' do get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'only' } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id) + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id, shared_group_user.id) end end @@ -61,13 +65,15 @@ RSpec.describe Projects::ProjectMembersController do before do group.add_owner(user_in_group) project_in_group.add_maintainer(user) + shared_group.add_owner(shared_group_user) + create(:group_group_link, shared_group: sub_group, shared_with_group: shared_group) sign_in(user) end - it 'lists inherited project members by default' do + it 'lists all project members including members from shared group by default' do get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id) + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id, shared_group_user.id) end it 'lists direct project members only' do @@ -76,10 +82,10 @@ RSpec.describe Projects::ProjectMembersController do expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id) end - it 'lists inherited project members only' do + it 'lists inherited project members and shared group members only' do get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'only' } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id) + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id, shared_group_user.id) end end diff --git a/spec/features/achievements/achievements_spec.rb b/spec/features/achievements/achievements_spec.rb new file mode 100644 index 00000000000..1bcee531feb --- /dev/null +++ b/spec/features/achievements/achievements_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe "Achievements", :js, feature_category: :user_profile do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group, :public) } + let_it_be(:achievement1) { create(:achievement, namespace: group) } + let_it_be(:achievement2) { create(:achievement, namespace: group, description: 'Achievement description') } + + before_all do + group.add_maintainer(user) + end + + before do + sign_in(user) + end + + it 'displays groups achievements' do + visit(group_achievements_path(group)) + + expect(page).to have_content(achievement1.name) + .and have_content(achievement2.name) + .and have_content(achievement2.description) + end + + context 'when creating a new achievement' do + before do + visit(new_group_achievement_path(group)) + end + + it 'returns to the achievements list, displays the new achievement and toast' do + achievement_name = 'Superstar' + achievement_desc = 'A legend!' + fill_in('Name', with: achievement_name) + fill_in('Description', with: achievement_desc) + + click_button('Save changes') + + expect(page).to have_current_path("#{group_achievements_path(group)}/") + .and have_content(achievement_name) + .and have_content(achievement_desc) + .and have_content(achievement2.description) + .and have_content('Achievement has been added.') + end + + it 'validates required fields' do + click_button('Save changes') + + expect(page).to have_content('Achievement name is required.') + end + + it 'validates field lengths' do + fill_in('Name', with: 'x' * 256) + fill_in('Description', with: 'y' * 1025) + click_button('Save changes') + + expect(page).to have_content('Achievement name cannot be longer than 255 characters.') + .and have_content('Achievement description cannot be longer than 1024 characters.') + end + + context 'when closing the form' do + it 'returns to the achievements list and does not display toast' do + find('button[aria-label="Close drawer"]').click + + expect(page).to have_current_path("#{group_achievements_path(group)}/") + expect(page).not_to have_content('Achievement has been added.') + end + end + end +end diff --git a/spec/features/groups/members/filter_members_spec.rb b/spec/features/groups/members/filter_members_spec.rb index bf9efd06a03..6bf0f2254bb 100644 --- a/spec/features/groups/members/filter_members_spec.rb +++ b/spec/features/groups/members/filter_members_spec.rb @@ -69,14 +69,31 @@ RSpec.describe 'Groups > Members > Filter members', :js, feature_category: :grou end end - it 'shows only inherited members' do + it 'shows only indirect members' do visit_members_list(nested_group, with_inherited_permissions: 'only') expect(member(0)).to include(user.name) expect(member(1)).to include(user_with_2fa.name) expect(all_rows.size).to eq(2) within filtered_search_bar_selector do - expect(page).to have_content 'Membership = Inherited' + expect(page).to have_content 'Membership = Indirect' + end + end + + context 'when the `webui_members_inherited_users` feature flag is disabled' do + before do + stub_feature_flags(webui_members_inherited_users: false) + end + + it 'shows only inherited members' do + visit_members_list(nested_group, with_inherited_permissions: 'only') + expect(member(0)).to include(user.name) + expect(member(1)).to include(user_with_2fa.name) + expect(all_rows.size).to eq(2) + + within filtered_search_bar_selector do + expect(page).to have_content 'Membership = Inherited' + end end end diff --git a/spec/finders/projects/members/effective_access_level_per_user_finder_spec.rb b/spec/finders/projects/members/effective_access_level_per_user_finder_spec.rb index 3872938d20e..91452fe0d88 100644 --- a/spec/finders/projects/members/effective_access_level_per_user_finder_spec.rb +++ b/spec/finders/projects/members/effective_access_level_per_user_finder_spec.rb @@ -26,13 +26,13 @@ RSpec.describe Projects::Members::EffectiveAccessLevelPerUserFinder, '#execute' end it 'includes the highest access level from all avenues of memberships for the specific user alone' do - expect(subject).to eq( - [{ + expect(subject.first).to match(hash_including( + { 'user_id' => user.id, 'access_level' => Gitlab::Access::MAINTAINER, # From project_group_link 'id' => nil - }] - ) + } + )) end end end diff --git a/spec/frontend/achievements/achievements_app_spec.js b/spec/frontend/achievements/achievements_app_spec.js index 46fc085af45..8bc99a286de 100644 --- a/spec/frontend/achievements/achievements_app_spec.js +++ b/spec/frontend/achievements/achievements_app_spec.js @@ -19,17 +19,23 @@ describe('Achievements app', () => { const findEmptyState = () => wrapper.findComponent(GlEmptyState); const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const findNewAchievementButton = () => wrapper.findByTestId('new-achievement-button'); const findPagingControls = () => wrapper.findComponent(GlKeysetPagination); const findTable = () => wrapper.findComponent(GlTableLite); - const mountComponent = ({ queryResponse = getGroupAchievementsResponse } = {}) => { + const mountComponent = ({ + canAdminAchievement = true, + queryResponse = getGroupAchievementsResponse, + } = {}) => { queryHandler = jest.fn().mockResolvedValue(queryResponse); fakeApollo = createMockApollo([[getGroupAchievementsQuery, queryHandler]]); wrapper = shallowMountExtended(AchievementsApp, { provide: { + canAdminAchievement, groupFullPath: 'flightjs', }, apolloProvider: fakeApollo, + stubs: ['router-link', 'router-view'], }); return waitForPromises(); }; @@ -52,6 +58,24 @@ describe('Achievements app', () => { expect(items).toContainEqual(expect.objectContaining({ name: 'Legend' })); }); + describe('new achievement button', () => { + describe('when user can admin_achievement', () => { + it('should render', async () => { + await mountComponent(); + + expect(findNewAchievementButton().exists()).toBe(true); + }); + }); + + describe('when user can not admin_achievement', () => { + it('should not render', async () => { + await mountComponent({ canAdminAchievement: false }); + + expect(findNewAchievementButton().exists()).toBe(false); + }); + }); + }); + describe('with no achievements', () => { it('should render the empty state', async () => { await mountComponent({ queryResponse: getGroupAchievementsEmptyResponse }); diff --git a/spec/frontend/achievements/achievements_form_spec.js b/spec/frontend/achievements/achievements_form_spec.js new file mode 100644 index 00000000000..3ff22112bbb --- /dev/null +++ b/spec/frontend/achievements/achievements_form_spec.js @@ -0,0 +1,120 @@ +import { GlAlert, GlButton, GlFormFields } from '@gitlab/ui'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import VueRouter from 'vue-router'; +import createAchievementResponse from 'test_fixtures/graphql/create_achievement_response.json'; +import createAchievementErrorResponse from 'test_fixtures/graphql/create_achievement_error_response.json'; +import getGroupAchievementsResponse from 'test_fixtures/graphql/get_group_achievements_response.json'; + +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import AchievementsForm from '~/achievements/components/achievements_form.vue'; +import createAchievementMutation from '~/achievements/components/graphql/create_achievement.mutation.graphql'; +import getGroupAchievementsQuery from '~/achievements/components/graphql/get_group_achievements.query.graphql'; +import routes from '~/achievements/routes'; + +jest.mock('~/lib/logger'); + +describe('Achievements form', () => { + Vue.use(VueApollo); + Vue.use(VueRouter); + + const groupFullPath = 'flightjs'; + const mockToastShow = jest.fn(); + + let wrapper; + + const findError = () => wrapper.findComponent(GlAlert); + const findFormFields = () => wrapper.findComponent(GlFormFields); + const findSaveButton = () => wrapper.findComponent(GlButton); + + const successMutationHandler = jest.fn().mockResolvedValue(createAchievementResponse); + + const mountComponent = ({ mutationHandler = successMutationHandler } = {}) => { + const fakeApollo = createMockApollo([[createAchievementMutation, mutationHandler]]); + fakeApollo.clients.defaultClient.cache.writeQuery({ + query: getGroupAchievementsQuery, + variables: { groupFullPath }, + data: getGroupAchievementsResponse.data, + }); + + const router = new VueRouter({ + base: '', + mode: 'history', + routes, + }); + router.push('/new'); + + wrapper = shallowMountExtended(AchievementsForm, { + apolloProvider: fakeApollo, + mocks: { + $toast: { + show: mockToastShow, + }, + }, + propsData: { storeQuery: { query: getGroupAchievementsQuery, variables: { groupFullPath } } }, + provide: { + groupFullPath, + groupId: 7, + }, + router, + }); + }; + + it('renders form fields with fields prop containing name and description objects', () => { + mountComponent(); + + expect(findFormFields().props('fields')).toEqual( + expect.objectContaining({ + name: expect.any(Object), + description: expect.any(Object), + }), + ); + }); + + it('renders save button', () => { + mountComponent(); + + expect(findSaveButton().exists()).toBe(true); + }); + + describe('when mutation is successful', () => { + it('displays the correct toast message', async () => { + mountComponent(); + + findFormFields().vm.$emit('input', { name: 'Achievement' }); + findFormFields().vm.$emit('submit'); + await waitForPromises(); + + expect(mockToastShow).toHaveBeenCalledWith('Achievement has been added.'); + }); + }); + + describe('when mutation returns an error', () => { + it('displays the error message', async () => { + mountComponent({ + mutationHandler: jest.fn().mockResolvedValue(createAchievementErrorResponse), + }); + + findFormFields().vm.$emit('input', { name: 'Achievement' }); + findFormFields().vm.$emit('submit'); + await waitForPromises(); + + expect(mockToastShow).not.toHaveBeenCalled(); + expect(findError().text()).toBe('Name has already been taken'); + }); + }); + + describe('when mutation fails', () => { + it('displays the correct toast message', async () => { + mountComponent({ mutationHandler: jest.fn().mockRejectedValue('ERROR') }); + + findFormFields().vm.$emit('input', { name: 'Achievement' }); + findFormFields().vm.$emit('submit'); + await waitForPromises(); + + expect(findError().text()).toBe('Something went wrong. Please try again.'); + }); + }); +}); diff --git a/spec/frontend/fixtures/achievements.rb b/spec/frontend/fixtures/achievements.rb index 7fcdd8fa843..f352af89231 100644 --- a/spec/frontend/fixtures/achievements.rb +++ b/spec/frontend/fixtures/achievements.rb @@ -11,32 +11,67 @@ RSpec.describe 'Achievements (JavaScript fixtures)', feature_category: :user_pro let_it_be(:group) { create(:group, :public) } - let(:query_path) { 'achievements/components/graphql/get_group_achievements.query.graphql' } - let(:query) { get_graphql_query_as_string(query_path) } + describe 'get_group_achievements.query.graphql' do + let(:query_path) { 'achievements/components/graphql/get_group_achievements.query.graphql' } + let(:query) { get_graphql_query_as_string(query_path) } - it "graphql/get_group_achievements_empty_response.json" do - post_graphql(query, current_user: nil, variables: { group_full_path: group.full_path }) - - expect_graphql_errors_to_be_empty - end - - context 'with achievements' do - before_all do - create(:achievement, namespace: group, name: "Hero") - create(:achievement, namespace: group, name: "Star") - create(:achievement, namespace: group, name: "Legend") - end - - it "graphql/get_group_achievements_response.json" do + it 'graphql/get_group_achievements_empty_response.json' do post_graphql(query, current_user: nil, variables: { group_full_path: group.full_path }) expect_graphql_errors_to_be_empty end - it "graphql/get_group_achievements_paginated_response.json" do - post_graphql(query, current_user: nil, variables: { group_full_path: group.full_path, first: 2 }) + context 'with achievements' do + before_all do + create(:achievement, namespace: group, name: 'Hero') + create(:achievement, namespace: group, name: 'Star') + create(:achievement, namespace: group, name: 'Legend') + end - expect_graphql_errors_to_be_empty + it 'graphql/get_group_achievements_response.json' do + post_graphql(query, current_user: nil, variables: { group_full_path: group.full_path }) + + expect_graphql_errors_to_be_empty + end + + it 'graphql/get_group_achievements_paginated_response.json' do + post_graphql(query, current_user: nil, variables: { group_full_path: group.full_path, first: 2 }) + + expect_graphql_errors_to_be_empty + end + end + end + + describe 'create_achievement.mutation.graphql' do + let_it_be(:user) { create(:user) } + let_it_be(:achievement) { create(:achievement, namespace: group, name: 'Hero') } + + let(:mutation_path) { 'achievements/components/graphql/create_achievement.mutation.graphql' } + let(:mutation) { get_graphql_query_as_string(mutation_path) } + let(:variables) { { input: { namespace_id: "gid://gitlab/Group/#{group.id}", name: achievement_name } } } + + before_all do + group.add_maintainer(user) + end + + before do + post_graphql(mutation, current_user: user, variables: variables) + end + + context 'with an available name' do + let(:achievement_name) { 'New' } + + it 'graphql/create_achievement_response.json' do + expect_graphql_errors_to_be_empty + end + end + + context 'with an existing name' do + let(:achievement_name) { achievement.name } + + it 'graphql/create_achievement_error_response.json' do + expect(graphql_data_at('achievements_create', 'errors')).to include('Name has already been taken') + end end end end diff --git a/spec/frontend/sidebar/components/reviewers/uncollapsed_reviewer_list_spec.js b/spec/frontend/sidebar/components/reviewers/uncollapsed_reviewer_list_spec.js index af05f4bd247..ed6e1a3afe0 100644 --- a/spec/frontend/sidebar/components/reviewers/uncollapsed_reviewer_list_spec.js +++ b/spec/frontend/sidebar/components/reviewers/uncollapsed_reviewer_list_spec.js @@ -24,7 +24,7 @@ describe('UncollapsedReviewerList component', () => { let wrapper; const findAllRerequestButtons = () => wrapper.findAll('[data-testid="re-request-button"]'); - const findAllReviewerApprovalIcons = () => wrapper.findAll('[name="status-success"]'); + const findAllReviewerApprovalIcons = () => wrapper.findAll('[name="check-circle"]'); const findAllReviewerAvatarLinks = () => wrapper.findAllComponents(ReviewerAvatarLink); const hasApprovalIconAnimation = () => @@ -202,9 +202,9 @@ describe('UncollapsedReviewerList component', () => { it.each` reviewState | approved | icon ${'UNREVIEWED'} | ${false} | ${'dash-circle'} - ${'REVIEWED'} | ${true} | ${'status-success'} - ${'REVIEWED'} | ${false} | ${'comment'} - ${'REQUESTED_CHANGES'} | ${false} | ${'status-alert'} + ${'REVIEWED'} | ${true} | ${'check-circle'} + ${'REVIEWED'} | ${false} | ${'comment-lines'} + ${'REQUESTED_CHANGES'} | ${false} | ${'error'} `( 'renders $icon for reviewState:$reviewState and approved:$approved', ({ reviewState, approved, icon }) => { diff --git a/spec/helpers/projects/project_members_helper_spec.rb b/spec/helpers/projects/project_members_helper_spec.rb index 2cc87e8aeb9..0a85b28c919 100644 --- a/spec/helpers/projects/project_members_helper_spec.rb +++ b/spec/helpers/projects/project_members_helper_spec.rb @@ -113,6 +113,9 @@ RSpec.describe Projects::ProjectMembersHelper do let_it_be(:top_group) { create(:group) } let_it_be(:sub_group) { create(:group, parent: top_group) } let_it_be(:project) { create(:project, group: sub_group) } + let_it_be(:members) { create_list(:project_member, 2, project: project) } + let_it_be(:invited) { create_list(:project_member, 2, :invited, project: project) } + let_it_be(:access_requests) { create_list(:project_member, 2, :access_request, project: project) } let_it_be(:group_link_1) { create(:group_group_link, shared_group: top_group, shared_with_group: shared_with_group_1, group_access: Gitlab::Access::GUEST) } let_it_be(:group_link_2) { create(:group_group_link, shared_group: top_group, shared_with_group: shared_with_group_4, group_access: Gitlab::Access::GUEST) } let_it_be(:group_link_3) { create(:group_group_link, shared_group: top_group, shared_with_group: shared_with_group_5, group_access: Gitlab::Access::DEVELOPER) } diff --git a/spec/models/members/members/invited_private_group_accessibility_assigner_spec.rb b/spec/models/members/members/invited_private_group_accessibility_assigner_spec.rb new file mode 100644 index 00000000000..1f720b779cf --- /dev/null +++ b/spec/models/members/members/invited_private_group_accessibility_assigner_spec.rb @@ -0,0 +1,180 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Members::InvitedPrivateGroupAccessibilityAssigner, feature_category: :groups_and_projects do + using RSpec::Parameterized::TableSyntax + + let_it_be(:user) { create(:user) } + + describe '#initialize' do + it 'raises an error when initializing using ActiveRecord::Relation' do + members = Member.all + expect { described_class.new(members, source: nil, current_user: user) }.to raise_error(ArgumentError) + end + + it 'does not raise an error when initializing using Array or MembersPresenter' do + expect { described_class.new([], source: nil, current_user: user) }.not_to raise_error + expect { described_class.new(MembersPresenter.new(nil), source: nil, current_user: user) }.not_to raise_error + end + end + + describe '#execute' do + shared_examples 'assigns is_source_accessible_to_current_user' do + let(:current_user) { user } + let_it_be(:member_user) { create(:user) } + + subject(:assigner) { described_class.new(members, source: source, current_user: current_user) } + + context 'for direct members' do + let_it_be(:source) { create(source_type) } # rubocop:disable Rails/SaveBang -- Using factory, not ActiveRecord + let_it_be(:direct_member) { source.add_developer(member_user) } + let(:members) { [direct_member] } + + it 'sets is_source_accessible_to_current_user to true for all members' do + assigner.execute + + expect(members.map(&:is_source_accessible_to_current_user)).to all(be_truthy) + end + end + + context 'for inherited members' do + let_it_be(:parent) { create(:group) } + let_it_be(:source) { create(source_type, parent_key => parent) } + let_it_be(:inherited_member) { parent.add_developer(member_user) } + let(:members) { [inherited_member] } + + it 'sets is_source_accessible_to_current_user to true for all members' do + assigner.execute + + expect(members.first.is_source_accessible_to_current_user).to eq(true) + end + end + + context 'for shared source members' do + let(:shared_source) { create(source_type, shared_source_visibility) } + let(:invited_group) { create(:group, invited_group_visibility) } + let(:source) { shared_source } + let(:invited_member) { invited_group.add_developer(member_user) } + let(:members) { [invited_member] } + let!(:link) { create_link(source, invited_group) } + + shared_examples 'sets correct is_source_accessible_to_current_user for invited members' do + with_them do + specify do + assigner.execute + + expect(members.first.is_source_accessible_to_current_user).to eq(can_see_invited_members_source?) + end + + context 'with multiple members belonging to different sources' do + it 'avoid N+1 queries' do + assigner # Initialize objects in let blocks + recorder = ActiveRecord::QueryRecorder.new { assigner.execute } + + members = create_list(:group, 3, invited_group_visibility).map do |invited_group| + create_link(shared_source, invited_group) + create(:group_member, group: invited_group) + end + + assigner = described_class.new(members, source: shared_source, current_user: current_user) + expect { assigner.execute }.not_to exceed_query_limit(recorder) + end + end + end + end + + context 'when current user is unauthenticated' do + let(:current_user) { nil } + + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | false + end + + include_examples 'sets correct is_source_accessible_to_current_user for invited members' + end + + context 'when current user non-member of shared source' do + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | false + end + + include_examples 'sets correct is_source_accessible_to_current_user for invited members' + end + + context 'when current user a member of shared source but not of invited group' do + before do + shared_source.add_developer(current_user) + end + + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | false + :private | :public | true + :private | :private | false + end + + include_examples 'sets correct is_source_accessible_to_current_user for invited members' + end + + context 'when current user a direct member of shared group and of invited group through sharing' do + before do + group = create(:group, :private) + group.add_developer(current_user) + create(:group_group_link, shared_group: invited_group, shared_with_group: group) + end + + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | true + :private | :public | true + :private | :private | true + end + + include_examples 'sets correct is_source_accessible_to_current_user for invited members' + end + + context 'when current user can manage member of shared group not invited group members' do + before do + shared_source.add_member(current_user, admin_member_access) + end + + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | true + :private | :public | true + :private | :private | true + end + + include_examples 'sets correct is_source_accessible_to_current_user for invited members' + end + end + end + + context 'for project members' do + let_it_be(:source_type) { 'project' } + let_it_be(:admin_member_access) { Gitlab::Access::MAINTAINER } + let_it_be(:parent_key) { :group } + + it_behaves_like 'assigns is_source_accessible_to_current_user' + + def create_link(shared, invited) + create(:project_group_link, project: shared, group: invited) + end + end + + context 'for group members' do + let_it_be(:source_type) { 'group' } + let_it_be(:admin_member_access) { Gitlab::Access::OWNER } + let_it_be(:parent_key) { :parent } + + it_behaves_like 'assigns is_source_accessible_to_current_user' + + def create_link(shared, invited) + create(:group_group_link, shared_group: shared, shared_with_group: invited) + end + end + end +end diff --git a/spec/requests/groups/achievements_controller_spec.rb b/spec/requests/groups/achievements_controller_spec.rb index 26ca0039984..b3f2bcdf07a 100644 --- a/spec/requests/groups/achievements_controller_spec.rb +++ b/spec/requests/groups/achievements_controller_spec.rb @@ -75,4 +75,10 @@ RSpec.describe Groups::AchievementsController, feature_category: :user_profile d it_behaves_like 'ok response with index template if authorized' end + + describe 'GET #new' do + subject { get new_group_achievement_path(group) } + + it_behaves_like 'ok response with index template if authorized' + end end diff --git a/spec/scripts/internal_events/cli_spec.rb b/spec/scripts/internal_events/cli_spec.rb index ab442224c4e..23dae73e0f9 100644 --- a/spec/scripts/internal_events/cli_spec.rb +++ b/spec/scripts/internal_events/cli_spec.rb @@ -611,7 +611,7 @@ RSpec.describe Cli, feature_category: :service_ping do ]) expect_cli_output do - output = plain_last_lines(1000) + output = plain_last_lines output.include?(expected_example_prompt) && output.include?(expected_rails_example) && @@ -675,7 +675,7 @@ RSpec.describe Cli, feature_category: :service_ping do ]) expect_cli_output do - output = plain_last_lines(320) + output = plain_last_lines output.include?(expected_example_prompt) && output.include?(expected_event1_example) && @@ -846,10 +846,10 @@ RSpec.describe Cli, feature_category: :service_ping do prompt.input.rewind end - def plain_last_lines(size) - prompt.output.string - .lines - .last(size) + def plain_last_lines(size = nil) + lines = prompt.output.string.lines + lines = lines.last(size) if size + lines .join('') .gsub(/\e[^\sm]{2,4}[mh]/, '') # Ignore text colors .gsub(/(\e\[(2K|1G|1A))+\z/, '') # Remove trailing characters if timeout occurs diff --git a/spec/serializers/member_entity_spec.rb b/spec/serializers/member_entity_spec.rb index 350355eb72d..2f4a803d61b 100644 --- a/spec/serializers/member_entity_spec.rb +++ b/spec/serializers/member_entity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MemberEntity do +RSpec.describe MemberEntity, feature_category: :groups_and_projects do let_it_be(:current_user) { create(:user) } let(:entity) { described_class.new(member, { current_user: current_user, group: group, source: source }) } @@ -24,6 +24,28 @@ RSpec.describe MemberEntity do expect(entity_hash[:can_remove]).to be(true) end + + context 'when is_source_accessible_to_current_user is true' do + before do + allow(member).to receive(:is_source_accessible_to_current_user).and_return(true) + end + + it 'exposes source and created_by' do + expect(entity_hash[:source]).to be_present + expect(entity_hash[:created_by]).to be_present + end + end + + context 'when is_source_accessible_to_current_user is false' do + before do + allow(member).to receive(:is_source_accessible_to_current_user).and_return(false) + end + + it 'does not exposes source and created_by' do + expect(entity_hash[:source]).to be_nil + expect(entity_hash[:created_by]).to be_nil + end + end end shared_examples 'invite' do @@ -72,12 +94,20 @@ RSpec.describe MemberEntity do context 'group member' do let(:group) { create(:group) } let(:source) { group } - let(:member) { GroupMemberPresenter.new(create(:group_member, group: group), current_user: current_user) } + let(:member) do + GroupMemberPresenter.new( + create(:group_member, group: group, created_by: current_user), current_user: current_user + ) + end it_behaves_like 'member.json' context 'invite' do - let(:member) { GroupMemberPresenter.new(create(:group_member, :invited, group: group), current_user: current_user) } + let(:member) do + GroupMemberPresenter.new( + create(:group_member, :invited, group: group, created_by: current_user), current_user: current_user + ) + end it_behaves_like 'member.json' it_behaves_like 'invite' @@ -125,12 +155,20 @@ RSpec.describe MemberEntity do let(:project) { create(:project) } let(:group) { project.group } let(:source) { project } - let(:member) { ProjectMemberPresenter.new(create(:project_member, project: project), current_user: current_user) } + let(:member) do + ProjectMemberPresenter.new( + create(:project_member, project: project, created_by: current_user), current_user: current_user + ) + end it_behaves_like 'member.json' context 'invite' do - let(:member) { ProjectMemberPresenter.new(create(:project_member, :invited, project: project), current_user: current_user) } + let(:member) do + ProjectMemberPresenter.new( + create(:project_member, :invited, project: project, created_by: current_user), current_user: current_user + ) + end it_behaves_like 'member.json' it_behaves_like 'invite' diff --git a/spec/serializers/member_serializer_spec.rb b/spec/serializers/member_serializer_spec.rb index c35ecf5f636..ee198733ef2 100644 --- a/spec/serializers/member_serializer_spec.rb +++ b/spec/serializers/member_serializer_spec.rb @@ -2,8 +2,9 @@ require 'spec_helper' -RSpec.describe MemberSerializer do +RSpec.describe MemberSerializer, feature_category: :groups_and_projects do include MembersPresentation + using RSpec::Parameterized::TableSyntax let_it_be(:current_user) { create(:user) } @@ -15,6 +16,80 @@ RSpec.describe MemberSerializer do it { is_expected.to match_schema('members') } end + shared_examples 'shared source members' do + let_it_be(:member_user) { create(:user) } + let(:shared_source) { create(source_type, shared_source_visibility) } + let(:invited_group) { create(:group, invited_group_visibility) } + let(:source) { shared_source } + let(:group) { source.is_a?(Project) ? source.group : source } + let(:invited_member) { invited_group.add_developer(member_user) } + let(:members) { present_members([invited_member]) } + + shared_examples 'exposes source correctly' do + with_them do + before do + create_link(source, invited_group) + end + + specify do + representation + + expect(invited_member.is_source_accessible_to_current_user).to eq(can_see_invited_members_source?) + end + end + end + + context 'when current user is unauthenticated' do + let(:current_user) { nil } + + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | false + end + + include_examples 'exposes source correctly' + end + + context 'when current user non-member of shared source' do + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | false + end + + include_examples 'exposes source correctly' + end + + context 'when current user a member of shared source but not of invited group' do + before do + shared_source.add_developer(current_user) + end + + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | false + :private | :public | true + :private | :private | false + end + + include_examples 'exposes source correctly' + end + + context 'when current user can manage member of shared group but not invited group members' do + before do + shared_source.add_member(current_user, admin_member_access) + end + + where(:shared_source_visibility, :invited_group_visibility, :can_see_invited_members_source?) do + :public | :public | true + :public | :private | true + :private | :public | true + :private | :private | true + end + + include_examples 'exposes source correctly' + end + end + context 'group member' do let_it_be(:group) { create(:group) } let_it_be(:members) { present_members(create_list(:group_member, 1, group: group)) } @@ -29,6 +104,15 @@ RSpec.describe MemberSerializer do expect { representation }.to change(group_member, :last_owner) .from(nil).to(true) end + + it_behaves_like 'shared source members' do + let_it_be(:source_type) { :group } + let_it_be(:admin_member_access) { Gitlab::Access::OWNER } + + def create_link(shared, invited) + create(:group_group_link, shared_group: shared, shared_with_group: invited) + end + end end context 'project member' do @@ -45,5 +129,14 @@ RSpec.describe MemberSerializer do representation end + + it_behaves_like 'shared source members' do + let_it_be(:source_type) { :project } + let_it_be(:admin_member_access) { Gitlab::Access::MAINTAINER } + + def create_link(shared, invited) + create(:project_group_link, project: shared, group: invited) + end + end end end diff --git a/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb index 5892fc32e94..e3ae1671642 100644 --- a/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true RSpec.shared_examples 'move quick action' do + before do + allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(105) + end + context 'move the issue to another project' do let(:target_project) { create(:project, :public) }