diff options
26 files changed, 239 insertions, 73 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 6dd25430c9e..4f1b2705eae 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -c8e24f24ed55dc90cd7f3ad4272421b4fce368f7 +704ada1b240c283c1ff73547e2dd8b1f69887765 diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 3d3b7f31dfd..f6a6648fd79 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -14,6 +14,7 @@ class GraphqlController < ApplicationController # The query string of a standard IntrospectionQuery, used to compare incoming requests for caching CACHED_INTROSPECTION_QUERY_STRING = CachedIntrospectionQuery.query_string + INTROSPECTION_QUERY_OPERATION_NAME = 'IntrospectionQuery' # If a user is using their session to access GraphQL, we need to have session # storage, since the admin-mode check is session wide. @@ -58,7 +59,7 @@ class GraphqlController < ApplicationController urgency :low, [:execute] def execute - result = if Feature.enabled?(:cache_introspection_query) && params[:operationName] == 'IntrospectionQuery' + result = if Feature.enabled?(:cache_introspection_query) && introspection_query? execute_introspection_query else multiplex? ? execute_multiplex : execute_query @@ -294,9 +295,7 @@ class GraphqlController < ApplicationController end def introspection_query_can_use_cache? - graphql_query = GraphQL::Query.new(GitlabSchema, query: query, variables: build_variables(params[:variables])) - - CACHED_INTROSPECTION_QUERY_STRING == graphql_query.query_string.squish + CACHED_INTROSPECTION_QUERY_STRING == graphql_query_object.query_string.squish end def introspection_query_cache_key @@ -306,6 +305,15 @@ class GraphqlController < ApplicationController ['introspection-query-cache', Gitlab.revision, context[:remove_deprecated]] end + def introspection_query? + if params.key?(:operationName) + params[:operationName] == INTROSPECTION_QUERY_OPERATION_NAME + else + # If we don't provide operationName param, we infer it from the query + graphql_query_object.selected_operation_name == INTROSPECTION_QUERY_OPERATION_NAME + end + end + def log_introspection_query_cache_details(can_use_introspection_query_cache) Gitlab::AppLogger.info( message: "IntrospectionQueryCache", @@ -315,4 +323,9 @@ class GraphqlController < ApplicationController introspection_query_cache_key: introspection_query_cache_key.to_s ) end + + def graphql_query_object + @graphql_query_object ||= GraphQL::Query.new(GitlabSchema, query: query, + variables: build_variables(params[:variables])) + end end diff --git a/app/graphql/types/ci/runner_type.rb b/app/graphql/types/ci/runner_type.rb index 8e509cc8493..160ee0234fa 100644 --- a/app/graphql/types/ci/runner_type.rb +++ b/app/graphql/types/ci/runner_type.rb @@ -24,8 +24,9 @@ module Types field :admin_url, GraphQL::Types::String, null: true, description: 'Admin URL of the runner. Only available for administrators.' field :architecture_name, GraphQL::Types::String, null: true, - description: 'Architecture provided by the the runner.', - method: :architecture + deprecated: { reason: "Use field in `manager` object instead", milestone: '16.2' }, + description: 'Architecture provided by the the runner.', + method: :architecture field :contacted_at, Types::TimeType, null: true, description: 'Timestamp of last contact from this runner.', method: :contacted_at @@ -46,15 +47,17 @@ module Types description: 'URL of the registration page of the runner manager. Only available for the creator of the runner for a limited time during registration.', alpha: { milestone: '15.11' } field :executor_name, GraphQL::Types::String, null: true, - description: 'Executor last advertised by the runner.', - method: :executor_name + deprecated: { reason: "Use field in `manager` object instead", milestone: '16.2' }, + description: 'Executor last advertised by the runner.', + method: :executor_name field :groups, null: true, resolver: ::Resolvers::Ci::RunnerGroupsResolver, description: 'Groups the runner is associated with. For group runners only.' field :id, ::Types::GlobalIDType[::Ci::Runner], null: false, description: 'ID of the runner.' field :ip_address, GraphQL::Types::String, null: true, - description: 'IP address of the runner.' + deprecated: { reason: "Use field in `manager` object instead", milestone: '16.2' }, + description: 'IP address of the runner.' field :job_count, GraphQL::Types::Int, null: true, description: "Number of jobs processed by the runner (limited to #{JOB_COUNT_LIMIT}, plus one to indicate that more items exist)." field :job_execution_status, @@ -82,8 +85,9 @@ module Types field :paused, GraphQL::Types::Boolean, null: false, description: 'Indicates the runner is paused and not available to run jobs.' field :platform_name, GraphQL::Types::String, null: true, - description: 'Platform provided by the runner.', - method: :platform + deprecated: { reason: "Use field in `manager` object instead", milestone: '16.2' }, + description: 'Platform provided by the runner.', + method: :platform field :project_count, GraphQL::Types::Int, null: true, description: 'Number of projects that the runner is associated with.' field :projects, @@ -94,7 +98,8 @@ module Types field :register_admin_url, GraphQL::Types::String, null: true, description: 'URL of the temporary registration page of the runner. Only available before the runner is registered. Only available for administrators.' field :revision, GraphQL::Types::String, null: true, - description: 'Revision of the runner.' + deprecated: { reason: "Use field in `manager` object instead", milestone: '16.2' }, + description: 'Revision of the runner.' field :run_untagged, GraphQL::Types::Boolean, null: false, description: 'Indicates the runner is able to run untagged jobs.' field :runner_type, ::Types::Ci::RunnerTypeEnum, null: false, @@ -112,7 +117,8 @@ module Types description: 'Runner token expiration time.', method: :token_expires_at field :version, GraphQL::Types::String, null: true, - description: 'Version of the runner.' + deprecated: { reason: "Use field in `manager` object instead", milestone: '16.2' }, + description: 'Version of the runner.' markdown_field :maintenance_note_html, null: true diff --git a/app/services/groups/participants_service.rb b/app/services/groups/participants_service.rb index 1de2b3c5a2e..b6faf3fd9a5 100644 --- a/app/services/groups/participants_service.rb +++ b/app/services/groups/participants_service.rb @@ -17,7 +17,11 @@ module Groups render_participants_as_hash(participants.uniq) end + private + def all_members + return [] if Feature.enabled?(:disable_all_mention) + count = group_members.count [{ username: "all", name: "All Group Members", count: count }] end diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index 8c807e0016b..44cd6e9926f 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -30,6 +30,8 @@ module Projects end def all_members + return [] if Feature.enabled?(:disable_all_mention) + [{ username: "all", name: "All Project and Group Members", count: project_members.count }] end diff --git a/config/feature_flags/development/job_webhook_retries_count.yml b/config/feature_flags/development/job_webhook_retries_count.yml deleted file mode 100644 index 96b33695440..00000000000 --- a/config/feature_flags/development/job_webhook_retries_count.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: job_webhook_retries_count -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/101618 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/382046 -milestone: '15.6' -type: development -group: group::delivery -default_enabled: false diff --git a/config/feature_flags/ops/disable_keep_around_refs.yml b/config/feature_flags/ops/disable_keep_around_refs.yml new file mode 100644 index 00000000000..2a894defd65 --- /dev/null +++ b/config/feature_flags/ops/disable_keep_around_refs.yml @@ -0,0 +1,8 @@ +--- +name: disable_keep_around_refs +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124123 +rollout_issue_url: +milestone: '16.1' +type: ops +group: group::gitaly +default_enabled: false diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index d9f6f10ea8a..a5c933d111c 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1295,6 +1295,24 @@ Input type: `AuditEventsStreamingInstanceHeadersCreateInput` | <a id="mutationauditeventsstreaminginstanceheaderscreateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | <a id="mutationauditeventsstreaminginstanceheaderscreateheader"></a>`header` | [`AuditEventsStreamingInstanceHeader`](#auditeventsstreaminginstanceheader) | Created header. | +### `Mutation.auditEventsStreamingInstanceHeadersDestroy` + +Input type: `AuditEventsStreamingInstanceHeadersDestroyInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationauditeventsstreaminginstanceheadersdestroyclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationauditeventsstreaminginstanceheadersdestroyheaderid"></a>`headerId` | [`AuditEventsStreamingInstanceHeaderID!`](#auditeventsstreaminginstanceheaderid) | Header to delete. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationauditeventsstreaminginstanceheadersdestroyclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationauditeventsstreaminginstanceheadersdestroyerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + ### `Mutation.auditEventsStreamingInstanceHeadersUpdate` Input type: `AuditEventsStreamingInstanceHeadersUpdateInput` @@ -13085,7 +13103,7 @@ CI/CD variables for a project. | <a id="cirunneraccesslevel"></a>`accessLevel` | [`CiRunnerAccessLevel!`](#cirunneraccesslevel) | Access level of the runner. | | <a id="cirunneractive"></a>`active` **{warning-solid}** | [`Boolean!`](#boolean) | **Deprecated** in 14.8. Use paused. | | <a id="cirunneradminurl"></a>`adminUrl` | [`String`](#string) | Admin URL of the runner. Only available for administrators. | -| <a id="cirunnerarchitecturename"></a>`architectureName` | [`String`](#string) | Architecture provided by the the runner. | +| <a id="cirunnerarchitecturename"></a>`architectureName` **{warning-solid}** | [`String`](#string) | **Deprecated** in 16.2. Use field in `manager` object instead. | | <a id="cirunnercontactedat"></a>`contactedAt` | [`Time`](#time) | Timestamp of last contact from this runner. | | <a id="cirunnercreatedat"></a>`createdAt` | [`Time`](#time) | Timestamp of creation of this runner. | | <a id="cirunnercreatedby"></a>`createdBy` | [`UserCore`](#usercore) | User that created this runner. | @@ -13093,10 +13111,10 @@ CI/CD variables for a project. | <a id="cirunnereditadminurl"></a>`editAdminUrl` | [`String`](#string) | Admin form URL of the runner. Only available for administrators. | | <a id="cirunnerephemeralauthenticationtoken"></a>`ephemeralAuthenticationToken` **{warning-solid}** | [`String`](#string) | **Introduced** in 15.9. This feature is an Experiment. It can be changed or removed at any time. Ephemeral authentication token used for runner manager registration. Only available for the creator of the runner for a limited time during registration. | | <a id="cirunnerephemeralregisterurl"></a>`ephemeralRegisterUrl` **{warning-solid}** | [`String`](#string) | **Introduced** in 15.11. This feature is an Experiment. It can be changed or removed at any time. URL of the registration page of the runner manager. Only available for the creator of the runner for a limited time during registration. | -| <a id="cirunnerexecutorname"></a>`executorName` | [`String`](#string) | Executor last advertised by the runner. | +| <a id="cirunnerexecutorname"></a>`executorName` **{warning-solid}** | [`String`](#string) | **Deprecated** in 16.2. Use field in `manager` object instead. | | <a id="cirunnergroups"></a>`groups` | [`GroupConnection`](#groupconnection) | Groups the runner is associated with. For group runners only. (see [Connections](#connections)) | | <a id="cirunnerid"></a>`id` | [`CiRunnerID!`](#cirunnerid) | ID of the runner. | -| <a id="cirunneripaddress"></a>`ipAddress` | [`String`](#string) | IP address of the runner. | +| <a id="cirunneripaddress"></a>`ipAddress` **{warning-solid}** | [`String`](#string) | **Deprecated** in 16.2. Use field in `manager` object instead. | | <a id="cirunnerjobcount"></a>`jobCount` | [`Int`](#int) | Number of jobs processed by the runner (limited to 1000, plus one to indicate that more items exist). | | <a id="cirunnerjobexecutionstatus"></a>`jobExecutionStatus` **{warning-solid}** | [`CiRunnerJobExecutionStatus`](#cirunnerjobexecutionstatus) | **Introduced** in 15.7. This feature is an Experiment. It can be changed or removed at any time. Job execution status of the runner. | | <a id="cirunnerlocked"></a>`locked` | [`Boolean`](#boolean) | Indicates the runner is locked. | @@ -13106,12 +13124,12 @@ CI/CD variables for a project. | <a id="cirunnermaximumtimeout"></a>`maximumTimeout` | [`Int`](#int) | Maximum timeout (in seconds) for jobs processed by the runner. | | <a id="cirunnerownerproject"></a>`ownerProject` | [`Project`](#project) | Project that owns the runner. For project runners only. | | <a id="cirunnerpaused"></a>`paused` | [`Boolean!`](#boolean) | Indicates the runner is paused and not available to run jobs. | -| <a id="cirunnerplatformname"></a>`platformName` | [`String`](#string) | Platform provided by the runner. | +| <a id="cirunnerplatformname"></a>`platformName` **{warning-solid}** | [`String`](#string) | **Deprecated** in 16.2. Use field in `manager` object instead. | | <a id="cirunnerprivateprojectsminutescostfactor"></a>`privateProjectsMinutesCostFactor` | [`Float`](#float) | Private projects' "compute cost factor" associated with the runner (GitLab.com only). | | <a id="cirunnerprojectcount"></a>`projectCount` | [`Int`](#int) | Number of projects that the runner is associated with. | | <a id="cirunnerpublicprojectsminutescostfactor"></a>`publicProjectsMinutesCostFactor` | [`Float`](#float) | Public projects' "compute cost factor" associated with the runner (GitLab.com only). | | <a id="cirunnerregisteradminurl"></a>`registerAdminUrl` | [`String`](#string) | URL of the temporary registration page of the runner. Only available before the runner is registered. Only available for administrators. | -| <a id="cirunnerrevision"></a>`revision` | [`String`](#string) | Revision of the runner. | +| <a id="cirunnerrevision"></a>`revision` **{warning-solid}** | [`String`](#string) | **Deprecated** in 16.2. Use field in `manager` object instead. | | <a id="cirunnerrununtagged"></a>`runUntagged` | [`Boolean!`](#boolean) | Indicates the runner is able to run untagged jobs. | | <a id="cirunnerrunnertype"></a>`runnerType` | [`CiRunnerType!`](#cirunnertype) | Type of the runner. | | <a id="cirunnershortsha"></a>`shortSha` | [`String`](#string) | First eight characters of the runner's token used to authenticate new job requests. Used as the runner's unique ID. | @@ -13119,7 +13137,7 @@ CI/CD variables for a project. | <a id="cirunnertokenexpiresat"></a>`tokenExpiresAt` | [`Time`](#time) | Runner token expiration time. | | <a id="cirunnerupgradestatus"></a>`upgradeStatus` **{warning-solid}** | [`CiRunnerUpgradeStatus`](#cirunnerupgradestatus) | **Introduced** in 14.10. This feature is an Experiment. It can be changed or removed at any time. Availability of upgrades for the runner. | | <a id="cirunneruserpermissions"></a>`userPermissions` | [`RunnerPermissions!`](#runnerpermissions) | Permissions for the current user on the resource. | -| <a id="cirunnerversion"></a>`version` | [`String`](#string) | Version of the runner. | +| <a id="cirunnerversion"></a>`version` **{warning-solid}** | [`String`](#string) | **Deprecated** in 16.2. Use field in `manager` object instead. | #### Fields with arguments diff --git a/doc/api/rest/deprecations.md b/doc/api/rest/deprecations.md index 20fa999516e..80480e3f88f 100644 --- a/doc/api/rest/deprecations.md +++ b/doc/api/rest/deprecations.md @@ -110,3 +110,10 @@ A runner's status will only relate to runner contact status, such as: When checking if a runner is `paused`, API users are advised to check the boolean attribute `paused` to be `true` instead. When checking if a runner is `active`, check if `paused` is `false`. + +## Runner will not return `ip_address` + +Breaking change. [Related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/415159). + +In GitLab 17.0, the [Runners API](../runners.md) will return `""` in place of `ip_address` for runners. +In v5 of the REST API, the field will be removed. diff --git a/doc/development/audit_event_guide/index.md b/doc/development/audit_event_guide/index.md index c7c723d1686..c49d3a243c0 100644 --- a/doc/development/audit_event_guide/index.md +++ b/doc/development/audit_event_guide/index.md @@ -32,7 +32,7 @@ To instrument an audit event, the following attributes should be provided: | Attribute | Type | Required? | Description | |:-------------|:---------------------|:----------|:------------------------------------------------------------------| | `name` | String | false | Action name to be audited. Represents the [type of the event](#event-type-definitions). Used for error tracking | -| `author` | User | true | User who authors the change | +| `author` | User | true | User who authors the change. Can be an [internal user](../internal_users.md). For example, [inactive project deletion](../../administration/inactive_project_deletion.md) audit events are authored by `GitLab-Admin-Bot`. | | `scope` | User, Project, Group | true | Scope which the audit event belongs to | | `target` | Object | true | Target object being audited | | `message` | String | true | Message describing the action ([not translated](#i18n-and-the-audit-event-message-attribute)) | diff --git a/doc/user/project/integrations/webhook_events.md b/doc/user/project/integrations/webhook_events.md index dfff537e4a1..0c0c1e1e367 100644 --- a/doc/user/project/integrations/webhook_events.md +++ b/doc/user/project/integrations/webhook_events.md @@ -1468,13 +1468,8 @@ Payload example: ### Number of retries -> `retries_count` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/382046) in GitLab 15.6 [with a flag](../../../administration/feature_flags.md) named `job_webhook_retries_count`. Disabled by default. - -FLAG: -On self-managed GitLab, by default this feature is not available. To make it available, -ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named -`job_webhook_retries_count`. -On GitLab.com, this feature is not available. +> - `retries_count` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/382046) in GitLab 15.6 [with a flag](../../../administration/feature_flags.md) named `job_webhook_retries_count`. Disabled by default. +> - `retries_count` [enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/382046) in GitLab 16.1. `retries_count` is an integer that indicates if the job is a retry. `0` means that the job has not been retried. `1` means that it's the first retry. diff --git a/lib/api/entities/ci/runner.rb b/lib/api/entities/ci/runner.rb index 9361709b6ed..441e1dc1117 100644 --- a/lib/api/entities/ci/runner.rb +++ b/lib/api/entities/ci/runner.rb @@ -6,6 +6,7 @@ module API class Runner < Grape::Entity expose :id, documentation: { type: 'integer', example: 8 } expose :description, documentation: { type: 'string', example: 'test-1-20150125' } + # TODO: return null in 17.0 and remove in v5 https://gitlab.com/gitlab-org/gitlab/-/issues/415159 expose :ip_address, documentation: { type: 'string', example: '127.0.0.1' } # TODO Remove in v5 in favor of `paused` for REST calls, see https://gitlab.com/gitlab-org/gitlab/-/issues/375709 expose :active, documentation: { type: 'boolean', example: true } diff --git a/lib/gitlab/data_builder/build.rb b/lib/gitlab/data_builder/build.rb index 8fec5cf3303..e1e9e4720bb 100644 --- a/lib/gitlab/data_builder/build.rb +++ b/lib/gitlab/data_builder/build.rb @@ -12,13 +12,14 @@ module Gitlab author_url = build_author_url(build.commit, commit) - data = { + { object_kind: 'build', ref: build.ref, tag: build.tag, before_sha: build.before_sha, sha: build.sha, + retries_count: build.retries_count, # TODO: should this be not prefixed with build_? # Leaving this way to have backward compatibility @@ -69,10 +70,6 @@ module Gitlab environment: build_environment(build) } - - data[:retries_count] = build.retries_count if Feature.enabled?(:job_webhook_retries_count, project) - - data end private diff --git a/lib/gitlab/git/keep_around.rb b/lib/gitlab/git/keep_around.rb index 38f0e47c4c7..5835a7001af 100644 --- a/lib/gitlab/git/keep_around.rb +++ b/lib/gitlab/git/keep_around.rb @@ -19,6 +19,8 @@ module Gitlab end def execute(shas) + return if disabled? + shas.uniq.each do |sha| next unless sha.present? && commit_by(oid: sha) @@ -32,6 +34,8 @@ module Gitlab end def kept_around?(sha) + return true if disabled? + ref_exists?(keep_around_ref_name(sha)) end @@ -40,6 +44,11 @@ module Gitlab private + def disabled? + Feature.enabled?(:disable_keep_around_refs, @repository, type: :ops) || + (@repository.project && Feature.enabled?(:disable_keep_around_refs, @repository.project, type: :ops)) + end + def keep_around_ref_name(sha) "refs/#{::Repository::REF_KEEP_AROUND}/#{sha}" end diff --git a/package.json b/package.json index b07e86aba2e..5d583d9fedc 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "@gitlab/svgs": "3.53.0", "@gitlab/ui": "64.13.0", "@gitlab/visual-review-tools": "1.7.3", - "@gitlab/web-ide": "0.0.1-dev-20230614124516", + "@gitlab/web-ide": "0.0.1-dev-20230620122149", "@mattiasbuelens/web-streams-adapter": "^0.1.0", "@popperjs/core": "^2.11.2", "@rails/actioncable": "6.1.4-7", diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index b4a7e41ccd2..da1676138ec 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -431,6 +431,13 @@ RSpec.describe GraphqlController, feature_category: :integrations do context 'when querying an IntrospectionQuery', :use_clean_rails_memory_store_caching do let_it_be(:query) { File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql')) } + it 'caches IntrospectionQuery even when operationName is not given' do + expect(GitlabSchema).to receive(:execute).exactly(:once) + + post :execute, params: { query: query } + post :execute, params: { query: query } + end + it 'caches the IntrospectionQuery' do expect(GitlabSchema).to receive(:execute).exactly(:once) diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb index 70178083e71..d0bfbeae78f 100644 --- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb +++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb @@ -131,6 +131,10 @@ RSpec.describe Projects::AutocompleteSourcesController do end shared_examples 'all members are returned' do + before do + stub_feature_flags(disable_all_mention: false) + end + it 'returns an array of member object' do get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } @@ -155,6 +159,19 @@ RSpec.describe Projects::AutocompleteSourcesController do name: invited_private_member.name, avatar_url: invited_private_member.avatar_url) end + + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + it 'does not return the all mention user' do + get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } + + expect(json_response).not_to include(a_hash_including( + { username: 'all', name: 'All Project and Group Members' })) + end + end end context 'with issue' do @@ -180,6 +197,10 @@ RSpec.describe Projects::AutocompleteSourcesController do end shared_examples 'only public members are returned for public project' do + before do + stub_feature_flags(disable_all_mention: false) + end + it 'only returns public members' do get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } @@ -193,6 +214,19 @@ RSpec.describe Projects::AutocompleteSourcesController do name: user.name, avatar_url: user.avatar_url) end + + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + it 'does not return the all mention user' do + get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } + + expect(json_response).not_to include(a_hash_including( + { username: 'all', name: 'All Project and Group Members' })) + end + end end context 'with issue' do diff --git a/spec/frontend/content_editor/remark_markdown_processing_spec.js b/spec/frontend/content_editor/remark_markdown_processing_spec.js index 927a7d59899..d4a02ab2202 100644 --- a/spec/frontend/content_editor/remark_markdown_processing_spec.js +++ b/spec/frontend/content_editor/remark_markdown_processing_spec.js @@ -1421,6 +1421,7 @@ body { }; }; + // NOTE: unicode \u001 and \u003 cannot be used in test names because they cause test report XML parsing errors it.each` desc | urlInput | urlOutput ${'protocol-based JS injection: simple, no spaces'} | ${protocolBasedInjectionSimpleNoSpaces} | ${null} @@ -1439,7 +1440,7 @@ body { ${'protocol-based JS injection: preceding colon'} | ${":javascript:alert('XSS');"} | ${":javascript:alert('XSS');"} ${'protocol-based JS injection: null char'} | ${"java\0script:alert('XSS')"} | ${"java�script:alert('XSS')"} ${'protocol-based JS injection: invalid URL char'} | ${"java\\script:alert('XSS')"} | ${"java\\script:alert('XSS')"} - `('sanitize $desc:\n\tURL "$urlInput" becomes "$urlOutput"', ({ urlInput, urlOutput }) => { + `('sanitize $desc becomes "$urlOutput"', ({ urlInput, urlOutput }) => { const exampleFactories = [docWithImageFactory, docWithLinkFactory]; exampleFactories.forEach(async (exampleFactory) => { diff --git a/spec/frontend/lib/utils/ref_validator_spec.js b/spec/frontend/lib/utils/ref_validator_spec.js index 7185ebf0a24..97896d74dff 100644 --- a/spec/frontend/lib/utils/ref_validator_spec.js +++ b/spec/frontend/lib/utils/ref_validator_spec.js @@ -65,9 +65,6 @@ describe('~/lib/utils/ref_validator', () => { ['foo.123.', validationMessages.DisallowedSequencePostfixesValidationMessage], ['foo/', validationMessages.DisallowedPostfixesValidationMessage], - - ['control-character\x7f', validationMessages.ControlCharactersValidationMessage], - ['control-character\x15', validationMessages.ControlCharactersValidationMessage], ])('tag with name "%s"', (tagName, validationMessage) => { it(`should be invalid with validation message "${validationMessage}"`, () => { const result = validateTag(tagName); @@ -75,5 +72,25 @@ describe('~/lib/utils/ref_validator', () => { expect(result.validationErrors).toContain(validationMessage); }); }); + + // NOTE: control characters cannot be used in test names because they cause test report XML parsing errors + describe.each([ + [ + 'control-character x7f', + 'control-character\x7f', + validationMessages.ControlCharactersValidationMessage, + ], + [ + 'control-character x15', + 'control-character\x15', + validationMessages.ControlCharactersValidationMessage, + ], + ])('tag with name "%s"', (_, tagName, validationMessage) => { + it(`should be invalid with validation message "${validationMessage}"`, () => { + const result = validateTag(tagName); + expect(result.isValid).toBe(false); + expect(result.validationErrors).toContain(validationMessage); + }); + }); }); }); diff --git a/spec/lib/gitlab/data_builder/build_spec.rb b/spec/lib/gitlab/data_builder/build_spec.rb index 92fef93bddb..7cd0af0dcec 100644 --- a/spec/lib/gitlab/data_builder/build_spec.rb +++ b/spec/lib/gitlab/data_builder/build_spec.rb @@ -66,25 +66,6 @@ RSpec.describe Gitlab::DataBuilder::Build, feature_category: :integrations do expect(control.count).to eq(14) end - context 'when job_webhook_retries_count feature flag is disabled' do - before do - stub_feature_flags(job_webhook_retries_count: false) - end - - it { expect(data).not_to have_key(:retries_count) } - - it 'does not exceed number of expected queries' do - ci_build # Make sure the Ci::Build model is created before recording. - - control = ActiveRecord::QueryRecorder.new(skip_cached: false) do - b = Ci::Build.find(ci_build.id) - described_class.build(b) # Don't use ci_build variable here since it has all associations loaded into memory - end - - expect(control.count).to eq(13) - end - end - context 'commit author_url' do context 'when no commit present' do let(:build) { build(:ci_build) } diff --git a/spec/lib/gitlab/git/keep_around_spec.rb b/spec/lib/gitlab/git/keep_around_spec.rb index d6359d55646..65bed3f2ae6 100644 --- a/spec/lib/gitlab/git/keep_around_spec.rb +++ b/spec/lib/gitlab/git/keep_around_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Gitlab::Git::KeepAround do let(:repository) { create(:project, :repository).repository } let(:service) { described_class.new(repository) } + let(:keep_around_ref_name) { "refs/#{::Repository::REF_KEEP_AROUND}/#{sample_commit.id}" } it "does not fail if we attempt to reference bad commit" do expect(service.kept_around?('abc1234')).to be_falsey @@ -16,6 +17,7 @@ RSpec.describe Gitlab::Git::KeepAround do service.execute([sample_commit.id]) expect(service.kept_around?(sample_commit.id)).to be_truthy + expect(repository.list_refs([keep_around_ref_name])).not_to be_empty end it "does not fail if writting the ref fails" do @@ -45,4 +47,17 @@ RSpec.describe Gitlab::Git::KeepAround do expect(service.kept_around?(another_sample_commit.id)).to be_truthy end end + + context 'when disable_keep_around_refs feature flag is enabled' do + before do + stub_feature_flags(disable_keep_around_refs: true) + end + + it 'does not create keep-around refs' do + service.execute([sample_commit.id]) + + expect(service.kept_around?(sample_commit.id)).to be_truthy + expect(repository.list_refs([keep_around_ref_name])).to be_empty + end + end end diff --git a/spec/services/groups/participants_service_spec.rb b/spec/services/groups/participants_service_spec.rb index eee9cfce1b1..749b3e9e3fd 100644 --- a/spec/services/groups/participants_service_spec.rb +++ b/spec/services/groups/participants_service_spec.rb @@ -3,25 +3,43 @@ require 'spec_helper' RSpec.describe Groups::ParticipantsService, feature_category: :groups_and_projects do - describe '#group_members' do + describe '#execute' do let(:user) { create(:user) } let(:parent_group) { create(:group) } let(:group) { create(:group, parent: parent_group) } let(:subgroup) { create(:group, parent: group) } let(:subproject) { create(:project, group: subgroup) } - it 'returns all members in parent groups, sub-groups, and sub-projects' do + subject(:service_result) { described_class.new(group, user).execute(nil) } + + before do parent_group.add_developer(create(:user)) subgroup.add_developer(create(:user)) subproject.add_developer(create(:user)) - result = described_class.new(group, user).execute(nil) + stub_feature_flags(disable_all_mention: false) + end + + it 'includes `All Group Members`' do + expect(service_result).to include(a_hash_including({ username: "all", name: "All Group Members" })) + end + it 'returns all members in parent groups, sub-groups, and sub-projects' do expected_users = (group.self_and_hierarchy.flat_map(&:users) + subproject.users) .map { |user| user_to_autocompletable(user) } expect(expected_users.count).to eq(3) - expect(result).to include(*expected_users) + expect(service_result).to include(*expected_users) + end + + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + it 'does not include `All Group Members`' do + expect(service_result).not_to include(a_hash_including({ username: "all", name: "All Group Members" })) + end end end diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb index 2f090577805..04c43dff2dc 100644 --- a/spec/services/projects/participants_service_spec.rb +++ b/spec/services/projects/participants_service_spec.rb @@ -10,12 +10,18 @@ RSpec.describe Projects::ParticipantsService, feature_category: :groups_and_proj before_all do project.add_developer(user) + + stub_feature_flags(disable_all_mention: false) end def run_service described_class.new(project, user).execute(noteable) end + it 'includes `All Project and Group Members`' do + expect(run_service).to include(a_hash_including({ username: "all", name: "All Project and Group Members" })) + end + context 'N+1 checks' do before do run_service # warmup, runs table cache queries and create queries @@ -99,6 +105,16 @@ RSpec.describe Projects::ParticipantsService, feature_category: :groups_and_proj end end end + + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + it 'does not include `All Project and Group Members`' do + expect(run_service).not_to include(a_hash_including({ username: "all", name: "All Project and Group Members" })) + end + end end describe '#project_members' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a2afa3d0ca7..55ab22e053f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -312,6 +312,9 @@ RSpec.configure do |config| # most cases. We do test the email verification flow in the appropriate specs. stub_feature_flags(require_email_verification: false) + # Keep-around refs should only be turned off for specific projects/repositories. + stub_feature_flags(disable_keep_around_refs: false) + allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(enable_rugged) else unstub_all_feature_flags diff --git a/spec/support/shared_examples/features/content_editor_shared_examples.rb b/spec/support/shared_examples/features/content_editor_shared_examples.rb index f70288168d7..2454bb7afc6 100644 --- a/spec/support/shared_examples/features/content_editor_shared_examples.rb +++ b/spec/support/shared_examples/features/content_editor_shared_examples.rb @@ -506,6 +506,8 @@ RSpec.shared_examples 'edits content using the content editor' do |params = { wi switch_to_content_editor type_in_content_editor :enter + + stub_feature_flags(disable_all_mention: false) end if params[:with_expanded_references] @@ -545,6 +547,26 @@ RSpec.shared_examples 'edits content using the content editor' do |params = { wi expect(page).to have_text('@abc123') end + context 'when `disable_all_mention` is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + it 'shows suggestions for members with descriptions' do + type_in_content_editor '@a' + + expect(find(suggestions_dropdown)).to have_text('abc123') + expect(find(suggestions_dropdown)).not_to have_text('All Group Members') + + type_in_content_editor 'bc' + + send_keys [:arrow_down, :enter] + + expect(page).not_to have_css(suggestions_dropdown) + expect(page).to have_text('@abc123') + end + end + it 'shows suggestions for merge requests' do type_in_content_editor '!' diff --git a/yarn.lock b/yarn.lock index dfeab1e5b42..fb8c0b999a0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1146,10 +1146,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.7.3.tgz#9ea641146436da388ffbad25d7f2abe0df52c235" integrity sha512-NMV++7Ew1FSBDN1xiZaauU9tfeSfgDHcOLpn+8bGpP+O5orUPm2Eu66R5eC5gkjBPaXosNAxNWtriee+aFk4+g== -"@gitlab/web-ide@0.0.1-dev-20230614124516": - version "0.0.1-dev-20230614124516" - resolved "https://registry.yarnpkg.com/@gitlab/web-ide/-/web-ide-0.0.1-dev-20230614124516.tgz#87ef511112e954dc3614e28fdf635b140bab22b7" - integrity sha512-VRKiDzhRNIHsyKbln8YtQjVvsOkF+4N1OUqaldu2gYX4Nfw9g57sTwnnX44aYcHFgPVgo5a/ScRpnCiBl6Phxg== +"@gitlab/web-ide@0.0.1-dev-20230620122149": + version "0.0.1-dev-20230620122149" + resolved "https://registry.yarnpkg.com/@gitlab/web-ide/-/web-ide-0.0.1-dev-20230620122149.tgz#2e5c2c41a3df91227440c1a319001bd59eac8aac" + integrity sha512-uJFT0aUiOvPynN8/zp0VB2CVxiiKmzzTiZIJseKY8V1b2kKvW554lRVbNwES34Fo/ZXoCU65ZCmIitbbPgsZYQ== "@graphql-eslint/eslint-plugin@3.19.1": version "3.19.1" |