From f94cd1d0fb5c0b42ec12a8db02ec90227bb98879 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 23 Oct 2023 12:11:40 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .gitlab/CODEOWNERS | 1 + .../mutations/ci/catalog/resources/unpublish.rb | 30 +++ app/graphql/types/mutation_type.rb | 3 +- app/models/ci/catalog/resource.rb | 4 + app/models/ci/pipeline.rb | 5 - app/services/members/create_service.rb | 29 ++- .../development/invitations_member_role_id.yml | 8 + .../reduced_build_attributes_list_for_rules.yml | 8 - ...-6-file-type-variable-extension-deprecation.yml | 13 ++ ...929155123_migrate_disable_merge_trains_value.rb | 55 ++++++ db/schema_migrations/20230929155123 | 1 + doc/api/graphql/reference/index.md | 22 +++ doc/api/invitations.md | 1 + doc/ci/cloud_services/google_cloud/index.md | 4 + doc/ci/pipelines/merge_trains.md | 35 +--- .../incident_management/manage_incidents.md | 2 +- doc/update/deprecations.md | 18 ++ doc/user/group/epics/manage_epics.md | 2 +- doc/user/profile/account/delete_account.md | 10 +- .../project/merge_requests/reviews/data_usage.md | 2 +- .../repository/reducing_the_repo_size_using_git.md | 7 +- gems/gitlab-http/Gemfile.lock | 2 +- gems/gitlab-utils/Gemfile.lock | 2 +- gems/rspec_flaky/Gemfile.lock | 2 +- lib/api/invitations.rb | 12 ++ .../representation/pull_request_comment.rb | 4 + .../importers/pull_request_notes_importer.rb | 124 +++++++++++- lib/gitlab/ci/build/context/build.rb | 10 +- .../representation/pull_request_comment_spec.rb | 8 +- .../importers/issues_notes_importer_spec.rb | 2 - .../importers/pull_request_notes_importer_spec.rb | 214 +++++++++++++++++++-- spec/lib/gitlab/ci/build/context/build_spec.rb | 20 -- ...5123_migrate_disable_merge_trains_value_spec.rb | 83 ++++++++ spec/models/ci/catalog/resource_spec.rb | 25 ++- spec/models/ci/pipeline_spec.rb | 21 -- .../graphql/mutations/ci/catalog/unpublish_spec.rb | 52 +++++ 36 files changed, 707 insertions(+), 134 deletions(-) create mode 100644 app/graphql/mutations/ci/catalog/resources/unpublish.rb create mode 100644 config/feature_flags/development/invitations_member_role_id.yml delete mode 100644 config/feature_flags/development/reduced_build_attributes_list_for_rules.yml create mode 100644 data/deprecations/16-6-file-type-variable-extension-deprecation.yml create mode 100644 db/migrate/20230929155123_migrate_disable_merge_trains_value.rb create mode 100644 db/schema_migrations/20230929155123 create mode 100644 spec/migrations/20230929155123_migrate_disable_merge_trains_value_spec.rb create mode 100644 spec/requests/api/graphql/mutations/ci/catalog/unpublish_spec.rb diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 3353f82d9d6..e0dacb8cb90 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -1482,6 +1482,7 @@ ee/lib/ee/api/entities/project.rb [Manage::Foundations] @gitlab-org/manage/foundations/engineering /lib/sidebars/ /ee/lib/sidebars/ +/ee/lib/ee/sidebars/ [Global Search] @gitlab-org/search-team/migration-maintainers /ee/elastic/migrate/ diff --git a/app/graphql/mutations/ci/catalog/resources/unpublish.rb b/app/graphql/mutations/ci/catalog/resources/unpublish.rb new file mode 100644 index 00000000000..e45e9646147 --- /dev/null +++ b/app/graphql/mutations/ci/catalog/resources/unpublish.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Mutations + module Ci + module Catalog + module Resources + class Unpublish < BaseMutation + graphql_name 'CatalogResourceUnpublish' + + authorize :add_catalog_resource + + argument :id, ::Types::GlobalIDType[::Ci::Catalog::Resource], + required: true, + description: 'Global ID of the catalog resource to unpublish.' + + def resolve(id:) + catalog_resource = ::Gitlab::Graphql::Lazy.force(GitlabSchema.find_by_gid(id)) + authorize!(catalog_resource&.project) + + catalog_resource.unpublish! + + { + errors: [] + } + end + end + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 2e64c22fb7b..a2d1841cc0e 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -39,7 +39,6 @@ module Types mount_mutation Mutations::Boards::Lists::Update mount_mutation Mutations::Boards::Lists::Destroy mount_mutation Mutations::Branches::Create, calls_gitaly: true - mount_mutation Mutations::Ci::Catalog::Resources::Create, alpha: { milestone: '15.11' } mount_mutation Mutations::Clusters::Agents::Create mount_mutation Mutations::Clusters::Agents::Delete mount_mutation Mutations::Clusters::AgentTokens::Create @@ -137,6 +136,8 @@ module Types mount_mutation Mutations::ContainerExpirationPolicies::Update mount_mutation Mutations::ContainerRepositories::Destroy mount_mutation Mutations::ContainerRepositories::DestroyTags + mount_mutation Mutations::Ci::Catalog::Resources::Create, alpha: { milestone: '15.11' } + mount_mutation Mutations::Ci::Catalog::Resources::Unpublish, alpha: { milestone: '16.6' } mount_mutation Mutations::Ci::Pipeline::Cancel mount_mutation Mutations::Ci::Pipeline::Destroy mount_mutation Mutations::Ci::Pipeline::Retry diff --git a/app/models/ci/catalog/resource.rb b/app/models/ci/catalog/resource.rb index 8ffc0292a69..3fcf69983d2 100644 --- a/app/models/ci/catalog/resource.rb +++ b/app/models/ci/catalog/resource.rb @@ -32,6 +32,10 @@ module Ci def latest_version project.releases.latest end + + def unpublish! + update!(state: :draft) + end end end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 0a876d26cc9..5bf4e846304 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -1366,11 +1366,6 @@ module Ci merge_request.merge_request_diff_for(merge_request_diff_sha) end - def reduced_build_attributes_list_for_rules? - ::Feature.enabled?(:reduced_build_attributes_list_for_rules, project) - end - strong_memoize_attr :reduced_build_attributes_list_for_rules? - private def add_message(severity, content) diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 9cedc7ee3a5..20460d2c312 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -82,16 +82,33 @@ module Members end def add_members - @members = source.add_members( - invites, - params[:access_level], - expires_at: params[:expires_at], - current_user: current_user - ) + @members = if Feature.enabled?(:invitations_member_role_id, source) + creator_service.add_members( + source, invites, params[:access_level], **create_params + ) + else + source.add_members( + invites, + params[:access_level], + expires_at: params[:expires_at], + current_user: current_user + ) + end members.each { |member| process_result(member) } end + def creator_service + "Members::#{source.class.to_s.pluralize}::CreatorService".constantize + end + + def create_params + { + expires_at: params[:expires_at], + current_user: current_user + } + end + def process_result(member) existing_errors = member.errors.full_messages diff --git a/config/feature_flags/development/invitations_member_role_id.yml b/config/feature_flags/development/invitations_member_role_id.yml new file mode 100644 index 00000000000..ccb319e4e35 --- /dev/null +++ b/config/feature_flags/development/invitations_member_role_id.yml @@ -0,0 +1,8 @@ +--- +name: invitations_member_role_id +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134100 +rollout_issue_url: +milestone: '16.6' +type: development +group: group::authorization +default_enabled: false diff --git a/config/feature_flags/development/reduced_build_attributes_list_for_rules.yml b/config/feature_flags/development/reduced_build_attributes_list_for_rules.yml deleted file mode 100644 index 85170fb02ba..00000000000 --- a/config/feature_flags/development/reduced_build_attributes_list_for_rules.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: reduced_build_attributes_list_for_rules -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132654 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/426259 -milestone: '16.5' -type: development -group: group::pipeline execution -default_enabled: false diff --git a/data/deprecations/16-6-file-type-variable-extension-deprecation.yml b/data/deprecations/16-6-file-type-variable-extension-deprecation.yml new file mode 100644 index 00000000000..b8fc9620a88 --- /dev/null +++ b/data/deprecations/16-6-file-type-variable-extension-deprecation.yml @@ -0,0 +1,13 @@ +- title: "File type variable expansion fixed in downstream pipelines" + removal_milestone: "17.0" # (required) The milestone when this feature is planned to be removed + announcement_milestone: "16.6" # (required) The milestone when this feature was first announced as deprecated. + breaking_change: true # (required) Change to false if this is not a breaking change. + reporter: jocelynjane # (required) GitLab username of the person reporting the change + stage: Verify # (required) String value of the stage that the feature was created in. e.g., Growth + issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/419445 # (required) Link to the deprecation issue in GitLab + body: | # (required) Do not modify this line, instead modify the lines below. + Previously, if you tried to reference a [file type CI/CD variable](https://docs.gitlab.com/ee/ci/variables/#use-file-type-cicd-variables) in another CI/CD variable, the CI/CD variable would expand to contain the contents of the file. This behavior was incorrect because it did not comply with typical shell variable expansion rules. The CI/CD variable reference should expand to only contain the path to the file, not the contents of the file itself. This was [fixed for most use cases in GitLab 15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/29407). Unfortunately, passing CI/CD variables to downstream pipelines was an edge case not yet fixed, but which will now be fixed in GitLab 17.0. + + With this change, a variable configured in the `.gitlab-ci.yml` file can reference a file variable and be passed to a downstream pipeline, and the file variable will be passed to the downstream pipeline as well. The downstream pipeline will expand the variable reference to the file path, not the file contents. + + This breaking change could disrupt user workflows that depend on expanding a file variable in a downstream pipeline. diff --git a/db/migrate/20230929155123_migrate_disable_merge_trains_value.rb b/db/migrate/20230929155123_migrate_disable_merge_trains_value.rb new file mode 100644 index 00000000000..59eadd07733 --- /dev/null +++ b/db/migrate/20230929155123_migrate_disable_merge_trains_value.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +class MigrateDisableMergeTrainsValue < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + class Gate < MigrationRecord + self.table_name = 'feature_gates' + end + + UPDATE_QUERY = <<-SQL + UPDATE project_ci_cd_settings SET merge_trains_enabled = :merge_trains_enabled + WHERE project_id IN (:project_ids) + SQL + + def update_merge_trains_enabled(project_ids, merge_trains_enabled) + ApplicationRecord.connection.execute( + ApplicationRecord.sanitize_sql([ + UPDATE_QUERY, + { + project_ids: project_ids, + merge_trains_enabled: merge_trains_enabled.to_s.upcase + } + ]) + ) + end + + def get_project_ids + project_ids = Gate.where(feature_key: :disable_merge_trains, key: 'actors').pluck('value') + + project_ids.filter_map do |project_id| + # ensure actor is a project formatted correctly + match = project_id.match(/Project:[0-9]+/)[0] + # Extract the project id if there is an actor + match ? project_id.gsub('Project:', '').to_i : nil + end + end + + def up + project_ids = get_project_ids + + return unless project_ids + + update_merge_trains_enabled(project_ids, false) + end + + def down + project_ids = get_project_ids + + return unless project_ids + + update_merge_trains_enabled(project_ids, true) + end +end diff --git a/db/schema_migrations/20230929155123 b/db/schema_migrations/20230929155123 new file mode 100644 index 00000000000..e2332c208aa --- /dev/null +++ b/db/schema_migrations/20230929155123 @@ -0,0 +1 @@ +91650c6c2dd7066036be3b276331361e7e514ec65a048aebabd43110e860e8ff \ No newline at end of file diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index dea79bc1d25..a4688faf6ac 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1812,6 +1812,28 @@ Input type: `BulkRunnerDeleteInput` | `deletedIds` | [`[CiRunnerID!]`](#cirunnerid) | IDs of records effectively deleted. Only present if operation was performed synchronously. | | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +### `Mutation.catalogResourceUnpublish` + +WARNING: +**Introduced** in 16.6. +This feature is an Experiment. It can be changed or removed at any time. + +Input type: `CatalogResourceUnpublishInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `id` | [`CiCatalogResourceID!`](#cicatalogresourceid) | Global ID of the catalog resource to unpublish. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + ### `Mutation.catalogResourcesCreate` WARNING: diff --git a/doc/api/invitations.md b/doc/api/invitations.md index e3619932fea..9c209f04d65 100644 --- a/doc/api/invitations.md +++ b/doc/api/invitations.md @@ -43,6 +43,7 @@ POST /projects/:id/invitations | `access_level` | integer | yes | A valid access level | | `expires_at` | string | no | A date string in the format YEAR-MONTH-DAY | | `invite_source` | string | no | The source of the invitation that starts the member creation process. See [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/327120). | +| `member_role_id` **(ULTIMATE ALL)** | integer | no | Assigns the new member to the provided custom role. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134100) in GitLab 16.6 under a feature flag `invitations_member_role_id`. | ```shell curl --request POST --header "PRIVATE-TOKEN: " \ diff --git a/doc/ci/cloud_services/google_cloud/index.md b/doc/ci/cloud_services/google_cloud/index.md index a733f3d59cb..fd8aca7045c 100644 --- a/doc/ci/cloud_services/google_cloud/index.md +++ b/doc/ci/cloud_services/google_cloud/index.md @@ -22,6 +22,10 @@ This tutorial assumes you have a Google Cloud account and a Google Cloud project Your account must have at least the **Workload Identity Pool Admin** permission on the Google Cloud project. +NOTE: +If you would prefer to use a Terraform module and a CI/CD template instead of this tutorial, +see [How OIDC can simplify authentication of GitLab CI/CD pipelines with Google Cloud](https://about.gitlab.com/blog/2023/06/28/introduction-of-oidc-modules-for-integration-between-google-cloud-and-gitlab-ci/). + To complete this tutorial: 1. [Create the Google Cloud Workload Identity Pool](#create-the-google-cloud-workload-identity-pool). diff --git a/doc/ci/pipelines/merge_trains.md b/doc/ci/pipelines/merge_trains.md index b7f081886a6..a54087262e7 100644 --- a/doc/ci/pipelines/merge_trains.md +++ b/doc/ci/pipelines/merge_trains.md @@ -90,6 +90,8 @@ are cancelled. ## Enable merge trains +> `disable_merge_trains` feature flag [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/282477) in GitLab 16.5. + Prerequisites: - You must have the Maintainer role. @@ -97,17 +99,15 @@ Prerequisites: - Your pipeline must be [configured to use merge request pipelines](merge_request_pipelines.md#prerequisites). Otherwise your merge requests may become stuck in an unresolved state or your pipelines might be dropped. +- You must have [merged results pipelines enabled](merged_results_pipelines.md#enable-merged-results-pipelines). To enable merge trains: 1. On the left sidebar, select **Search or go to** and find your project. 1. Select **Settings > Merge requests**. 1. In the **Merge method** section, verify that **Merge commit** is selected. -1. In the **Merge options** section: - - In GitLab 13.6 and later, select **Enable merged results pipelines** and **Enable merge trains**. - - In GitLab 13.5 and earlier, select **Enable merge trains and pipelines for merged results**. - Additionally, [a feature flag](#disable-merge-trains-in-gitlab-135-and-earlier) - must be set correctly. +1. In the **Merge options** section, ensure **Enable merged results pipelines** is enabled + and select **Enable merge trains**. 1. Select **Save changes**. ## Start a merge train @@ -174,31 +174,6 @@ WARNING: Merging immediately can use a lot of CI/CD resources. Use this option only in critical situations. -## Disable merge trains in GitLab 13.5 and earlier **(PREMIUM SELF)** - -In [GitLab 13.6 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/244831), -you can [enable or disable merge trains in the project settings](#enable-merge-trains). - -In GitLab 13.5 and earlier, merge trains are automatically enabled when -[merged results pipelines](merged_results_pipelines.md) are enabled. -To use merged results pipelines but not merge trains, enable the `disable_merge_trains` -[feature flag](../../user/feature_flags.md). - -[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) -can enable the feature flag to disable merge trains: - -```ruby -Feature.enable(:disable_merge_trains) -``` - -After you enable this feature flag, GitLab cancels existing merge trains. - -To disable the feature flag, which enables merge trains again: - -```ruby -Feature.disable(:disable_merge_trains) -``` - ## Troubleshooting ### Merge request dropped from the merge train diff --git a/doc/operations/incident_management/manage_incidents.md b/doc/operations/incident_management/manage_incidents.md index ba21a210359..1b48de9e478 100644 --- a/doc/operations/incident_management/manage_incidents.md +++ b/doc/operations/incident_management/manage_incidents.md @@ -114,7 +114,7 @@ To view an incident's [details page](incidents.md#incident-details), select it f Whether you can view an incident depends on the [project visibility level](../../user/public_access.md) and the incident's confidentiality status: -- Public project and a non-confidential incident: You don't have to be a member of the project. +- Public project and a non-confidential incident: Anyone can view the incident. - Private project and non-confidential incident: You must have at least the Guest role for the project. - Confidential incident (regardless of project visibility): You must have at least the Reporter role for the project. diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index 746d87bebc3..8dcb9f056ff 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -409,6 +409,24 @@ major release, GitLab 17.0. This gem sees very little use and is better suited f
+### File type variable expansion fixed in downstream pipelines + +
+- Announced in GitLab 16.6 +- Removal in GitLab 17.0 ([breaking change](https://docs.gitlab.com/ee/update/terminology.html#breaking-change)) +- To discuss this change or learn more, see the [deprecation issue](https://gitlab.com/gitlab-org/gitlab/-/issues/419445). +
+ +Previously, if you tried to reference a [file type CI/CD variable](https://docs.gitlab.com/ee/ci/variables/#use-file-type-cicd-variables) in another CI/CD variable, the CI/CD variable would expand to contain the contents of the file. This behavior was incorrect because it did not comply with typical shell variable expansion rules. The CI/CD variable reference should expand to only contain the path to the file, not the contents of the file itself. This was [fixed for most use cases in GitLab 15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/29407). Unfortunately, passing CI/CD variables to downstream pipelines was an edge case not yet fixed, but which will now be fixed in GitLab 17.0. + +With this change, a variable configured in the `.gitlab-ci.yml` file can reference a file variable and be passed to a downstream pipeline, and the file variable will be passed to the downstream pipeline as well. The downstream pipeline will expand the variable reference to the file path, not the file contents. + +This breaking change could disrupt user workflows that depend on expanding a file variable in a downstream pipeline. + +
+ +
+ ### Filepath field in Releases and Release Links APIs
diff --git a/doc/user/group/epics/manage_epics.md b/doc/user/group/epics/manage_epics.md index 5675393441e..a5cc3ad9070 100644 --- a/doc/user/group/epics/manage_epics.md +++ b/doc/user/group/epics/manage_epics.md @@ -206,7 +206,7 @@ To view epics in a group: Whether you can view an epic depends on the [group visibility level](../../public_access.md) and the epic's [confidentiality status](#make-an-epic-confidential): -- Public group and a non-confidential epic: You don't have to be a member of the group. +- Public group and a non-confidential epic: Anyone can view the epic. - Private group and non-confidential epic: You must have at least the Guest role for the group. - Confidential epic (regardless of group visibility): You must have at least the Reporter role for the group. diff --git a/doc/user/profile/account/delete_account.md b/doc/user/profile/account/delete_account.md index d41eee911f9..70c12cbcf00 100644 --- a/doc/user/profile/account/delete_account.md +++ b/doc/user/profile/account/delete_account.md @@ -54,10 +54,9 @@ Using the **Delete user and contributions** option may result in removing more d When deleting users, you can either: -- Delete just the user. Not all associated records are deleted with the user. Instead of being deleted, these records - are moved to a system-wide user with the username Ghost User. The Ghost User's purpose is to act as a container for - such records. Any commits made by a deleted user still display the username of the original user. - The user's personal projects are deleted, not moved to the Ghost User. +- Delete just the user, but move contributions to a system-wide "Ghost User": + - The `@ghost` acts as a container for all deleted users' contributions. + - The user's profile and personal projects are deleted, instead of moved to the Ghost User. - Delete the user and their contributions, including: - Abuse reports. - Emoji reactions. @@ -74,6 +73,9 @@ When deleting users, you can either: [merge requests](../../project/merge_requests/index.md) and [snippets](../../snippets.md). +In both cases, commits retain [user information](https://git-scm.com/book/en/v2/Git-Internals-Git-Objects#_git_commit_objects) +and therefore data integrity within a [Git repository](../../project/repository/index.md). + An alternative to deleting is [blocking a user](../../../administration/moderate_users.md#block-a-user). When a user is deleted from an [abuse report](../../../administration/review_abuse_reports.md) or spam log, these associated diff --git a/doc/user/project/merge_requests/reviews/data_usage.md b/doc/user/project/merge_requests/reviews/data_usage.md index b4b9b19c932..b32c527ab75 100644 --- a/doc/user/project/merge_requests/reviews/data_usage.md +++ b/doc/user/project/merge_requests/reviews/data_usage.md @@ -13,7 +13,7 @@ GitLab Duo Suggested Reviewers is the first user-facing GitLab machine learning ### Enabling the feature -When a Project Maintainer or Owner enables Suggested Reviewers in project settings GitLab kicks off a data extraction job for the project which leverages the Merge Request API to understand pattern of review including recency, domain experience, and frequency to suggest an appropriate reviewer. +When a Project Maintainer or Owner enables Suggested Reviewers in project settings, GitLab kicks off a data extraction job for the project which leverages the Merge Request API to understand pattern of review including recency, domain experience, and frequency to suggest an appropriate reviewer. If projects do not use the [merge request approval process](../approvals/index.md) or do not have any historical merge request data, Suggested Reviewers cannot suggest reviewers. This data extraction job can take a few hours to complete (possibly up to a day), which is largely dependent on the size of the project. The process is automated and no action is needed during this process. Once data extraction is complete, you start getting suggestions in merge requests. diff --git a/doc/user/project/repository/reducing_the_repo_size_using_git.md b/doc/user/project/repository/reducing_the_repo_size_using_git.md index ff9ef5b78f8..ca7f2ae2043 100644 --- a/doc/user/project/repository/reducing_the_repo_size_using_git.md +++ b/doc/user/project/repository/reducing_the_repo_size_using_git.md @@ -325,12 +325,15 @@ are accurate. To expedite this process, see the ['Prune Unreachable Objects' housekeeping task](../../../administration/housekeeping.md). -### Sidekiq process fails to export a project +### Sidekiq process fails to export a project **(FREE SELF)** Occasionally the Sidekiq process can fail to export a project, for example if it is terminated during execution. -To bypass the Sidekiq process, use the Rails console to manually trigger the project export: +GitLab.com users should [contact Support](https://about.gitlab.com/support/#contact-support) to resolve this issue. + +Self-managed users can use the Rails console to bypass the Sidekiq process and +manually trigger the project export: ```ruby project = Project.find(1) diff --git a/gems/gitlab-http/Gemfile.lock b/gems/gitlab-http/Gemfile.lock index c15bcd7cc18..1023e12efd6 100644 --- a/gems/gitlab-http/Gemfile.lock +++ b/gems/gitlab-http/Gemfile.lock @@ -2,7 +2,7 @@ PATH remote: ../gitlab-rspec specs: gitlab-rspec (0.1.0) - activesupport (>= 6.1, < 7.1) + activesupport (>= 6.1, < 8) rspec (~> 3.0) PATH diff --git a/gems/gitlab-utils/Gemfile.lock b/gems/gitlab-utils/Gemfile.lock index 971d90ce146..e6cfe03e60e 100644 --- a/gems/gitlab-utils/Gemfile.lock +++ b/gems/gitlab-utils/Gemfile.lock @@ -2,7 +2,7 @@ PATH remote: ../gitlab-rspec specs: gitlab-rspec (0.1.0) - activesupport (>= 6.1, < 7.1) + activesupport (>= 6.1, < 8) rspec (~> 3.0) PATH diff --git a/gems/rspec_flaky/Gemfile.lock b/gems/rspec_flaky/Gemfile.lock index 3f40a41483e..6be845e81fb 100644 --- a/gems/rspec_flaky/Gemfile.lock +++ b/gems/rspec_flaky/Gemfile.lock @@ -2,7 +2,7 @@ PATH remote: ../gitlab-rspec specs: gitlab-rspec (0.1.0) - activesupport (>= 6.1, < 7.1) + activesupport (>= 6.1, < 8) rspec (~> 3.0) PATH diff --git a/lib/api/invitations.rb b/lib/api/invitations.rb index 34f9538b047..6b4b7b355d0 100644 --- a/lib/api/invitations.rb +++ b/lib/api/invitations.rb @@ -10,6 +10,12 @@ module API helpers ::API::Helpers::MembersHelpers + helpers do + params :invitation_params_ee do + # Overriden in EE + end + end + %w[group project].each do |source_type| params do requires :id, type: String, desc: "The #{source_type} ID" @@ -26,6 +32,8 @@ module API optional :user_id, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The user ID of the new member or multiple IDs separated by commas.' optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY' optional :invite_source, type: String, desc: 'Source that triggered the member creation process', default: 'invitations-api' + + use :invitation_params_ee end post ":id/invitations", urgency: :low do ::Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/354016') @@ -80,6 +88,8 @@ module API requires :email, type: String, desc: 'The email address of the invitation' optional :access_level, type: Integer, values: Gitlab::Access.all_values, desc: 'A valid access level (defaults: `30`, developer access level)' optional :expires_at, type: DateTime, desc: 'Date string in ISO 8601 format (`YYYY-MM-DDTHH:MM:SSZ`)' + + use :invitation_params_ee end put ":id/invitations/:email", requirements: { email: %r{[^/]+} } do source = find_source(source_type, params.delete(:id)) @@ -145,3 +155,5 @@ module API end end end + +API::Members.prepend_mod diff --git a/lib/bitbucket/representation/pull_request_comment.rb b/lib/bitbucket/representation/pull_request_comment.rb index 34dbf9ad22d..11ce0c26677 100644 --- a/lib/bitbucket/representation/pull_request_comment.rb +++ b/lib/bitbucket/representation/pull_request_comment.rb @@ -31,6 +31,10 @@ module Bitbucket raw.key?('parent') end + def deleted? + raw.fetch('deleted', false) + end + private def inline diff --git a/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer.rb b/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer.rb index 8ea8b1562f2..934e4ee1720 100644 --- a/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer.rb +++ b/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer.rb @@ -4,21 +4,22 @@ module Gitlab module BitbucketImport module Importers class PullRequestNotesImporter - include Loggable - include ErrorTracking + include ParallelScheduling def initialize(project, hash) @project = project - @importer = Gitlab::BitbucketImport::Importer.new(project) + @formatter = Gitlab::ImportFormatter.new + @user_finder = UserFinder.new(project) + @ref_converter = Gitlab::BitbucketImport::RefConverter.new(project) @object = hash.with_indifferent_access + @position_map = {} + @discussion_map = {} end def execute log_info(import_stage: 'import_pull_request_notes', message: 'starting', iid: object[:iid]) - merge_request = project.merge_requests.find_by(iid: object[:iid]) # rubocop: disable CodeReuse/ActiveRecord - - importer.import_pull_request_comments(merge_request, merge_request) if merge_request + import_pull_request_comments if merge_request log_info(import_stage: 'import_pull_request_notes', message: 'finished', iid: object[:iid]) rescue StandardError => e @@ -27,7 +28,116 @@ module Gitlab private - attr_reader :object, :project, :importer + attr_reader :object, :project, :formatter, :user_finder, :ref_converter, :discussion_map, :position_map + + def import_pull_request_comments + inline_comments, pr_comments = comments.partition(&:inline?) + + import_inline_comments(inline_comments) + import_standalone_pr_comments(pr_comments) + end + + def import_inline_comments(inline_comments) + children, parents = inline_comments.partition(&:has_parent?) + + parents.each do |comment| + position_map[comment.iid] = build_position(comment) + + import_comment(comment) + end + + children.each do |comment| + position_map[comment.iid] = position_map.fetch(comment.parent_id, nil) + + import_comment(comment) + end + end + + def import_comment(comment) + position = position_map[comment.iid] + discussion_id = discussion_map[comment.parent_id] + + note = create_diff_note(comment, position, discussion_id) + + discussion_map[comment.iid] = note&.discussion_id + end + + def create_diff_note(comment, position, discussion_id) + attributes = pull_request_comment_attributes(comment) + attributes.merge!(position: position, type: 'DiffNote', discussion_id: discussion_id) + + note = merge_request.notes.build(attributes) + + return note if note.save + + # Bitbucket supports the ability to comment on any line, not just the + # line in the diff. If we can't add the note as a DiffNote, fallback to creating + # a regular note. + + log_info(import_stage: 'create_diff_note', message: 'creating fallback DiffNote', iid: merge_request.iid) + create_fallback_diff_note(comment, position) + rescue StandardError => e + Gitlab::ErrorTracking.log_exception( + e, + import_stage: 'create_diff_note', comment_id: comment.iid, error: e.message + ) + + nil + end + + def create_fallback_diff_note(comment, position) + attributes = pull_request_comment_attributes(comment) + note = "*Comment on" + + note += " #{position.old_path}:#{position.old_line} -->" if position&.old_line + note += " #{position.new_path}:#{position.new_line}" if position&.new_line + note += "*\n\n#{comment.note}" + + attributes[:note] = note + merge_request.notes.create!(attributes) + end + + def build_position(pr_comment) + params = { + diff_refs: merge_request.diff_refs, + old_path: pr_comment.file_path, + new_path: pr_comment.file_path, + old_line: pr_comment.old_pos, + new_line: pr_comment.new_pos + } + + Gitlab::Diff::Position.new(params) + end + + def import_standalone_pr_comments(pr_comments) + pr_comments.each do |comment| + attributes = pull_request_comment_attributes(comment) + merge_request.notes.create!(attributes) + end + end + + def pull_request_comment_attributes(comment) + { + project: project, + author_id: user_finder.gitlab_user_id(project, comment.author), + note: comment_note(comment), + created_at: comment.created_at, + updated_at: comment.updated_at + } + end + + def comment_note(comment) + author = formatter.author_line(comment.author) unless user_finder.find_user_id(comment.author) + author.to_s + ref_converter.convert_note(comment.note.to_s) + end + + def merge_request + @merge_request ||= project.merge_requests.iid_in(object[:iid]).first + end + + def comments + client.pull_request_comments(project.import_source, merge_request.iid).reject(&:deleted?) + end end end end diff --git a/lib/gitlab/ci/build/context/build.rb b/lib/gitlab/ci/build/context/build.rb index 48b138b0258..bbcdcd7d389 100644 --- a/lib/gitlab/ci/build/context/build.rb +++ b/lib/gitlab/ci/build/context/build.rb @@ -33,13 +33,9 @@ module Gitlab # Assigning tags and needs is slow and they are not needed for rules # evaluation since we don't use them to compute the variables at this point. def build_attributes - if pipeline.reduced_build_attributes_list_for_rules? - attributes - .except(:tag_list, :needs_attributes) - .merge!(pipeline_attributes, ci_stage_attributes) - else - attributes.merge(pipeline_attributes, ci_stage_attributes) - end + attributes + .except(:tag_list, :needs_attributes) + .merge!(pipeline_attributes, ci_stage_attributes) end def ci_stage_attributes diff --git a/spec/lib/bitbucket/representation/pull_request_comment_spec.rb b/spec/lib/bitbucket/representation/pull_request_comment_spec.rb index e748cd7b955..6fdd1dfa561 100644 --- a/spec/lib/bitbucket/representation/pull_request_comment_spec.rb +++ b/spec/lib/bitbucket/representation/pull_request_comment_spec.rb @@ -2,7 +2,7 @@ require 'fast_spec_helper' -RSpec.describe Bitbucket::Representation::PullRequestComment do +RSpec.describe Bitbucket::Representation::PullRequestComment, feature_category: :importers do describe '#iid' do it { expect(described_class.new('id' => 1).iid).to eq(1) } end @@ -33,4 +33,10 @@ RSpec.describe Bitbucket::Representation::PullRequestComment do it { expect(described_class.new('parent' => {}).has_parent?).to be_truthy } it { expect(described_class.new({}).has_parent?).to be_falsey } end + + describe '#deleted?' do + it { expect(described_class.new('deleted' => true).deleted?).to be_truthy } + it { expect(described_class.new('deleted' => false).deleted?).to be_falsey } + it { expect(described_class.new({}).deleted?).to be_falsey } + end end diff --git a/spec/lib/gitlab/bitbucket_import/importers/issues_notes_importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importers/issues_notes_importer_spec.rb index 043cd7f17b9..1466b91fa8b 100644 --- a/spec/lib/gitlab/bitbucket_import/importers/issues_notes_importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importers/issues_notes_importer_spec.rb @@ -4,8 +4,6 @@ require 'spec_helper' RSpec.describe Gitlab::BitbucketImport::Importers::IssuesNotesImporter, feature_category: :importers do let_it_be(:project) { create(:project, :import_started) } - # let_it_be(:merge_request_1) { create(:merge_request, source_project: project) } - # let_it_be(:merge_request_2) { create(:merge_request, source_project: project, source_branch: 'other-branch') } let_it_be(:issue_1) { create(:issue, project: project) } let_it_be(:issue_2) { create(:issue, project: project) } diff --git a/spec/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer_spec.rb index 4a30f225d66..332f6e5bd03 100644 --- a/spec/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importers/pull_request_notes_importer_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' -RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, feature_category: :importers do +RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, :clean_gitlab_redis_cache, feature_category: :importers do let_it_be(:project) do - create(:project, :import_started, + create(:project, :repository, :import_started, import_data_attributes: { credentials: { 'base_uri' => 'http://bitbucket.org/', 'user' => 'bitbucket', 'password' => 'password' } } @@ -12,28 +12,216 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, fea end let_it_be(:merge_request) { create(:merge_request, source_project: project) } - + let_it_be(:merge_request_diff) { create(:merge_request_diff, :external, merge_request: merge_request) } + let_it_be(:bitbucket_user) { create(:user) } + let_it_be(:identity) { create(:identity, user: bitbucket_user, extern_uid: 'bitbucket_user', provider: :bitbucket) } let(:hash) { { iid: merge_request.iid } } - let(:importer_helper) { Gitlab::BitbucketImport::Importer.new(project) } + let(:client) { Bitbucket::Client.new({}) } + let(:ref_converter) { Gitlab::BitbucketImport::RefConverter.new(project) } + let(:user_finder) { Gitlab::BitbucketImport::UserFinder.new(project) } + let(:note_body) { 'body' } + let(:comments) { [Bitbucket::Representation::PullRequestComment.new(note_hash)] } + let(:created_at) { Date.today - 2.days } + let(:updated_at) { Date.today } + let(:note_hash) do + { + 'id' => 12, + 'user' => { 'nickname' => 'bitbucket_user' }, + 'content' => { 'raw' => note_body }, + 'created_on' => created_at, + 'updated_on' => updated_at + } + end subject(:importer) { described_class.new(project, hash) } before do - allow(Gitlab::BitbucketImport::Importer).to receive(:new).and_return(importer_helper) + allow(Bitbucket::Client).to receive(:new).and_return(client) + allow(Gitlab::BitbucketImport::RefConverter).to receive(:new).and_return(ref_converter) + allow(Gitlab::BitbucketImport::UserFinder).to receive(:new).and_return(user_finder) + allow(client).to receive(:pull_request_comments).and_return(comments) end describe '#execute' do - it 'calls Importer.import_pull_request_comments' do - expect(importer_helper).to receive(:import_pull_request_comments).once + context 'for standalone pr comments' do + it 'calls RefConverter' do + expect(ref_converter).to receive(:convert_note).once.and_call_original + + importer.execute + end + + it 'creates a note with the correct attributes' do + expect { importer.execute }.to change { merge_request.notes.count }.from(0).to(1) + + note = merge_request.notes.first + + expect(note.note).to eq(note_body) + expect(note.author).to eq(bitbucket_user) + expect(note.created_at).to eq(created_at) + expect(note.updated_at).to eq(updated_at) + end + + context 'when the author does not have a bitbucket identity' do + before do + identity.update!(provider: :github) + end + + it 'sets the author to the project creator and adds the author to the note' do + importer.execute + + note = merge_request.notes.first + + expect(note.author).to eq(project.creator) + expect(note.note).to eq("*Created by: bitbucket_user*\n\nbody") + end + end + + context 'when the note is deleted' do + let(:note_hash) do + { + 'id' => 12, + 'user' => { 'nickname' => 'bitbucket_user' }, + 'content' => { 'raw' => note_body }, + 'deleted' => true, + 'created_on' => created_at, + 'updated_on' => updated_at + } + end + + it 'does not create a note' do + expect { importer.execute }.not_to change { merge_request.notes.count } + end + end + end + + context 'for threaded inline comments' do + let(:path) { project.repository.commit.raw_diffs.first.new_path } + let(:reply_body) { 'Some reply' } + let(:comments) do + [ + Bitbucket::Representation::PullRequestComment.new(pr_comment_1), + Bitbucket::Representation::PullRequestComment.new(pr_comment_2) + ] + end - importer.execute + let(:pr_comment_1) do + { + 'id' => 14, + 'inline' => { + 'path' => path, + 'from' => nil, + 'to' => 1 + }, + 'parent' => { 'id' => 13 }, + 'user' => { 'nickname' => 'bitbucket_user' }, + 'content' => { 'raw' => reply_body }, + 'created_on' => created_at, + 'updated_on' => updated_at + } + end + + let(:pr_comment_2) do + { + 'id' => 13, + 'inline' => { + 'path' => path, + 'from' => nil, + 'to' => 1 + }, + 'user' => { 'nickname' => 'non_existent_user' }, + 'content' => { 'raw' => note_body }, + 'created_on' => created_at, + 'updated_on' => updated_at + } + end + + it 'creates notes in the correct position with the right attributes' do + expect { importer.execute }.to change { merge_request.notes.count }.from(0).to(2) + + expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1) + + notes = merge_request.notes.order(:id).to_a + + start_note = notes.first + expect(start_note).to be_a(DiffNote) + expect(start_note.note).to eq("*Created by: non_existent_user*\n\n#{note_body}") + expect(start_note.author).to eq(project.creator) + + reply_note = notes.last + expect(reply_note).to be_a(DiffNote) + expect(reply_note.note).to eq(reply_body) + expect(reply_note.author).to eq(bitbucket_user) + end + + context 'when the comments are not part of the diff' do + let(:pr_comment_1) do + { + 'id' => 14, + 'inline' => { + 'path' => path, + 'from' => nil, + 'to' => nil + }, + 'parent' => { 'id' => 13 }, + 'user' => { 'nickname' => 'bitbucket_user' }, + 'content' => { 'raw' => reply_body }, + 'created_on' => created_at, + 'updated_on' => updated_at + } + end + + let(:pr_comment_2) do + { + 'id' => 13, + 'inline' => { + 'path' => path, + 'from' => nil, + 'to' => nil + }, + 'user' => { 'nickname' => 'bitbucket_user' }, + 'content' => { 'raw' => note_body }, + 'created_on' => created_at, + 'updated_on' => updated_at + } + end + + it 'creates them as normal notes' do + expect { importer.execute }.to change { merge_request.notes.count }.from(0).to(2) + + notes = merge_request.notes.order(:id).to_a + + first_note = notes.first + expect(first_note).not_to be_a(DiffNote) + expect(first_note.note).to eq("*Comment on*\n\n#{note_body}") + expect(first_note.author).to eq(bitbucket_user) + + second_note = notes.last + expect(second_note).not_to be_a(DiffNote) + expect(second_note.note).to eq("*Comment on*\n\n#{reply_body}") + expect(second_note.author).to eq(bitbucket_user) + end + end + + context 'when an error is raised for one note' do + before do + allow(user_finder).to receive(:gitlab_user_id).and_call_original + allow(user_finder).to receive(:gitlab_user_id).with(project, 'bitbucket_user').and_raise(StandardError) + end + + it 'tracks the error and continues to import other notes' do + expect(Gitlab::ErrorTracking).to receive(:log_exception) + .with(anything, hash_including(comment_id: 14)).and_call_original + + expect { importer.execute }.to change { merge_request.notes.count }.from(0).to(1) + end + end end context 'when the merge request does not exist' do let(:hash) { { iid: 'nonexistent' } } - it 'does not call Importer.import_pull_request_comments' do - expect(importer_helper).not_to receive(:import_pull_request_comments) + it 'does not call #import_pull_request_comments' do + expect(importer).not_to receive(:import_pull_request_comments) importer.execute end @@ -46,8 +234,8 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, fea merge_request.update!(source_project: another_project, target_project: another_project) end - it 'does not call Importer.import_pull_request_comments' do - expect(importer_helper).not_to receive(:import_pull_request_comments) + it 'does not call #import_pull_request_comments' do + expect(importer).not_to receive(:import_pull_request_comments) importer.execute end @@ -55,7 +243,7 @@ RSpec.describe Gitlab::BitbucketImport::Importers::PullRequestNotesImporter, fea context 'when an error is raised' do before do - allow(importer_helper).to receive(:import_pull_request_comments).and_raise(StandardError) + allow(importer).to receive(:import_pull_request_comments).and_raise(StandardError) end it 'tracks the failure and does not fail' do diff --git a/spec/lib/gitlab/ci/build/context/build_spec.rb b/spec/lib/gitlab/ci/build/context/build_spec.rb index fae02e140f2..9fdb4ee9393 100644 --- a/spec/lib/gitlab/ci/build/context/build_spec.rb +++ b/spec/lib/gitlab/ci/build/context/build_spec.rb @@ -41,16 +41,6 @@ RSpec.describe Gitlab::Ci::Build::Context::Build, feature_category: :pipeline_co it { expect(context.variables).to be_instance_of(Gitlab::Ci::Variables::Collection) } it_behaves_like 'variables collection' - - context 'with FF disabled' do - before do - stub_feature_flags(reduced_build_attributes_list_for_rules: false) - end - - it { expect(context.variables).to be_instance_of(Gitlab::Ci::Variables::Collection) } - - it_behaves_like 'variables collection' - end end describe '#variables_hash' do @@ -59,15 +49,5 @@ RSpec.describe Gitlab::Ci::Build::Context::Build, feature_category: :pipeline_co it { expect(context.variables_hash).to be_instance_of(ActiveSupport::HashWithIndifferentAccess) } it_behaves_like 'variables collection' - - context 'with FF disabled' do - before do - stub_feature_flags(reduced_build_attributes_list_for_rules: false) - end - - it { expect(context.variables_hash).to be_instance_of(ActiveSupport::HashWithIndifferentAccess) } - - it_behaves_like 'variables collection' - end end end diff --git a/spec/migrations/20230929155123_migrate_disable_merge_trains_value_spec.rb b/spec/migrations/20230929155123_migrate_disable_merge_trains_value_spec.rb new file mode 100644 index 00000000000..ee011687bbb --- /dev/null +++ b/spec/migrations/20230929155123_migrate_disable_merge_trains_value_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! 'migrate_disable_merge_trains_value' + +RSpec.describe MigrateDisableMergeTrainsValue, schema: 20230929155123, feature_category: :continuous_integration do + let!(:feature_gates) { table(:feature_gates) } + let!(:projects) { table(:projects) } + let!(:project_ci_cd_settings) { table(:project_ci_cd_settings) } + let!(:namespace1) { table(:namespaces).create!(name: 'name1', path: 'path1') } + let!(:namespace2) { table(:namespaces).create!(name: 'name2', path: 'path2') } + + let!(:project_with_flag_on) do + projects + .create!( + name: "project", + path: "project", + namespace_id: namespace1.id, + project_namespace_id: namespace1.id + ) + end + + let!(:project_with_flag_off) do + projects + .create!( + name: "project2", + path: "project2", + namespace_id: namespace2.id, + project_namespace_id: namespace2.id + ) + end + + let!(:settings_flag_on) do + project_ci_cd_settings.create!( + merge_trains_enabled: true, + project_id: project_with_flag_on.id + ) + end + + let!(:settings_flag_off) do + project_ci_cd_settings.create!( + merge_trains_enabled: true, + project_id: project_with_flag_off.id + ) + end + + let!(:migration) { described_class.new } + + before do + # Enable the feature flag + feature_gates.create!( + feature_key: 'disable_merge_trains', + key: 'actors', + value: "Project:#{project_with_flag_on.id}" + ) + + migration.up + end + + describe '#up' do + it 'migrates the flag value into the setting value' do + expect( + settings_flag_on.reload.merge_trains_enabled + ).to eq(false) + expect( + settings_flag_off.reload.merge_trains_enabled + ).to eq(true) + end + end + + describe '#down' do + it 'reverts the migration' do + migration.down + + expect( + settings_flag_on.reload.merge_trains_enabled + ).to eq(true) + expect( + settings_flag_off.reload.merge_trains_enabled + ).to eq(true) + end + end +end diff --git a/spec/models/ci/catalog/resource_spec.rb b/spec/models/ci/catalog/resource_spec.rb index 4ce1433e015..f9723385111 100644 --- a/spec/models/ci/catalog/resource_spec.rb +++ b/spec/models/ci/catalog/resource_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do let_it_be(:project) { create(:project, name: 'A') } let_it_be(:project_2) { build(:project, name: 'Z') } let_it_be(:project_3) { build(:project, name: 'L') } - let_it_be(:resource) { create(:ci_catalog_resource, project: project, latest_released_at: tomorrow) } + let_it_be_with_reload(:resource) { create(:ci_catalog_resource, project: project, latest_released_at: tomorrow) } let_it_be(:resource_2) { create(:ci_catalog_resource, project: project_2, latest_released_at: today) } let_it_be(:resource_3) { create(:ci_catalog_resource, project: project_3, latest_released_at: nil) } @@ -95,4 +95,27 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do expect(resource.state).to eq('draft') end end + + describe '#unpublish!' do + context 'when the catalog resource is in published state' do + it 'updates the state to draft' do + resource.update!(state: :published) + expect(resource.state).to eq('published') + + resource.unpublish! + + expect(resource.reload.state).to eq('draft') + end + end + + context 'when the catalog resource is already in draft state' do + it 'leaves the state as draft' do + expect(resource.state).to eq('draft') + + resource.unpublish! + + expect(resource.reload.state).to eq('draft') + end + end + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 887ec48ec8f..0094f56d96e 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -5578,25 +5578,4 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end end - - describe '#reduced_build_attributes_list_for_rules?' do - subject { pipeline.reduced_build_attributes_list_for_rules? } - - let(:pipeline) { build_stubbed(:ci_pipeline, project: project, user: user) } - - it { is_expected.to be_truthy } - - it 'memoizes the result' do - expect { subject } - .to change { pipeline.strong_memoized?(:reduced_build_attributes_list_for_rules?) } - end - - context 'with the FF disabled' do - before do - stub_feature_flags(reduced_build_attributes_list_for_rules: false) - end - - it { is_expected.to be_falsey } - end - end end diff --git a/spec/requests/api/graphql/mutations/ci/catalog/unpublish_spec.rb b/spec/requests/api/graphql/mutations/ci/catalog/unpublish_spec.rb new file mode 100644 index 00000000000..07465777263 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/catalog/unpublish_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'CatalogResourceUnpublish', feature_category: :pipeline_composition do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be_with_reload(:resource) { create(:ci_catalog_resource) } + + let(:mutation) do + graphql_mutation( + :catalog_resource_unpublish, + id: resource.to_gid.to_s + ) + end + + subject(:post_query) { post_graphql_mutation(mutation, current_user: current_user) } + + context 'when unauthorized' do + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when authorized' do + before_all do + resource.project.add_owner(current_user) + end + + context 'when the catalog resource is in published state' do + it 'updates the state to draft' do + resource.update!(state: :published) + expect(resource.state).to eq('published') + + post_query + + expect(resource.reload.state).to eq('draft') + expect_graphql_errors_to_be_empty + end + end + + context 'when the catalog resource is already in draft state' do + it 'leaves the state as draft' do + expect(resource.state).to eq('draft') + + post_query + + expect(resource.reload.state).to eq('draft') + expect_graphql_errors_to_be_empty + end + end + end +end -- cgit v1.2.3