diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb index 7ab4ddaf628..2607741db3e 100644 --- a/app/controllers/concerns/group_tree.rb +++ b/app/controllers/concerns/group_tree.rb @@ -8,7 +8,7 @@ module GroupTree def render_group_tree(groups) groups = groups.sort_by_attribute(@sort = safe_params[:sort]) - groups = if safe_params[:filter].present? + groups = if search_descendants? filtered_groups_with_ancestors(groups) elsif safe_params[:parent_id].present? groups.where(parent_id: safe_params[:parent_id]).page(safe_params[:page]) @@ -24,8 +24,12 @@ module GroupTree format.json do serializer = GroupChildSerializer.new(current_user: current_user) .with_pagination(request, response) - serializer.expand_hierarchy if safe_params[:filter].present? - render json: serializer.represent(@groups) + + serializer.expand_hierarchy if search_descendants? + + render json: serializer.represent(@groups, { + upto_preloaded_ancestors_only: inactive? + }) end end # rubocop:enable Gitlab/ModuleWithInstanceVariables @@ -44,10 +48,20 @@ module GroupTree # # Pagination needs to be applied before loading the ancestors to # make sure ancestors are not cut off by pagination. - Group.where(id: filtered_groups.select(:id)).self_and_ancestors + ancestors = Group.where(id: filtered_groups.select(:id)).self_and_ancestors + ancestors = ancestors.self_or_ancestors_inactive if inactive? + ancestors end # rubocop: enable CodeReuse/ActiveRecord + def inactive? + safe_params[:active] == false + end + + def search_descendants? + safe_params[:filter].present? || inactive? + end + def safe_params params.merge( active: Gitlab::Utils.to_boolean(params[:active]), diff --git a/app/controllers/dashboard/groups_controller.rb b/app/controllers/dashboard/groups_controller.rb index 39bee37ee05..61291459302 100644 --- a/app/controllers/dashboard/groups_controller.rb +++ b/app/controllers/dashboard/groups_controller.rb @@ -10,7 +10,7 @@ class Dashboard::GroupsController < Dashboard::ApplicationController urgency :low, [:index] def index - groups = GroupsFinder.new(current_user, all_available: false).execute + groups = GroupsFinder.new(current_user, all_available: false, active: safe_params[:active]).execute render_group_tree(groups) end end diff --git a/app/controllers/explore/groups_controller.rb b/app/controllers/explore/groups_controller.rb index f0091420564..ae4ef46deb0 100644 --- a/app/controllers/explore/groups_controller.rb +++ b/app/controllers/explore/groups_controller.rb @@ -9,6 +9,6 @@ class Explore::GroupsController < Explore::ApplicationController MAX_QUERY_SIZE = 10_000 def index - render_group_tree GroupsFinder.new(current_user).execute.limit(MAX_QUERY_SIZE) + render_group_tree GroupsFinder.new(current_user, active: safe_params[:active]).execute.limit(MAX_QUERY_SIZE) end end diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 822273de457..6f8ad6ccf6b 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -2238,6 +2238,7 @@ Input type: `AiDuoWorkflowCreateInput` | `agentPrivileges` | [`[Int!]`](#int) | Actions the agent is allowed to perform. | | `allowAgentToRequestUser` | [`Boolean`](#boolean) | When enabled, Duo Workflow may stop to ask the user questions before proceeding. | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `environment` | [`WorkflowEnvironment`](#workflowenvironment) | Environment for the workflow. | | `goal` | [`String`](#string) | Goal of the workflow. | | `preApprovedAgentPrivileges` | [`[Int!]`](#int) | Actions the agent can perform without asking for approval. | | `projectId` | [`ProjectID`](#projectid) | Global ID of the project the user is acting on. | diff --git a/doc/development/code_review.md b/doc/development/code_review.md index aa97534b841..e22361e51f2 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -88,7 +88,7 @@ To find a domain expert: - In the Merge Request approvals widget, select [View eligible approvers](../user/project/merge_requests/approvals/rules.md#eligible-approvers). This widget shows recommended and required approvals per area of the codebase. - These rules are defined in [Code Owners](../user/project/merge_requests/approvals/rules.md#code-owners-as-eligible-approvers). + These rules are defined in [Code Owners](../user/project/merge_requests/approvals/rules.md#code-owners-as-approvers). - View the list of team members who work in the [stage or group](https://handbook.gitlab.com/handbook/product/categories/#devops-stages) related to the merge request. - View team members' domain expertise on the [engineering projects](https://handbook.gitlab.com/handbook/engineering/projects/) page or on the [GitLab team page](https://about.gitlab.com/company/team/). Domains are self-identified, so use your judgment to map the changes on your merge request to a domain. - Look for team members who have contributed to the files in the merge request. View the logs by running `git log `. diff --git a/doc/subscriptions/bronze_starter.md b/doc/subscriptions/bronze_starter.md index 829bc198547..411238f3f0d 100644 --- a/doc/subscriptions/bronze_starter.md +++ b/doc/subscriptions/bronze_starter.md @@ -67,7 +67,7 @@ the tiers are no longer mentioned in GitLab documentation: - [Multiple assignees](../user/project/merge_requests/_index.md#assign-a-user-to-a-merge-request) - [Approval rule information for reviewers](../user/project/merge_requests/reviews/_index.md#request-a-review) - [Required Approvals](../user/project/merge_requests/approvals/_index.md#required-approvals) - - [Code Owners as eligible approvers](../user/project/merge_requests/approvals/rules.md#code-owners-as-eligible-approvers) + - [Code Owners as eligible approvers](../user/project/merge_requests/approvals/rules.md#code-owners-as-approvers) - [Approval rules](../user/project/merge_requests/approvals/rules.md) features - [Restricting push and merge access to certain users](../user/project/repository/branches/protected.md) - Metrics and analytics: diff --git a/doc/user/project/codeowners/_index.md b/doc/user/project/codeowners/_index.md index 19f72960101..c78399e99f3 100644 --- a/doc/user/project/codeowners/_index.md +++ b/doc/user/project/codeowners/_index.md @@ -2,7 +2,7 @@ stage: Create group: Source Code 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 -description: Use Code Owners to define experts for your code base, and set review requirements based on file type or location. +description: Use Code Owners to define experts for your codebase, and set review requirements based on file type or location. title: Code Owners --- @@ -51,6 +51,8 @@ For example: +For information about who is eligible to approve merge requests as either an approver or Code Owner, see [Approver by membership type](../merge_requests/approvals/rules.md#approver-by-membership-type). + ## Code Owners and protected branches To ensure merge request changes are reviewed and approved by Code Owners, specified in the @@ -109,7 +111,7 @@ Prerequisites: 1. Create a `CODEOWNERS` file in your [preferred location](#codeowners-file). 1. Define some rules in the file following the [`CODEOWNERS` syntax](reference.md). Some suggestions: - - Configure [All eligible approvers](../merge_requests/approvals/rules.md#code-owners-as-eligible-approvers) approval rule. + - Configure [All eligible approvers](../merge_requests/approvals/rules.md#code-owners-as-approvers) approval rule. - [Require Code Owner approval](../repository/branches/protected.md#require-code-owner-approval) on a protected branch. 1. Commit your changes, and push them up to GitLab. diff --git a/doc/user/project/merge_requests/approvals/rules.md b/doc/user/project/merge_requests/approvals/rules.md index e6d9ede5246..03a5b7cd2eb 100644 --- a/doc/user/project/merge_requests/approvals/rules.md +++ b/doc/user/project/merge_requests/approvals/rules.md @@ -15,7 +15,7 @@ title: Merge request approval rules Approval rules define how many [approvals](_index.md) a merge request must receive before it can be merged, and which users should do the approving. They can be used in conjunction -with [code owners](#code-owners-as-eligible-approvers) to ensure that changes are +with [code owners](#code-owners-as-approvers) to ensure that changes are reviewed both by the group maintaining the feature, and any groups responsible for specific areas of oversight. @@ -119,122 +119,13 @@ reduces the number of approvals left (the **Approvals** column) for all rules th For an overview, see [Multiple Approvers](https://www.youtube.com/watch?v=8JQJ5821FrA). -## Eligible approvers - -To be eligible as an approver for your project, a user must be a direct member of at least one of the following: - -- Your project. -- Your project's group. -- Any of your project's group's parent groups. -- Another group that has been [shared with your project](../../members/sharing_projects_groups.md#sharing-projects). -- Another group that has been [shared with your project's group or any of the group's parents](../../members/sharing_projects_groups.md#sharing-groups). -- A [group added as approvers](#group-approvers). - -Users with the Developer role can approve merge requests if one of the following is true: - -- Users added as approvers at the project or merge request level. -- Users who are [Code owners](#code-owners-as-eligible-approvers) of the files - changed in the merge request. - -Users with the Reporter role can approve only if all of the following are true: - -- The users are part of a group that has been [shared](../../members/sharing_projects_groups.md) with the project. - The group must have at least the Reporter role. -- The group has been added as merge request approvers. -- Approval permissions for users with the reporter role [are enabled](#enable-approval-permissions-for-users-with-the-reporter-role). - -To show who has participated in the merge request review, the Approvals widget in -a merge request displays a **Commented by** column. This column lists eligible approvers -who commented on the merge request. It helps authors and reviewers identify who to -contact with questions about the merge request's content. - -If the number of required approvals is greater than the number of assigned approvers, -approvals from other users with at least the Developer role -in the project count toward meeting the required number of approvals, even if the -users were not explicitly listed in the approval rules. - -### Get notified about all merge requests you can approve +## Get notified about all merge requests you can approve To get email notifications every time a merge request you're eligible to approve is created: - [Set your notification level](../../../profile/notifications.md#edit-notification-settings) to **Custom** and select the **Merge request you're eligible to approve is created** event. -### Group approvers - -You can add a group of users as approvers. All direct members of this group -can approve the rule. Inherited members cannot approve the rule. - -Typically the group is a subgroup in your top-level namespace, unless you are -collaborating with an external group. If you are collaborating with another group -and want to use members of that group as approvers, you can either: - -- [Share access to the project](../../members/sharing_projects_groups.md#sharing-projects). -- [Share access to your project's group](../../members/sharing_projects_groups.md#sharing-groups), - which gives the external group approval access to all projects in your project's group. - -A user's membership in an approver group determines their individual approval permissions -in the following ways: - -- Inherited members are not considered approvers. Only direct members can approve merge requests. -- A user from a group approver group who is later also added as an individual approver - counts as one approver, not two. -- Merge request authors do not count as eligible approvers on their own merge requests by default. - To change this behavior, disable the - [**Prevent author approval**](settings.md#prevent-approval-by-author) - project setting. -- By default, committers to merge requests can approve a merge request. To change this behavior, enable - the [**Prevent committers approval**](settings.md#prevent-approvals-by-users-who-add-commits) - project setting. - -### Code owners as eligible approvers - -If you add [code owners](../../codeowners/_index.md) to your repository, the owners of files -become eligible approvers in the project. To enable this merge request approval rule: - -1. On the left sidebar, select **Search or go to** and find your project. -1. Select **Settings > Merge requests**. -1. In the **Merge request approvals** section, in the **Approval rules** section, locate the **All eligible users** rule. -1. In the **Approvals required** column, enter the number of approvals required. - -You can also -[require code owner approval](../../repository/branches/protected.md#require-code-owner-approval) -for protected branches. - -## Enable approval permissions for users with the Reporter role - -Before users with the Reporter role can merge to a protected branch, you might have to grant them -permission to approve merge requests. -Some users (like managers) might not need permission to push or merge code, but still need -oversight on proposed work. - -Users with the Reporter role can approve merge requests only through regular approval rules. -Code owner approval rules require users to have at least the Developer role. For more information, -see [Eligible approvers](#eligible-approvers). - -Prerequisites: - -- You must select a specific branch, as this method does not work with `All Branches` or `All protected branches` settings. -- The shared group must be added to an approval rule and not individual users, even when the added user is part of the group. - -To enable approval permissions for these users without granting them push access: - -1. [Create a protected branch](../../repository/branches/protected.md) -1. [Create a new group](../../../group/_index.md#create-a-group). -1. [Add the user to the group](../../../group/_index.md#add-users-to-a-group), - and select the Reporter role for the user. Do not assign roles with higher permissions than - Reporter due to a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/492467). - Assigning higher roles may result in unexpected behavior. -1. [Share the project with your group](../../members/sharing_projects_groups.md#invite-a-group-to-a-project), - with at least the Reporter role. -1. On the left sidebar, select **Search or go to** and find your project. -1. Select **Settings > Merge requests**. -1. In the **Merge request approvals** section, in the **Approval rules** section: - - For a new rule, select **Add approval rule** and target the protected branch. - - For an existing rule, select **Edit** and target the protected branch. -1. On the right sidebar, in **Add approvers**, select the group you created. -1. Select **Save changes**. - ## Edit or override merge request approval rules You can override the merge request approval rules for a project by either: @@ -300,6 +191,40 @@ approval rule for certain branches: 1. To enable this configuration, follow [Require Code Owner approval on a protected branch](../../repository/branches/protected.md#require-code-owner-approval). +## Enable approval permissions for users with the Reporter role + +Before users with the Reporter role can merge to a protected branch, you might have to grant them +permission to approve merge requests. +Some users (like managers) might not need permission to push or merge code, but still need +oversight on proposed work. + +Users with the Reporter role can approve merge requests only through regular approval rules. +Code owner approval rules require users to have at least the Developer role. For more information, +see [Eligible approvers](#eligible-approvers). + +Prerequisites: + +- You must select a specific branch, as this method does not work with `All Branches` or `All protected branches` settings. +- The shared group must be added to an approval rule and not individual users, even when the added user is part of the group. + +To enable approval permissions for these users without granting them push access: + +1. [Create a protected branch](../../repository/branches/protected.md) +1. [Create a new group](../../../group/_index.md#create-a-group). +1. [Add the user to the group](../../../group/_index.md#add-users-to-a-group), + and select the Reporter role for the user. Do not assign roles with higher permissions than + Reporter due to a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/492467). + Assigning higher roles may result in unexpected behavior. +1. [Share the project with your group](../../members/sharing_projects_groups.md#invite-a-group-to-a-project), + with at least the Reporter role. +1. On the left sidebar, select **Search or go to** and find your project. +1. Select **Settings > Merge requests**. +1. In the **Merge request approvals** section, in the **Approval rules** section: + - For a new rule, select **Add approval rule** and target the protected branch. + - For an existing rule, select **Edit** and target the protected branch. +1. On the right sidebar, in **Add approvers**, select the group you created. +1. Select **Save changes**. + ## Security Approvals {{< details >}} @@ -329,6 +254,123 @@ on the merge request to indicate which steps are needed to proceed. These policies are both created and edited in the [security policy editor](../../../application_security/policies/_index.md#policy-editor). +## Eligible approvers + +To be eligible as an approver for your project, a user must be a direct member of at least one of the following: + +- Your project. +- Your project's group. +- Any of your project's group's parent groups. +- Another group that has been [shared with your project](../../members/sharing_projects_groups.md#sharing-projects). +- Another group that has been [shared with your project's group or any of the group's parents](../../members/sharing_projects_groups.md#sharing-groups). +- A [group added as approvers](#group-approvers). + +Users with the Developer role can approve merge requests if one of the following is true: + +- Users added as approvers at the project or merge request level. +- Users who are [Code owners](#code-owners-as-approvers) of the files + changed in the merge request. + +Users with the Reporter role can approve only if all of the following are true: + +- The users are part of a group that has been [shared](../../members/sharing_projects_groups.md) with the project. + The group must have at least the Reporter role. +- The group has been added as merge request approvers. +- Approval permissions for users with the reporter role [are enabled](#enable-approval-permissions-for-users-with-the-reporter-role). + +To show who has participated in the merge request review, the Approvals widget in +a merge request displays a **Commented by** column. This column lists eligible approvers +who commented on the merge request. It helps authors and reviewers identify who to +contact with questions about the merge request's content. + +If the number of required approvals is greater than the number of assigned approvers, +approvals from other users with at least the Developer role +in the project count toward meeting the required number of approvals, even if the +users were not explicitly listed in the approval rules. + +### Code owners as approvers + +If you add [code owners](../../codeowners/_index.md) to your repository, the owners of files +become eligible approvers in the project. To enable this merge request approval rule: + +1. On the left sidebar, select **Search or go to** and find your project. +1. Select **Settings > Merge requests**. +1. In the **Merge request approvals** section, in the **Approval rules** section, locate the **All eligible users** rule. +1. In the **Approvals required** column, enter the number of approvals required. + +You can also +[require code owner approval](../../repository/branches/protected.md#require-code-owner-approval) +for protected branches. + +### Approver by membership type + +The following tables show how membership type affects eligibility for both approval rules and +Code Owners. + +#### User eligibility + +When you assign individual users as approvers for approval rules or reference users in `CODEOWNERS` +files, like `@username`: + +| Membership type | Approval rules | Code Owners | +|--------------------------------------------------------------------------------------------------------------------|---------------------------------------------|-------------| +| Direct member of the project | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | +| Direct member of the project's group | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | +| Inherited member of the project's group | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | +| Direct member of a [group invited to the project](../../members/sharing_projects_groups.md#sharing-projects) | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | +| Inherited member of a group invited to the project | {{< icon name="dash-circle" >}} No | {{< icon name="dash-circle" >}} No | +| Direct member of a [group invited to the project's group](../../members/sharing_projects_groups.md#sharing-groups) | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | +| Inherited member of a group invited to the project's group | {{< icon name="dash-circle" >}} No | {{< icon name="dash-circle" >}} No | +| Direct member of a group invited to the project's group's parent groups | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | +| Inherited member of a group invited to the project's group's parent groups | {{< icon name="dash-circle" >}} No | {{< icon name="dash-circle" >}} No | + +#### Group eligibility + +When you assign groups as approvers for approval rules or reference groups in `CODEOWNERS` files, +like `@group-name`, only direct members of eligible groups can provide approvals: + +| Group type | Approval rules | Code Owners | +|------------|----------------|-------------| +| [Groups invited to the project](../../members/sharing_projects_groups.md#sharing-projects) | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | +| [Groups invited to the project's group](../../members/sharing_projects_groups.md#sharing-groups) | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="dash-circle" >}} No | +| Groups invited to a parent of the project's group | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="dash-circle" >}} No | +| The project's group | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | +| A parent of the project's group | {{< icon name="check-circle-filled" >}} Yes | {{< icon name="check-circle-filled" >}} Yes | + +{{< alert type="note" >}} + +For group-based approvals, only direct members of the group can approve merge requests. +Inherited members of the eligible groups cannot provide approvals. + +{{< /alert >}} + +### Group approvers + +You can add a group of users as approvers. All direct members of this group +can approve the rule. Inherited members cannot approve the rule. + +Typically the group is a subgroup in your top-level namespace, unless you are +collaborating with an external group. If you are collaborating with another group +and want to use members of that group as approvers, you can either: + +- [Share access to the project](../../members/sharing_projects_groups.md#sharing-projects). +- [Share access to your project's group](../../members/sharing_projects_groups.md#sharing-groups), + which gives the external group approval access to all projects in your project's group. + +A user's membership in an approver group determines their individual approval permissions +in the following ways: + +- Inherited members are not considered approvers. Only direct members can approve merge requests. +- A user from a group approver group who is later also added as an individual approver + counts as one approver, not two. +- Merge request authors do not count as eligible approvers on their own merge requests by default. + To change this behavior, disable the + [**Prevent author approval**](settings.md#prevent-approval-by-author) + project setting. +- By default, committers to merge requests can approve a merge request. To change this behavior, enable + the [**Prevent committers approval**](settings.md#prevent-approvals-by-users-who-add-commits) + project setting. + ## Troubleshooting ### Approval rule name can't be blank diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 20175d1f0a3..ab714063ced 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15509,6 +15509,9 @@ msgstr "" msgid "Compliance Center|Violations" msgstr "" +msgid "Compliance and security policy group for this instance" +msgstr "" + msgid "Compliance center" msgstr "" @@ -56566,6 +56569,9 @@ msgstr "" msgid "SecurityOrchestration|Save allowlist" msgstr "" +msgid "SecurityOrchestration|Save changes" +msgstr "" + msgid "SecurityOrchestration|Save denylist" msgstr "" @@ -56644,6 +56650,9 @@ msgstr "" msgid "SecurityOrchestration|Select a cadence" msgstr "" +msgid "SecurityOrchestration|Select a group" +msgstr "" + msgid "SecurityOrchestration|Select a project to store your security policies in. %{linkStart}More information.%{linkEnd}" msgstr "" @@ -56842,6 +56851,9 @@ msgstr "" msgid "SecurityOrchestration|Type in custom variable" msgstr "" +msgid "SecurityOrchestration|Unassign group" +msgstr "" + msgid "SecurityOrchestration|Unknown source" msgstr "" @@ -57795,6 +57807,9 @@ msgstr "" msgid "Select a group" msgstr "" +msgid "Select a group to create, distribute, and manage compliance frameworks and security polices for this GitLab instance. You should select a group without existing compliance frameworks." +msgstr "" + msgid "Select a label" msgstr "" diff --git a/qa/qa/page/admin/settings/component/usage_statistics.rb b/qa/qa/page/admin/settings/component/usage_statistics.rb index d11eff5b5f9..cd6e6aa274a 100644 --- a/qa/qa/page/admin/settings/component/usage_statistics.rb +++ b/qa/qa/page/admin/settings/component/usage_statistics.rb @@ -10,8 +10,9 @@ module QA element 'enable-usage-data-checkbox' end - def has_disabled_usage_data_checkbox? - has_element?('enable-usage-data-checkbox', disabled: true, visible: false) + def has_usage_data_checkbox_checked_and_disabled? + checkbox = find_element('enable-usage-data-checkbox', visible: false) + checkbox.disabled? && checkbox.checked? end end end diff --git a/qa/qa/specs/features/browser_ui/14_analytics/service_ping_default_enabled_spec.rb b/qa/qa/specs/features/browser_ui/14_analytics/service_ping_default_enabled_spec.rb index 360d70d6118..65fc7af76b0 100644 --- a/qa/qa/specs/features/browser_ui/14_analytics/service_ping_default_enabled_spec.rb +++ b/qa/qa/specs/features/browser_ui/14_analytics/service_ping_default_enabled_spec.rb @@ -2,8 +2,8 @@ module QA RSpec.describe 'Analytics' do - describe 'Service ping default enabled', product_group: :analytics_instrumentation do - context 'when using default enabled from gitlab.yml config', :requires_admin do + describe 'Service ping default checked', product_group: :analytics_instrumentation do + context 'when using default gitlab.yml config', :requires_admin do before do Flow::Login.sign_in_as_admin @@ -12,12 +12,12 @@ module QA end it( - 'has service ping toggle enabled', + 'has service ping checked (but disabled)', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348335' ) do Page::Admin::Settings::MetricsAndProfiling.perform do |setting| setting.expand_usage_statistics do |page| - expect(page).not_to have_disabled_usage_data_checkbox + expect(page).to have_usage_data_checkbox_checked_and_disabled end end end diff --git a/spec/controllers/concerns/group_tree_spec.rb b/spec/controllers/concerns/group_tree_spec.rb index 95d5b275391..4b705407121 100644 --- a/spec/controllers/concerns/group_tree_spec.rb +++ b/spec/controllers/concerns/group_tree_spec.rb @@ -3,20 +3,19 @@ require 'spec_helper' RSpec.describe GroupTree, feature_category: :groups_and_projects do - let(:group) { create(:group, :public) } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group, :public, owners: [user]) } controller(ApplicationController) do # `described_class` is not available in this context include GroupTree def index - render_group_tree GroupsFinder.new(current_user).execute + render_group_tree GroupsFinder.new(current_user, active: safe_params[:active]).execute end end before do - group.add_owner(user) sign_in(user) end @@ -79,6 +78,35 @@ RSpec.describe GroupTree, feature_category: :groups_and_projects do end end + context 'with active parameter' do + let_it_be(:active_ancestor) { group } + let_it_be(:inactive_ancestor) { create(:group, :archived, :public) } + let_it_be(:active_child) { create(:group, parent: active_ancestor) } + let_it_be(:inactive_child) { create(:group, parent: inactive_ancestor) } + + context 'when true' do + it 'only loads root group' do + allow(GroupsFinder).to receive(:execute).and_return(Group.where(id: active_child.id)) + + get :index, params: { active: true }, format: :json + + expect(assigns(:groups)).to include(active_ancestor) + expect(assigns(:groups)).not_to include(active_child, inactive_ancestor, inactive_child) + end + end + + context 'when false' do + it 'preloads inactive ancestors' do + allow(GroupsFinder).to receive(:execute).and_return(Group.where(id: inactive_child.id)) + + get :index, params: { active: false }, format: :json + + expect(assigns(:groups)).to include(inactive_ancestor, inactive_child) + expect(assigns(:groups)).not_to include(active_ancestor, active_child) + end + end + end + context 'json content' do it 'shows groups as json' do get :index, format: :json diff --git a/spec/controllers/dashboard/groups_controller_spec.rb b/spec/controllers/dashboard/groups_controller_spec.rb index f246d7bcaf1..dc4866b605a 100644 --- a/spec/controllers/dashboard/groups_controller_spec.rb +++ b/spec/controllers/dashboard/groups_controller_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Dashboard::GroupsController do include ExternalAuthorizationServiceHelpers - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } before do sign_in(user) @@ -53,5 +53,7 @@ RSpec.describe Dashboard::GroupsController do expect(response).to have_gitlab_http_status(:ok) end + + it_behaves_like 'groups controller with active parameter' end end diff --git a/spec/controllers/explore/groups_controller_spec.rb b/spec/controllers/explore/groups_controller_spec.rb index 867e2111774..e92c63383af 100644 --- a/spec/controllers/explore/groups_controller_spec.rb +++ b/spec/controllers/explore/groups_controller_spec.rb @@ -37,6 +37,8 @@ RSpec.describe Explore::GroupsController, feature_category: :groups_and_projects expect(response).to redirect_to new_user_session_path end end + + it_behaves_like 'groups controller with active parameter' end it_behaves_like 'explore groups' diff --git a/spec/support/shared_examples/controllers/groups_controller_shared_examples.rb b/spec/support/shared_examples/controllers/groups_controller_shared_examples.rb new file mode 100644 index 00000000000..2c324c5b5d0 --- /dev/null +++ b/spec/support/shared_examples/controllers/groups_controller_shared_examples.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'groups controller with active parameter' do + let_it_be(:active_group) { create(:group, :public, developers: [user]) } + let_it_be(:inactive_group) { create(:group, :archived, :public, developers: [user]) } + + let(:params) { {} } + + before do + get :index, params: params, format: :json + end + + context 'when true' do + let(:params) { { active: true } } + + it 'returns active group', :aggregate_failures do + expect(assigns(:groups)).to include(active_group) + expect(assigns(:groups)).not_to include(inactive_group) + end + end + + context 'when false' do + let(:params) { { active: false } } + + it 'returns inactive group' do + expect(assigns(:groups)).to contain_exactly(inactive_group) + end + + context 'when active group has inactive subgroup' do + let_it_be(:active_subgroup) { create(:group, parent: active_group) } + let_it_be(:inactive_subgroup) { create(:group, :archived, parent: active_group) } + + it 'returns inactive subgroup' do + expect(assigns(:groups)).to contain_exactly(inactive_group, inactive_subgroup) + end + end + + context 'when inactive group has active subgroup' do + let_it_be(:active_subgroup) { create(:group, parent: inactive_group) } + + it 'returns inactive group with subgroup' do + expect(assigns(:groups)).to contain_exactly(inactive_group, active_subgroup) + end + end + + context 'when inactive group has inactive subgroup' do + let_it_be(:inactive_subgroup) { create(:group, :archived, parent: inactive_group) } + + it 'returns inactive group with subgroup' do + expect(assigns(:groups)).to contain_exactly(inactive_group, inactive_subgroup) + end + end + + context "when groups has lower-level inactive subgroup" do + let_it_be(:inactive_subgroup) { create(:group, :archived, parent: active_group) } + let_it_be(:inactive_subsubgroup) { create(:group, :archived, parent: inactive_subgroup) } + + let(:params) { { active: false, filter: inactive_subsubgroup.name } } + + it 'returns inactive subgroup with its inactive parents' do + expect(json_response.first['id']).to eq(inactive_subgroup.id) + expect(json_response.first['children'].first['id']).to eq(inactive_subsubgroup.id) + end + end + end +end