diff options
18 files changed, 86 insertions, 135 deletions
diff --git a/app/assets/javascripts/ide/components/new_dropdown/modal.vue b/app/assets/javascripts/ide/components/new_dropdown/modal.vue index 1c5a00568eb..e3c230f7660 100644 --- a/app/assets/javascripts/ide/components/new_dropdown/modal.vue +++ b/app/assets/javascripts/ide/components/new_dropdown/modal.vue @@ -6,6 +6,10 @@ import { __, sprintf } from '~/locale'; import { modalTypes } from '../../constants'; import { trimPathComponents, getPathParent } from '../../utils'; +const i18n = { + cancelButtonText: __('Cancel'), +}; + export default { components: { GlModal, @@ -43,6 +47,18 @@ export default { return __('Create file'); }, + actionPrimary() { + return { + text: this.buttonLabel, + attributes: [{ variant: 'confirm' }], + }; + }, + actionCancel() { + return { + text: i18n.cancelButtonText, + attributes: [{ variant: 'default' }], + }; + }, isCreatingNewFile() { return this.modalType === modalTypes.blob; }, @@ -136,11 +152,11 @@ export default { data-qa-selector="new_file_modal" data-testid="ide-new-entry" :title="modalTitle" - :ok-title="buttonLabel" - ok-variant="success" size="lg" - @ok="submitForm" - @hide="resetData" + :action-primary="actionPrimary" + :action-cancel="actionCancel" + @primary="submitForm" + @cancel="resetData" > <div class="form-group row"> <label class="label-bold col-form-label col-sm-2"> {{ __('Name') }} </label> diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index a493016c268..b440c56849f 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -36,11 +36,6 @@ class Projects::IssuesController < Projects::ApplicationController before_action :authorize_import_issues!, only: [:import_csv] before_action :authorize_download_code!, only: [:related_branches] - # Limit the amount of issues created per minute - before_action -> { check_rate_limit!(:issues_create, scope: [@project, @current_user])}, - only: [:create], - if: -> { Feature.disabled?('rate_limited_service_issues_create', project, default_enabled: :yaml) } - before_action do push_frontend_feature_flag(:improved_emoji_picker, project, default_enabled: :yaml) push_frontend_feature_flag(:vue_issues_list, project&.group, default_enabled: :yaml) diff --git a/app/services/concerns/rate_limited_service.rb b/app/services/concerns/rate_limited_service.rb index c8dc60355cf..26aa150fd65 100644 --- a/app/services/concerns/rate_limited_service.rb +++ b/app/services/concerns/rate_limited_service.rb @@ -36,7 +36,6 @@ module RateLimitedService def rate_limit!(service) evaluated_scope = evaluated_scope_for(service) - return if feature_flag_disabled?(evaluated_scope[:project]) if rate_limiter.throttled?(key, **opts.merge(scope: evaluated_scope.values, users_allowlist: users_allowlist)) raise RateLimitedError.new(key: key, rate_limiter: rate_limiter), _('This endpoint has been requested too many times. Try again later.') @@ -54,10 +53,6 @@ module RateLimitedService all[var] = service.public_send(var) # rubocop: disable GitlabSecurity/PublicSend end end - - def feature_flag_disabled?(project) - Feature.disabled?("rate_limited_service_#{key}", project, default_enabled: :yaml) - end end prepended do diff --git a/app/views/groups/runners/edit.html.haml b/app/views/groups/runners/edit.html.haml index a0d7b8acb47..4a5bab94246 100644 --- a/app/views/groups/runners/edit.html.haml +++ b/app/views/groups/runners/edit.html.haml @@ -1,8 +1,14 @@ - breadcrumb_title _('Edit') - page_title _('Edit'), "##{@runner.id} (#{@runner.short_sha})" -- add_to_breadcrumbs _('CI/CD Settings'), group_settings_ci_cd_path(@group) + +- if Feature.enabled?(:runner_list_group_view_vue_ui, @group, default_enabled: :yaml) + - add_to_breadcrumbs _('Runners'), group_runners_path(@group) +- else + - add_to_breadcrumbs _('CI/CD Settings'), group_settings_ci_cd_path(@group) + - add_to_breadcrumbs "#{@runner.short_sha}", group_runner_path(@group, @runner) + %h2.page-title = s_('Runners|Runner #%{runner_id}' % { runner_id: @runner.id }) = render 'shared/runners/runner_type_badge', runner: @runner diff --git a/app/views/groups/runners/show.html.haml b/app/views/groups/runners/show.html.haml index 5cf83e8ccfd..72701491c67 100644 --- a/app/views/groups/runners/show.html.haml +++ b/app/views/groups/runners/show.html.haml @@ -1,3 +1,6 @@ -- add_to_breadcrumbs _('CI/CD Settings'), group_settings_ci_cd_path(@group) +- if Feature.enabled?(:runner_list_group_view_vue_ui, @group, default_enabled: :yaml) + - add_to_breadcrumbs _('Runners'), group_runners_path(@group) +- else + - add_to_breadcrumbs _('CI/CD Settings'), group_settings_ci_cd_path(@group) = render 'shared/runners/runner_details', runner: @runner diff --git a/app/views/projects/runners/_group_runners.html.haml b/app/views/projects/runners/_group_runners.html.haml index c25fd7a7587..8134ee8f417 100644 --- a/app/views/projects/runners/_group_runners.html.haml +++ b/app/views/projects/runners/_group_runners.html.haml @@ -28,7 +28,11 @@ = _('This group does not have any group runners yet.') - if can?(current_user, :admin_group_runners, @project.group) - - group_link = link_to _("group's CI/CD settings."), group_settings_ci_cd_path(@project.group) + - if Feature.enabled?(:runner_list_group_view_vue_ui, @group, default_enabled: :yaml) + - register_runners_path = group_runners_path(@project.group) + - else + - register_runners_path = group_settings_ci_cd_path(@project.group) + - group_link = link_to _("group's CI/CD settings."), register_runners_path = _('Group owners can register group runners in the %{link}').html_safe % { link: group_link } - else = _('Ask your group owner to set up a group runner.') diff --git a/config/feature_flags/development/rate_limited_service_issues_create.yml b/config/feature_flags/development/rate_limited_service_issues_create.yml deleted file mode 100644 index 95ece10aa6c..00000000000 --- a/config/feature_flags/development/rate_limited_service_issues_create.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: rate_limited_service_issues_create -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68526 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/342677 -milestone: '14.4' -type: development -group: group::project management -default_enabled: false diff --git a/doc/user/group/index.md b/doc/user/group/index.md index b6abc2b41aa..9d06314f295 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -230,8 +230,8 @@ In [GitLab Premium or higher](https://about.gitlab.com/pricing/), GitLab adminis There are two different ways to add a new project to a group: -- Select a group, and then click **New project**. You can then continue [creating your project](../../user/project/working_with_projects.md#create-a-project). -- While you are creating a project, select a group from the dropdown menu. +- Select a group, and then select **New project**. You can then continue [creating your project](../../user/project/working_with_projects.md#create-a-project). +- While you are creating a project, select a group from the dropdown list.  @@ -308,7 +308,7 @@ All the members of the `Engineering` group are added to the `Frontend` group. ## Manage group memberships via LDAP **(PREMIUM SELF)** -Group syncing allows LDAP groups to be mapped to GitLab groups. This provides more control over per-group user management. To configure group syncing, edit the `group_base` **DN** (`'OU=Global Groups,OU=GitLab INT,DC=GitLab,DC=org'`). This **OU** contains all groups that will be associated with GitLab groups. +Group syncing allows LDAP groups to be mapped to GitLab groups. This provides more control over per-group user management. To configure group syncing, edit the `group_base` **DN** (`'OU=Global Groups,OU=GitLab INT,DC=GitLab,DC=org'`). This **OU** contains all groups that are associated with GitLab groups. Group links can be created by using either a CN or a filter. To create these group links, go to the group's **Settings > LDAP Synchronization** page. After configuring the link, it may take more than an hour for the users to sync with the GitLab group. @@ -325,9 +325,9 @@ To create group links via CN: 1. Select the **LDAP Server** for the link. 1. As the **Sync method**, select `LDAP Group cn`. -1. In the **LDAP Group cn** field, begin typing the CN of the group. There is a dropdown menu with matching CNs in the configured `group_base`. Select your CN from this list. +1. In the **LDAP Group cn** field, begin typing the CN of the group. There is a dropdown list with matching CNs in the configured `group_base`. Select your CN from this list. 1. In the **LDAP Access** section, select the [permission level](../permissions.md) for users synced in this group. -1. Select the **Add Synchronization** button. +1. Select **Add Synchronization**. <!-- vale gitlab.Spelling = YES --> @@ -339,7 +339,7 @@ To create group links via filter: 1. As the **Sync method**, select `LDAP user filter`. 1. Input your filter in the **LDAP User filter** box. Follow the [documentation on user filters](../../administration/auth/ldap/index.md#set-up-ldap-user-filter). 1. In the **LDAP Access** section, select the [permission level](../permissions.md) for users synced in this group. -1. Select the **Add Synchronization** button. +1. Select **Add Synchronization**. ### Override user permissions **(PREMIUM SELF)** @@ -347,7 +347,7 @@ LDAP user permissions can be manually overridden by an administrator. To overrid 1. Go to your group's **Group information > Members** page. 1. In the row for the user you are editing, select the pencil (**{pencil}**) icon. -1. Select the brown **Edit permissions** button in the modal. +1. Select **Edit permissions** in the modal. Now you can edit the user's permissions from the **Members** page. @@ -375,7 +375,7 @@ Changing a group's path (group URL) can have unintended side effects. Read before you proceed. If you are changing the path so it can be claimed by another group or user, -you may need to rename the group too. Both names and paths must +you must rename the group too. Both names and paths must be unique. To retain ownership of the original namespace and protect the URL redirects, @@ -467,7 +467,7 @@ To restore a group that is marked for deletion: This setting is only available on top-level groups. It affects all subgroups. -When checked, any group within the top-level group hierarchy can be shared only with other groups within the hierarchy. +When checked, any group in the top-level group hierarchy can be shared only with other groups in the hierarchy. For example, with these groups: @@ -496,7 +496,7 @@ To prevent a project from being shared with other groups: 1. Go to the group's **Settings > General** page. 1. Expand the **Permissions and group features** section. -1. Select **Prevent sharing a project within `<group_name>` with other groups**. +1. Select **Prevent sharing a project in `<group_name>` with other groups**. 1. Select **Save changes**. This setting applies to all subgroups unless overridden by a group owner. Groups already @@ -603,7 +603,7 @@ You can export a list of members in a group or subgroup as a CSV. 1. Go to your group or subgroup and select either **Group information > Members** or **Subgroup information > Members**. 1. Select **Export as CSV**. -1. Once the CSV file has been generated, it is emailed as an attachment to the user that requested it. +1. After the CSV file has been generated, it is emailed as an attachment to the user that requested it. ## Restrict group access by IP address **(PREMIUM)** @@ -648,7 +648,7 @@ To restrict group access by IP address: > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/7297) in GitLab 12.2. > - Support for specifying multiple email domains [added](https://gitlab.com/gitlab-org/gitlab/-/issues/33143) in GitLab 13.1. -> - Support for restricting access to projects within the group [added](https://gitlab.com/gitlab-org/gitlab/-/issues/14004) in GitLab 14.1.2. +> - Support for restricting access to projects in the group [added](https://gitlab.com/gitlab-org/gitlab/-/issues/14004) in GitLab 14.1.2. You can prevent users with email addresses in specific domains from being added to a group and its projects. @@ -665,7 +665,7 @@ Any time you attempt to add a new user, the user's [primary email](../profile/in Only users with a [primary email](../profile/index.md#change-your-primary-email) that matches any of the configured email domain restrictions can be added to the group. -Some domains cannot be restricted. These are the most popular public email domains, such as: +The most popular public email domains cannot be restricted, such as: - `gmail.com`, `yahoo.com`, `aol.com`, `icloud.com` - `hotmail.com`, `hotmail.co.uk`, `hotmail.fr` @@ -722,7 +722,7 @@ To disable email notifications: You can prevent users from being added to a conversation and getting notified when anyone mentions a group in which those users are members. -Groups with disabled mentions are visualized accordingly in the autocompletion dropdown. +Groups with disabled mentions are visualized accordingly in the autocompletion dropdown list. This is particularly helpful for groups with a large number of users. diff --git a/lib/api/issues.rb b/lib/api/issues.rb index a5d6a6d7cf3..b5a559516f4 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -262,8 +262,6 @@ module API post ':id/issues' do Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/21140') - check_rate_limit!(:issues_create, scope: current_user) if Feature.disabled?("rate_limited_service_issues_create", user_project, default_enabled: :yaml) - authorize! :create_issue, user_project issue_params = declared_params(include_missing: false) diff --git a/lib/sidebars/groups/menus/ci_cd_menu.rb b/lib/sidebars/groups/menus/ci_cd_menu.rb index a1f98b918e6..c1d80458f49 100644 --- a/lib/sidebars/groups/menus/ci_cd_menu.rb +++ b/lib/sidebars/groups/menus/ci_cd_menu.rb @@ -29,7 +29,7 @@ module Sidebars ::Sidebars::MenuItem.new( title: _('Runners'), link: group_runners_path(context.group), - active_routes: { path: 'groups/runners#index' }, + active_routes: { controller: 'groups/runners' }, item_id: :runners ) end diff --git a/lib/sidebars/groups/menus/settings_menu.rb b/lib/sidebars/groups/menus/settings_menu.rb index 810b467ed2d..09226256476 100644 --- a/lib/sidebars/groups/menus/settings_menu.rb +++ b/lib/sidebars/groups/menus/settings_menu.rb @@ -89,10 +89,16 @@ module Sidebars end def ci_cd_menu_item + active_routes_path = if Feature.enabled?(:runner_list_group_view_vue_ui, context.group, default_enabled: :yaml) + 'ci_cd#show' + else + %w[ci_cd#show groups/runners#show groups/runners#edit] + end + ::Sidebars::MenuItem.new( title: _('CI/CD'), link: group_settings_ci_cd_path(context.group), - active_routes: { path: %w[ci_cd#show groups/runners#show groups/runners#edit] }, + active_routes: { path: active_routes_path }, item_id: :ci_cd ) end diff --git a/spec/frontend/ide/components/new_dropdown/modal_spec.js b/spec/frontend/ide/components/new_dropdown/modal_spec.js index 8134248bbf4..e8635444801 100644 --- a/spec/frontend/ide/components/new_dropdown/modal_spec.js +++ b/spec/frontend/ide/components/new_dropdown/modal_spec.js @@ -38,7 +38,7 @@ describe('new file modal component', () => { }); it(`sets button label as ${entryType}`, () => { - expect(document.querySelector('.btn-success').textContent.trim()).toBe(btnTitle); + expect(document.querySelector('.btn-confirm').textContent.trim()).toBe(btnTitle); }); it(`sets form label as ${entryType}`, () => { @@ -77,7 +77,7 @@ describe('new file modal component', () => { await nextTick(); expect(document.querySelector('.modal-title').textContent.trim()).toBe(modalTitle); - expect(document.querySelector('.btn-success').textContent.trim()).toBe(btnTitle); + expect(document.querySelector('.btn-confirm').textContent.trim()).toBe(btnTitle); }, ); diff --git a/spec/frontend_integration/ide/helpers/ide_helper.js b/spec/frontend_integration/ide/helpers/ide_helper.js index 54a522324f5..00ce39a5598 100644 --- a/spec/frontend_integration/ide/helpers/ide_helper.js +++ b/spec/frontend_integration/ide/helpers/ide_helper.js @@ -106,7 +106,7 @@ const fillFileNameModal = async (value, submitText = 'Create file') => { const nameField = await findByTestId(modal, 'file-name-field'); fireEvent.input(nameField, { target: { value } }); - const createButton = getByText(modal, submitText, { selector: 'button' }); + const createButton = getByText(modal, submitText, { selector: 'button > span' }); createButton.click(); }; diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb index 10098a66ae9..75538baf07f 100644 --- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -148,34 +148,11 @@ RSpec.describe Gitlab::Email::Handler::CreateIssueHandler do end end - context 'rate limiting' do - let(:rate_limited_service_feature_enabled) { nil } + it 'raises a RateLimitedService::RateLimitedError' do + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) - before do - stub_feature_flags(rate_limited_service_issues_create: rate_limited_service_feature_enabled) - end - - context 'when :rate_limited_service Feature is disabled' do - let(:rate_limited_service_feature_enabled) { false } - - it 'does not attempt to throttle' do - expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) - - setup_attachment - receiver.execute - end - end - - context 'when :rate_limited_service Feature is enabled' do - let(:rate_limited_service_feature_enabled) { true } - - it 'raises a RateLimitedService::RateLimitedError' do - allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) - - setup_attachment - expect { receiver.execute }.to raise_error(RateLimitedService::RateLimitedError, _('This endpoint has been requested too many times. Try again later.')) - end - end + setup_attachment + expect { receiver.execute }.to raise_error(RateLimitedService::RateLimitedError, _('This endpoint has been requested too many times. Try again later.')) end end diff --git a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb index 7c34fb1a926..8d008986464 100644 --- a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb @@ -382,7 +382,6 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do subject { 2.times { receiver.execute } } before do - stub_feature_flags(rate_limited_service_issues_create: true) stub_application_setting(issues_create_limit: 1) end diff --git a/spec/lib/sidebars/groups/menus/settings_menu_spec.rb b/spec/lib/sidebars/groups/menus/settings_menu_spec.rb index 252da8ea699..71b696516b6 100644 --- a/spec/lib/sidebars/groups/menus/settings_menu_spec.rb +++ b/spec/lib/sidebars/groups/menus/settings_menu_spec.rb @@ -72,6 +72,18 @@ RSpec.describe Sidebars::Groups::Menus::SettingsMenu do let(:item_id) { :ci_cd } it_behaves_like 'access rights checks' + + describe 'when runner list group view is disabled' do + before do + stub_feature_flags(runner_list_group_view_vue_ui: false) + end + + it_behaves_like 'access rights checks' + + it 'has group runners as active_routes' do + expect(subject.active_routes[:path]).to match_array %w[ci_cd#show groups/runners#show groups/runners#edit] + end + end end describe 'Applications menu' do diff --git a/spec/services/concerns/rate_limited_service_spec.rb b/spec/services/concerns/rate_limited_service_spec.rb index 97f5ca53c0d..04007e8e75a 100644 --- a/spec/services/concerns/rate_limited_service_spec.rb +++ b/spec/services/concerns/rate_limited_service_spec.rb @@ -36,79 +36,28 @@ RSpec.describe RateLimitedService do subject { described_class::RateLimiterScopedAndKeyed.new(key: key, opts: opts, rate_limiter: rate_limiter) } describe '#rate_limit!' do - let(:project_with_feature_enabled) { create(:project) } - let(:project_without_feature_enabled) { create(:project) } + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user) } - let(:project) { nil } - - let(:current_user) { create(:user) } let(:service) { instance_double(Issues::CreateService, project: project, current_user: current_user) } let(:evaluated_scope) { [project, current_user] } let(:evaluated_opts) { { scope: evaluated_scope, users_allowlist: %w[support-bot] } } - let(:rate_limited_service_issues_create_feature_enabled) { nil } - - before do - stub_feature_flags(rate_limited_service_issues_create: rate_limited_service_issues_create_feature_enabled) - end - shared_examples 'a service that does not attempt to throttle' do - it 'does not attempt to throttle' do - expect(rate_limiter).not_to receive(:throttled?) + context 'when rate limiting is not in effect' do + let(:throttled) { false } + it 'does not raise an exception' do expect(subject.rate_limit!(service)).to be_nil end end - shared_examples 'a service that does attempt to throttle' do + context 'when rate limiting is in effect' do before do - allow(rate_limiter).to receive(:throttled?).and_return(throttled) - end - - context 'when rate limiting is not in effect' do - let(:throttled) { false } - - it 'does not raise an exception' do - expect(subject.rate_limit!(service)).to be_nil - end - end - - context 'when rate limiting is in effect' do - let(:throttled) { true } - - it 'raises a RateLimitedError exception' do - expect { subject.rate_limit!(service) }.to raise_error(described_class::RateLimitedError, 'This endpoint has been requested too many times. Try again later.') - end + allow(rate_limiter).to receive(:throttled?).and_return(true) end - end - - context 'when :rate_limited_service_issues_create feature is globally disabled' do - let(:rate_limited_service_issues_create_feature_enabled) { false } - - it_behaves_like 'a service that does not attempt to throttle' - end - - context 'when :rate_limited_service_issues_create feature is globally enabled' do - let(:throttled) { nil } - let(:rate_limited_service_issues_create_feature_enabled) { true } - let(:project) { project_without_feature_enabled } - - it_behaves_like 'a service that does attempt to throttle' - end - - context 'when :rate_limited_service_issues_create feature is enabled for project_with_feature_enabled' do - let(:throttled) { nil } - let(:rate_limited_service_issues_create_feature_enabled) { project_with_feature_enabled } - - context 'for project_without_feature_enabled' do - let(:project) { project_without_feature_enabled } - - it_behaves_like 'a service that does not attempt to throttle' - end - - context 'for project_with_feature_enabled' do - let(:project) { project_with_feature_enabled } - it_behaves_like 'a service that does attempt to throttle' + it 'raises a RateLimitedError exception' do + expect { subject.rate_limit!(service) }.to raise_error(described_class::RateLimitedError, 'This endpoint has been requested too many times. Try again later.') end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index f4bb1f0877b..889d368c8d3 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -335,7 +335,6 @@ RSpec.describe Issues::CreateService do let(:user) { create(:user) } before do - stub_feature_flags(rate_limited_service_issues_create: true) stub_application_setting(issues_create_limit: 1) end |