From cdd5eba514e79c7801ff2eeb76e915e3f31ca5d7 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 13 Jun 2022 12:08:29 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- Gemfile | 2 +- Gemfile.lock | 4 +- .../test_reports/test_case_details.vue | 2 +- .../new_namespace/new_namespace_page.vue | 6 +- .../diffs/overflow_warning_component.rb | 7 - app/components/diffs/stats_component.rb | 8 - app/components/pajamas/component.rb | 3 - app/graphql/resolvers/work_items_resolver.rb | 60 ++++++ app/graphql/types/project_type.rb | 8 + app/graphql/types/work_item_sort_enum.rb | 11 + .../concerns/integrations/has_data_fields.rb | 3 +- app/models/integrations/harbor.rb | 2 +- .../metadata/append_package_file_service.rb | 4 +- app/uploaders/gitlab_uploader.rb | 8 + app/views/abuse_reports/new.html.haml | 2 +- app/views/admin/topics/_form.html.haml | 2 +- app/views/groups/_new_group_fields.html.haml | 2 +- .../projects/branch_rules/_show.html.haml | 2 +- app/views/shared/issuable/_sidebar.html.haml | 3 +- .../application_settings_cache.md | 6 +- doc/api/graphql/reference/index.md | 42 ++++ doc/install/requirements.md | 4 +- doc/user/group/import/index.md | 2 +- doc/user/permissions.md | 2 +- doc/user/project/integrations/harbor.md | 2 +- lib/gitlab/gitaly_client/operation_service.rb | 32 +-- package.json | 2 +- .../resolvers/work_items_resolver_spec.rb | 190 ++++++++++++++++++ .../gitaly_client/operation_service_spec.rb | 74 +++++-- .../integrations/has_data_fields_spec.rb | 88 ++++---- spec/models/integrations/harbor_spec.rb | 10 + .../api/graphql/project/work_items_spec.rb | 121 +++++++++++ spec/requests/api/integrations_spec.rb | 54 +++-- .../append_package_file_service_spec.rb | 21 +- .../integrations_shared_context.rb | 36 +++- spec/tooling/quality/test_level_spec.rb | 43 +++- spec/uploaders/gitlab_uploader_spec.rb | 6 + tooling/quality/test_level.rb | 13 +- yarn.lock | 8 +- 39 files changed, 736 insertions(+), 159 deletions(-) create mode 100644 app/graphql/resolvers/work_items_resolver.rb create mode 100644 app/graphql/types/work_item_sort_enum.rb create mode 100644 spec/graphql/resolvers/work_items_resolver_spec.rb create mode 100644 spec/requests/api/graphql/project/work_items_spec.rb diff --git a/Gemfile b/Gemfile index ffcc8e289b3..a990feb2ea7 100644 --- a/Gemfile +++ b/Gemfile @@ -485,7 +485,7 @@ gem 'ssh_data', '~> 1.2' gem 'spamcheck', '~> 0.1.0' # Gitaly GRPC protocol definitions -gem 'gitaly', '~> 15.0.0-rc3' +gem 'gitaly', '~> 15.1.0-rc1' # KAS GRPC protocol definitions gem 'kas-grpc', '~> 0.0.2' diff --git a/Gemfile.lock b/Gemfile.lock index b98a18d8f8f..d21c8de0f2a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -466,7 +466,7 @@ GEM rails (>= 3.2.0) git (1.7.0) rchardet (~> 1.8) - gitaly (15.0.0.pre.rc3) + gitaly (15.1.0.pre.rc1) grpc (~> 1.0) github-markup (1.7.0) gitlab (4.16.1) @@ -1529,7 +1529,7 @@ DEPENDENCIES gettext (~> 3.3) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly (~> 15.0.0.pre.rc3) + gitaly (~> 15.1.0.pre.rc1) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) gitlab-dangerfiles (~> 3.1.0) diff --git a/app/assets/javascripts/pipelines/components/test_reports/test_case_details.vue b/app/assets/javascripts/pipelines/components/test_reports/test_case_details.vue index efd0310334e..76ee6ab613b 100644 --- a/app/assets/javascripts/pipelines/components/test_reports/test_case_details.vue +++ b/app/assets/javascripts/pipelines/components/test_reports/test_case_details.vue @@ -60,7 +60,7 @@ export default { }, modalCloseButton: { text: __('Close'), - attributes: [{ variant: 'info' }], + attributes: [{ variant: 'confirm' }], }, }; diff --git a/app/assets/javascripts/vue_shared/new_namespace/new_namespace_page.vue b/app/assets/javascripts/vue_shared/new_namespace/new_namespace_page.vue index 1f3cc663848..8e9b8ef3e6f 100644 --- a/app/assets/javascripts/vue_shared/new_namespace/new_namespace_page.vue +++ b/app/assets/javascripts/vue_shared/new_namespace/new_namespace_page.vue @@ -130,11 +130,7 @@ export default {
- - - +
diff --git a/app/components/diffs/overflow_warning_component.rb b/app/components/diffs/overflow_warning_component.rb index 0d0e225beb4..5123809cfdc 100644 --- a/app/components/diffs/overflow_warning_component.rb +++ b/app/components/diffs/overflow_warning_component.rb @@ -2,12 +2,6 @@ module Diffs class OverflowWarningComponent < BaseComponent - # Skipping coverage because of https://gitlab.com/gitlab-org/gitlab/-/issues/357381 - # - # This is fully tested by the output in the view part of this component, - # but undercoverage doesn't understand the relationship between the two parts. - # - # :nocov: def initialize(diffs:, diff_files:, project:, commit: nil, merge_request: nil) @diffs = diffs @diff_files = diff_files @@ -68,6 +62,5 @@ module Diffs def button_classes "btn gl-alert-action btn-default gl-button btn-default-secondary" end - # :nocov: end end diff --git a/app/components/diffs/stats_component.rb b/app/components/diffs/stats_component.rb index 55589c7b015..74788133aa2 100644 --- a/app/components/diffs/stats_component.rb +++ b/app/components/diffs/stats_component.rb @@ -28,13 +28,6 @@ module Diffs Gitlab::Json.dump(diffs_map) end - # Disabled undercoverage reports for this method - # as it returns a false positive on the last line, - # which is covered in the tests - # - # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/357381 - # - # :nocov: def diff_file_path_text(diff_file, max: 60) path = diff_file.new_path @@ -42,7 +35,6 @@ module Diffs "...#{path[-(max - 3)..]}" end - # :nocov: private diff --git a/app/components/pajamas/component.rb b/app/components/pajamas/component.rb index b05d93b680e..70c5f5be596 100644 --- a/app/components/pajamas/component.rb +++ b/app/components/pajamas/component.rb @@ -4,8 +4,6 @@ module Pajamas class Component < ViewComponent::Base private - # :nocov: - # Filter a given a value against a list of allowed values # If no value is given or value is not allowed return default one # @@ -18,6 +16,5 @@ module Pajamas default end - # :nocov: end end diff --git a/app/graphql/resolvers/work_items_resolver.rb b/app/graphql/resolvers/work_items_resolver.rb new file mode 100644 index 00000000000..1bc74131b9e --- /dev/null +++ b/app/graphql/resolvers/work_items_resolver.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Resolvers + class WorkItemsResolver < BaseResolver + include SearchArguments + include LooksAhead + + type Types::WorkItemType.connection_type, null: true + + argument :iid, GraphQL::Types::String, + required: false, + description: 'IID of the issue. For example, "1".' + argument :iids, [GraphQL::Types::String], + required: false, + description: 'List of IIDs of work items. For example, `["1", "2"]`.' + argument :sort, Types::WorkItemSortEnum, + description: 'Sort work items by this criteria.', + required: false, + default_value: :created_desc + argument :state, Types::IssuableStateEnum, + required: false, + description: 'Current state of this work item.' + argument :types, [Types::IssueTypeEnum], + as: :issue_types, + description: 'Filter work items by the given work item types.', + required: false + + def resolve_with_lookahead(**args) + # The project could have been loaded in batch by `BatchLoader`. + # At this point we need the `id` of the project to query for issues, so + # make sure it's loaded and not `nil` before continuing. + parent = object.respond_to?(:sync) ? object.sync : object + return WorkItem.none if parent.nil? || !parent.work_items_feature_flag_enabled? + + args[:iids] ||= [args.delete(:iid)].compact if args[:iid] + args[:attempt_project_search_optimizations] = true if args[:search].present? + + finder = ::WorkItems::WorkItemsFinder.new(current_user, args) + + Gitlab::Graphql::Loaders::IssuableLoader.new(parent, finder).batching_find_all { |q| apply_lookahead(q) } + end + + def ready?(**args) + validate_anonymous_search_access! if args[:search].present? + + super + end + + private + + def unconditional_includes + [ + { + project: [:project_feature, :group] + }, + :author + ] + end + end +end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index a6cb451e614..603d5ead540 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -144,6 +144,14 @@ module Types extras: [:lookahead], resolver: Resolvers::IssuesResolver + field :work_items, + Types::WorkItemType.connection_type, + null: true, + deprecated: { milestone: '15.1', reason: :alpha }, + description: 'Work items of the project.', + extras: [:lookahead], + resolver: Resolvers::WorkItemsResolver + field :issue_status_counts, Types::IssueStatusCountsType, null: true, diff --git a/app/graphql/types/work_item_sort_enum.rb b/app/graphql/types/work_item_sort_enum.rb new file mode 100644 index 00000000000..e644313d409 --- /dev/null +++ b/app/graphql/types/work_item_sort_enum.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Types + class WorkItemSortEnum < SortEnum + graphql_name 'WorkItemSort' + description 'Values for sorting work items' + + value 'TITLE_ASC', 'Title by ascending order.', value: :title_asc + value 'TITLE_DESC', 'Title by descending order.', value: :title_desc + end +end diff --git a/app/models/concerns/integrations/has_data_fields.rb b/app/models/concerns/integrations/has_data_fields.rb index 25a1d855119..635147a2f3c 100644 --- a/app/models/concerns/integrations/has_data_fields.rb +++ b/app/models/concerns/integrations/has_data_fields.rb @@ -12,7 +12,8 @@ module Integrations self.class_eval <<-RUBY, __FILE__, __LINE__ + 1 unless method_defined?(arg) def #{arg} - data_fields.send('#{arg}') || (properties && properties['#{arg}']) + value = data_fields.send('#{arg}') + value.nil? ? properties&.dig('#{arg}') : value end end diff --git a/app/models/integrations/harbor.rb b/app/models/integrations/harbor.rb index 6b561575f30..44813795fc0 100644 --- a/app/models/integrations/harbor.rb +++ b/app/models/integrations/harbor.rb @@ -81,7 +81,7 @@ module Integrations [ { key: 'HARBOR_URL', value: url }, { key: 'HARBOR_PROJECT', value: project_name }, - { key: 'HARBOR_USERNAME', value: username }, + { key: 'HARBOR_USERNAME', value: username.gsub(/^robot\$/, 'robot$$') }, { key: 'HARBOR_PASSWORD', value: password, public: false, masked: true } ] end diff --git a/app/services/packages/maven/metadata/append_package_file_service.rb b/app/services/packages/maven/metadata/append_package_file_service.rb index e991576ebc6..c778620ea8e 100644 --- a/app/services/packages/maven/metadata/append_package_file_service.rb +++ b/app/services/packages/maven/metadata/append_package_file_service.rb @@ -36,7 +36,7 @@ module Packages sha256: file_sha256 ) - append_metadata_file(content: file_md5, file_name: MD5_FILE_NAME) + append_metadata_file(content: file_md5, file_name: MD5_FILE_NAME) unless Gitlab::FIPS.enabled? append_metadata_file(content: file_sha1, file_name: SHA1_FILE_NAME) append_metadata_file(content: file_sha256, file_name: SHA256_FILE_NAME) append_metadata_file(content: file_sha512, file_name: SHA512_FILE_NAME) @@ -70,6 +70,8 @@ module Packages end def digest_from(content, type) + return if type == :md5 && Gitlab::FIPS.enabled? + digest_class = case type when :md5 Digest::MD5 diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index 9758d3c87aa..abe06bd97e1 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -31,6 +31,14 @@ class GitlabUploader < CarrierWave::Uploader::Base def absolute_path(upload_record) File.join(root, upload_record.path) end + + def version(*args, **kwargs, &block) + # CarrierWave uploaded file "versions" are not tracked in the uploads + # table. Because of this they won't get replicated to Geo secondaries. + # If we ever want to use versions, we need to fix that first. Also see + # https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1757. + raise "using CarrierWave alternate file version is not supported" + end end storage_options Gitlab.config.uploads diff --git a/app/views/abuse_reports/new.html.haml b/app/views/abuse_reports/new.html.haml index cb8f1bd1e9c..258fdb4ad9a 100644 --- a/app/views/abuse_reports/new.html.haml +++ b/app/views/abuse_reports/new.html.haml @@ -7,7 +7,7 @@ = _("A member of the abuse team will review your report as soon as possible.") %hr = form_for @abuse_report, html: { class: 'js-quick-submit js-requires-input'} do |f| - = form_errors(@abuse_report) + = form_errors(@abuse_report, pajamas_alert: true) = f.hidden_field :user_id .form-group.row diff --git a/app/views/admin/topics/_form.html.haml b/app/views/admin/topics/_form.html.haml index 9b9d97950cc..1c1bc61aef2 100644 --- a/app/views/admin/topics/_form.html.haml +++ b/app/views/admin/topics/_form.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @topic, url: url, html: { multipart: true, class: 'js-project-topic-form gl-show-field-errors common-note-form js-quick-submit js-requires-input' }, authenticity_token: true do |f| - = form_errors(@topic) + = form_errors(@topic, pajamas_alert: true) .form-group = f.label :name do diff --git a/app/views/groups/_new_group_fields.html.haml b/app/views/groups/_new_group_fields.html.haml index fe2ee62d9be..83211505f36 100644 --- a/app/views/groups/_new_group_fields.html.haml +++ b/app/views/groups/_new_group_fields.html.haml @@ -1,4 +1,4 @@ -= form_errors(@group) += form_errors(@group, pajamas_alert: true) = render 'shared/group_form', f: f, autofocus: true .row diff --git a/app/views/projects/branch_rules/_show.html.haml b/app/views/projects/branch_rules/_show.html.haml index 5f1e7d74b35..af0e656d301 100644 --- a/app/views/projects/branch_rules/_show.html.haml +++ b/app/views/projects/branch_rules/_show.html.haml @@ -3,7 +3,7 @@ %section.settings.no-animate#branch-rules{ class: ('expanded' if expanded) } .settings-header %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Branch rules') - %button.btn.gl-button.btn-default.js-settings-toggle + = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do = expanded ? _('Collapse') : _('Expand') %p = _('Define rules for who can push, merge, and the required approvals for each branch.') diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index f1d7fca37e1..55f5dce8b37 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -106,8 +106,7 @@ .sidebar-collapsed-icon{ data: { toggle: 'tooltip', placement: 'left', container: 'body', boundary: 'viewport' }, title: _('Move issue') } = sprite_icon('long-arrow') .dropdown.sidebar-move-issue-dropdown.hide-collapsed - %button.gl-button.btn.btn-default.btn-block.js-sidebar-dropdown-toggle.js-move-issue{ type: 'button', - data: { toggle: 'dropdown', display: 'static', track_label: "right_sidebar", track_property: "move_issue", track_action: "click_button", track_value: "" } } + = render Pajamas::ButtonComponent.new(block: true, button_options: { class: 'js-sidebar-dropdown-toggle js-move-issue', data: { toggle: 'dropdown', display: 'static', track_label: "right_sidebar", track_property: "move_issue", track_action: "click_button", track_value: "" } } ) do = _('Move issue') .dropdown-menu.dropdown-menu-selectable.dropdown-extended-height = dropdown_title(_('Move issue')) diff --git a/doc/administration/application_settings_cache.md b/doc/administration/application_settings_cache.md index 33649cf53fc..6c58e6886c4 100644 --- a/doc/administration/application_settings_cache.md +++ b/doc/administration/application_settings_cache.md @@ -4,7 +4,7 @@ group: Memory info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Change the expiration interval for application cache **(FREE SELF)** +# Application cache interval **(FREE SELF)** By default, GitLab caches application settings for 60 seconds. Occasionally, you may need to increase that interval to have more delay between application @@ -14,6 +14,8 @@ We recommend you set this value to greater than `0` seconds. Setting it to `0` causes the `application_settings` table to load for every request. This causes extra load for Redis and PostgreSQL. +## Change the expiration interval for application cache + To change the expiry value: **For Omnibus installations** @@ -32,8 +34,6 @@ To change the expiry value: gitlab-ctl restart ``` ---- - **For installations from source** 1. Edit `config/gitlab.yml`: diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index f46b0f4d2dc..34724733810 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -15806,6 +15806,31 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `taskable` | [`Boolean`](#boolean) | If `true`, only taskable work item types will be returned. Argument is experimental and can be removed in the future without notice. | +##### `Project.workItems` + +Work items of the project. + +WARNING: +**Deprecated** in 15.1. +This feature is in Alpha, and can be removed or changed at any point. + +Returns [`WorkItemConnection`](#workitemconnection). + +This field returns a [connection](#connections). It accepts the +four standard [pagination arguments](#connection-pagination-arguments): +`before: String`, `after: String`, `first: Int`, `last: Int`. + +###### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `iid` | [`String`](#string) | IID of the issue. For example, "1". | +| `iids` | [`[String!]`](#string) | List of IIDs of work items. For example, `["1", "2"]`. | +| `search` | [`String`](#string) | Search query for title or description. | +| `sort` | [`WorkItemSort`](#workitemsort) | Sort work items by this criteria. | +| `state` | [`IssuableState`](#issuablestate) | Current state of this work item. | +| `types` | [`[IssueType!]`](#issuetype) | Filter work items by the given work item types. | + ### `ProjectCiCdSetting` #### Fields @@ -19887,6 +19912,23 @@ Weight ID wildcard values. | `ANY` | Weight is assigned. | | `NONE` | No weight is assigned. | +### `WorkItemSort` + +Values for sorting work items. + +| Value | Description | +| ----- | ----------- | +| `CREATED_ASC` | Created at ascending order. | +| `CREATED_DESC` | Created at descending order. | +| `TITLE_ASC` | Title by ascending order. | +| `TITLE_DESC` | Title by descending order. | +| `UPDATED_ASC` | Updated at ascending order. | +| `UPDATED_DESC` | Updated at descending order. | +| `created_asc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `CREATED_ASC`. | +| `created_desc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `CREATED_DESC`. | +| `updated_asc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `UPDATED_ASC`. | +| `updated_desc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `UPDATED_DESC`. | + ### `WorkItemState` State of a GitLab work item. diff --git a/doc/install/requirements.md b/doc/install/requirements.md index 629b1841875..d01860cb24f 100644 --- a/doc/install/requirements.md +++ b/doc/install/requirements.md @@ -113,8 +113,8 @@ the following table) as these were used for development and testing: |----------------|----------------------------| | 13.0 | 11 | | 14.0 | 12.10 | -| 15.0 | 12.0 | -| 16.0 (planned) | 13.0 | +| 15.0 | 12.10 | +| 16.0 (planned) | 13.6 | You must also ensure the following extensions are loaded into every GitLab database. [Read more about this requirement, and troubleshooting](postgresql_extensions.md). diff --git a/doc/user/group/import/index.md b/doc/user/group/import/index.md index f2d1bb8b069..ae1465d0b1b 100644 --- a/doc/user/group/import/index.md +++ b/doc/user/group/import/index.md @@ -18,7 +18,7 @@ this feature, ask an administrator to [enable the feature flag](../../../adminis `bulk_import_projects`. On GitLab.com, migrating group resources is available but migrating project resources is not available. -You can migrate your existing top-level groups to any of the following: +Users with the Owner role on a top-level group can migrate it to: - Another top-level group. - The subgroup of any existing top-level group. diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 552df5173af..c025967579a 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -431,7 +431,7 @@ The following table lists group permissions available for each role: | View [Billing](../subscriptions/gitlab_com/index.md#view-your-gitlab-saas-subscription) | | | | | ✓ (4) | | View group [Usage Quotas](usage_quotas.md) page | | | | | ✓ (4) | | Manage group runners | | | | | ✓ | -| GitLab migration | | | | | ✓ | +| [Migrate groups](group/import/index.md) | | | | | ✓ | diff --git a/doc/user/project/integrations/harbor.md b/doc/user/project/integrations/harbor.md index a952abf691d..1319c9e74cd 100644 --- a/doc/user/project/integrations/harbor.md +++ b/doc/user/project/integrations/harbor.md @@ -39,7 +39,7 @@ GitLab supports integrating Harbor projects at the group or project level. Compl After the Harbor integration is activated: -- The global variables `$HARBOR_USER`, `$HARBOR_PASSWORD`, `$HARBOR_URL`, and `$HARBOR_PROJECT` are created for CI/CD use. +- The global variables `$HARBOR_USERNAME`, `$HARBOR_PASSWORD`, `$HARBOR_URL`, and `$HARBOR_PROJECT` are created for CI/CD use. - The project-level integration settings override the group-level integration settings. ## Secure your requests to the Harbor APIs diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 5a1699541d0..d575c0f470d 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -101,6 +101,16 @@ module Gitlab if pre_receive_error = response.pre_receive_error.presence raise Gitlab::Git::PreReceiveError, pre_receive_error end + rescue GRPC::BadStatus => e + detailed_error = decode_detailed_error(e) + + case detailed_error&.error + when :custom_hook + raise Gitlab::Git::PreReceiveError.new(custom_hook_error_message(detailed_error.custom_hook), + fallback_message: e.details) + else + raise + end end def user_merge_to_ref(user, source_sha:, branch:, target_ref:, message:, first_parent_ref:, allow_conflicts: false) @@ -164,14 +174,8 @@ module Gitlab # These messages were returned from internal/allowed API calls raise Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message) when :custom_hook - # Custom hooks may return messages via either stdout or stderr which have a specific prefix. If - # that prefix is present we'll want to print the hook's output, otherwise we'll want to print the - # Gitaly error as a fallback. - custom_hook_error = detailed_error.custom_hook - custom_hook_output = custom_hook_error.stderr.presence || custom_hook_error.stdout - error_message = EncodingHelper.encode!(custom_hook_output) - - raise Gitlab::Git::PreReceiveError.new(error_message, fallback_message: e.details) + raise Gitlab::Git::PreReceiveError.new(custom_hook_error_message(detailed_error.custom_hook), + fallback_message: e.details) when :reference_update # We simply ignore any reference update errors which are typically an # indicator of multiple RPC calls trying to update the same reference @@ -308,10 +312,6 @@ module Gitlab timeout: GitalyClient.long_timeout ) - if response.git_error.presence - raise Gitlab::Git::Repository::GitError, response.git_error - end - response.squash_sha rescue GRPC::BadStatus => e detailed_error = decode_detailed_error(e) @@ -550,6 +550,14 @@ module Gitlab # Error Class might not be known to ruby yet nil end + + def custom_hook_error_message(custom_hook_error) + # Custom hooks may return messages via either stdout or stderr which have a specific prefix. If + # that prefix is present we'll want to print the hook's output, otherwise we'll want to print the + # Gitaly error as a fallback. + custom_hook_output = custom_hook_error.stderr.presence || custom_hook_error.stdout + EncodingHelper.encode!(custom_hook_output) + end end end end diff --git a/package.json b/package.json index 1ba5944e93d..3beda242e39 100644 --- a/package.json +++ b/package.json @@ -207,7 +207,7 @@ "@babel/plugin-transform-modules-commonjs": "^7.10.1", "@gitlab/eslint-plugin": "12.1.0", "@gitlab/stylelint-config": "4.0.0", - "@graphql-eslint/eslint-plugin": "3.10.3", + "@graphql-eslint/eslint-plugin": "3.10.4", "@testing-library/dom": "^7.16.2", "@types/jest": "^26.0.24", "@vue/test-utils": "1.3.0", diff --git a/spec/graphql/resolvers/work_items_resolver_spec.rb b/spec/graphql/resolvers/work_items_resolver_spec.rb new file mode 100644 index 00000000000..df79cd74694 --- /dev/null +++ b/spec/graphql/resolvers/work_items_resolver_spec.rb @@ -0,0 +1,190 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Resolvers::WorkItemsResolver do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:reporter) { create(:user) } + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:other_project) { create(:project, group: group) } + + let_it_be(:item1) do + create(:work_item, project: project, state: :opened, created_at: + 3.hours.ago, updated_at: 3.hours.ago) + end + + let_it_be(:item2) do + create(:work_item, project: project, state: :closed, title: 'foo', + created_at: 1.hour.ago, updated_at: 1.hour.ago, closed_at: + 1.hour.ago) + end + + let_it_be(:item3) do + create(:work_item, project: other_project, state: :closed, title: 'foo', + created_at: 1.hour.ago, updated_at: 1.hour.ago, closed_at: + 1.hour.ago) + end + + let_it_be(:item4) { create(:work_item) } + + specify do + expect(described_class).to have_nullable_graphql_type(Types::WorkItemType.connection_type) + end + + context "with a project" do + before_all do + project.add_developer(current_user) + project.add_reporter(reporter) + end + + describe '#resolve' do + it 'finds all items' do + expect(resolve_items).to contain_exactly(item1, item2) + end + + it 'filters by state' do + expect(resolve_items(state: 'opened')).to contain_exactly(item1) + expect(resolve_items(state: 'closed')).to contain_exactly(item2) + end + + context 'when searching items' do + it 'returns correct items' do + expect(resolve_items(search: 'foo')).to contain_exactly(item2) + end + + it 'uses project search optimization' do + expected_arguments = a_hash_including( + search: 'foo', + attempt_project_search_optimizations: true + ) + expect(::WorkItems::WorkItemsFinder).to receive(:new).with(anything, expected_arguments).and_call_original + + resolve_items(search: 'foo') + end + + context 'with anonymous user' do + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:public_item) { create(:work_item, project: public_project, title: 'Test item') } + + context 'with disable_anonymous_search enabled' do + before do + stub_feature_flags(disable_anonymous_search: true) + end + + it 'generates an error' do + error_message = "User must be authenticated to include the `search` argument." + + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do + resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil }) + end + end + end + + context 'with disable_anonymous_search disabled' do + before do + stub_feature_flags(disable_anonymous_search: false) + end + + it 'returns correct items' do + expect( + resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil }) + ).to contain_exactly(public_item) + end + end + end + end + + describe 'sorting' do + context 'when sorting by created' do + it 'sorts items ascending' do + expect(resolve_items(sort: 'created_asc').to_a).to eq [item1, item2] + end + + it 'sorts items descending' do + expect(resolve_items(sort: 'created_desc').to_a).to eq [item2, item1] + end + end + + context 'when sorting by title' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:item1) { create(:work_item, project: project, title: 'foo') } + let_it_be(:item2) { create(:work_item, project: project, title: 'bar') } + let_it_be(:item3) { create(:work_item, project: project, title: 'baz') } + let_it_be(:item4) { create(:work_item, project: project, title: 'Baz 2') } + + it 'sorts items ascending' do + expect(resolve_items(sort: :title_asc).to_a).to eq [item2, item3, item4, item1] + end + + it 'sorts items descending' do + expect(resolve_items(sort: :title_desc).to_a).to eq [item1, item4, item3, item2] + end + end + end + + it 'returns items user can see' do + project.add_guest(current_user) + + create(:work_item, confidential: true) + + expect(resolve_items).to contain_exactly(item1, item2) + end + + it 'batches queries that only include IIDs', :request_store do + result = batch_sync(max_queries: 7) do + [item1, item2] + .map { |item| resolve_items(iid: item.iid.to_s) } + .flat_map(&:to_a) + end + + expect(result).to contain_exactly(item1, item2) + end + + it 'finds a specific item with iids', :request_store do + result = batch_sync(max_queries: 7) do + resolve_items(iids: [item1.iid]).to_a + end + + expect(result).to contain_exactly(item1) + end + + it 'finds multiple items with iids' do + create(:work_item, project: project, author: current_user) + + expect(batch_sync { resolve_items(iids: [item1.iid, item2.iid]).to_a }) + .to contain_exactly(item1, item2) + end + + it 'finds only the items within the project we are looking at' do + another_project = create(:project) + iids = [item1, item2].map(&:iid) + + iids.each do |iid| + create(:work_item, project: another_project, iid: iid) + end + + expect(batch_sync { resolve_items(iids: iids).to_a }).to contain_exactly(item1, item2) + end + end + end + + context "when passing a non existent, batch loaded project" do + let!(:project) do + BatchLoader::GraphQL.for("non-existent-path").batch do |_fake_paths, loader, _| + loader.call("non-existent-path", nil) + end + end + + it "returns nil without breaking" do + expect(resolve_items(iids: ["don't", "break"])).to be_empty + end + end + + def resolve_items(args = {}, context = { current_user: current_user }) + resolve(described_class, obj: project, args: args, ctx: context) + end +end diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 9b6efa30ff7..4320c5460da 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -170,6 +170,65 @@ RSpec.describe Gitlab::GitalyClient::OperationService do Gitlab::Git::PreReceiveError, "something failed") end end + + context 'with a custom hook error' do + let(:stdout) { nil } + let(:stderr) { nil } + let(:error_message) { "error_message" } + let(:custom_hook_error) do + new_detailed_error( + GRPC::Core::StatusCodes::PERMISSION_DENIED, + error_message, + Gitaly::UserDeleteBranchError.new( + custom_hook: Gitaly::CustomHookError.new( + stdout: stdout, + stderr: stderr, + hook_type: Gitaly::CustomHookError::HookType::HOOK_TYPE_PRERECEIVE + ))) + end + + shared_examples 'a failed branch deletion' do + it 'raises a PreRecieveError' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_delete_branch).with(request, kind_of(Hash)) + .and_raise(custom_hook_error) + + expect { subject }.to raise_error do |error| + expect(error).to be_a(Gitlab::Git::PreReceiveError) + expect(error.message).to eq(expected_message) + expect(error.raw_message).to eq(expected_raw_message) + end + end + end + + context 'when details contain stderr' do + let(:stderr) { "something" } + let(:stdout) { "GL-HOOK-ERR: stdout is overridden by stderr" } + let(:expected_message) { error_message } + let(:expected_raw_message) { stderr } + + it_behaves_like 'a failed branch deletion' + end + + context 'when details contain stdout' do + let(:stderr) { " \n" } + let(:stdout) { "something" } + let(:expected_message) { error_message } + let(:expected_raw_message) { stdout } + + it_behaves_like 'a failed branch deletion' + end + end + + context 'with a non-detailed error' do + it 'raises a GRPC error' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_delete_branch).with(request, kind_of(Hash)) + .and_raise(GRPC::Internal.new('non-detailed error')) + + expect { subject }.to raise_error(GRPC::Internal) + end + end end describe '#user_merge_branch' do @@ -635,21 +694,6 @@ RSpec.describe Gitlab::GitalyClient::OperationService do expect(subject).to eq(squash_sha) end - context "when git_error is present" do - let(:response) do - Gitaly::UserSquashResponse.new(git_error: "something failed") - end - - it "raises a GitError exception" do - expect_any_instance_of(Gitaly::OperationService::Stub) - .to receive(:user_squash).with(request, kind_of(Hash)) - .and_return(response) - - expect { subject }.to raise_error( - Gitlab::Git::Repository::GitError, "something failed") - end - end - shared_examples '#user_squash with an error' do it 'raises a GitError exception' do expect_any_instance_of(Gitaly::OperationService::Stub) diff --git a/spec/models/concerns/integrations/has_data_fields_spec.rb b/spec/models/concerns/integrations/has_data_fields_spec.rb index b28fef571c6..374c5c33b50 100644 --- a/spec/models/concerns/integrations/has_data_fields_spec.rb +++ b/spec/models/concerns/integrations/has_data_fields_spec.rb @@ -6,84 +6,84 @@ RSpec.describe Integrations::HasDataFields do let(:url) { 'http://url.com' } let(:username) { 'username_one' } let(:properties) do - { url: url, username: username } + { url: url, username: username, jira_issue_transition_automatic: false } end shared_examples 'data fields' do describe '#arg' do - it 'returns an argument correctly' do - expect(service.url).to eq(url) + it 'returns the expected values' do + expect(integration).to have_attributes(properties) end end describe '{arg}_changed?' do it 'returns false when the property has not been assigned a new value' do - service.username = 'new_username' - service.validate - expect(service.url_changed?).to be_falsy + integration.username = 'new_username' + integration.validate + expect(integration.url_changed?).to be_falsy end it 'returns true when the property has been assigned a different value' do - service.url = "http://example.com" - service.validate - expect(service.url_changed?).to be_truthy + integration.url = "http://example.com" + integration.validate + expect(integration.url_changed?).to be_truthy end it 'returns true when the property has been assigned a different value twice' do - service.url = "http://example.com" - service.url = "http://example.com" - service.validate - expect(service.url_changed?).to be_truthy + integration.url = "http://example.com" + integration.url = "http://example.com" + integration.validate + expect(integration.url_changed?).to be_truthy end it 'returns false when the property has been re-assigned the same value' do - service.url = 'http://url.com' - service.validate - expect(service.url_changed?).to be_falsy + integration.url = 'http://url.com' + integration.validate + expect(integration.url_changed?).to be_falsy end end describe '{arg}_touched?' do it 'returns false when the property has not been assigned a new value' do - service.username = 'new_username' - service.validate - expect(service.url_changed?).to be_falsy + integration.username = 'new_username' + integration.validate + expect(integration.url_changed?).to be_falsy end it 'returns true when the property has been assigned a different value' do - service.url = "http://example.com" - service.validate - expect(service.url_changed?).to be_truthy + integration.url = "http://example.com" + integration.validate + expect(integration.url_changed?).to be_truthy end it 'returns true when the property has been assigned a different value twice' do - service.url = "http://example.com" - service.url = "http://example.com" - service.validate - expect(service.url_changed?).to be_truthy + integration.url = "http://example.com" + integration.url = "http://example.com" + integration.validate + expect(integration.url_changed?).to be_truthy end it 'returns true when the property has been re-assigned the same value' do - service.url = 'http://url.com' - expect(service.url_touched?).to be_truthy + integration.url = 'http://url.com' + expect(integration.url_touched?).to be_truthy end it 'returns false when the property has been re-assigned the same value' do - service.url = 'http://url.com' - service.validate - expect(service.url_changed?).to be_falsy + integration.url = 'http://url.com' + integration.validate + expect(integration.url_changed?).to be_falsy end end describe 'data_fields_present?' do - it 'returns true from the issue tracker service' do - expect(service.data_fields_present?).to be true + it 'returns true from the issue tracker integration' do + expect(integration.data_fields_present?).to be true end end end context 'when data are stored in data_fields' do - let(:service) do + let(:integration) do create(:jira_integration, url: url, username: username) end @@ -91,21 +91,21 @@ RSpec.describe Integrations::HasDataFields do describe '{arg}_was?' do it 'returns nil' do - service.url = 'http://example.com' - service.validate - expect(service.url_was).to be_nil + integration.url = 'http://example.com' + integration.validate + expect(integration.url_was).to be_nil end end end - context 'when service and data_fields are not persisted' do - let(:service) do + context 'when integration and data_fields are not persisted' do + let(:integration) do Integrations::Jira.new end describe 'data_fields_present?' do it 'returns true' do - expect(service.data_fields_present?).to be true + expect(integration.data_fields_present?).to be true end end end @@ -113,9 +113,7 @@ RSpec.describe Integrations::HasDataFields do context 'when data are stored in properties' do let(:integration) { create(:jira_integration, :without_properties_callback, properties: properties) } - it_behaves_like 'data fields' do - let(:service) { integration } - end + it_behaves_like 'data fields' describe '{arg}_was?' do it 'returns nil when the property has not been assigned a new value' do @@ -148,9 +146,7 @@ RSpec.describe Integrations::HasDataFields do end end - it_behaves_like 'data fields' do - let(:service) { integration } - end + it_behaves_like 'data fields' describe '{arg}_was?' do it 'returns nil' do diff --git a/spec/models/integrations/harbor_spec.rb b/spec/models/integrations/harbor_spec.rb index 4a6eb27d63a..9e3d4b524a6 100644 --- a/spec/models/integrations/harbor_spec.rb +++ b/spec/models/integrations/harbor_spec.rb @@ -67,6 +67,16 @@ RSpec.describe Integrations::Harbor do harbor_integration.update!(active: false) expect(harbor_integration.ci_variables).to match_array([]) end + + context 'with robot username' do + it 'returns username variable with $$' do + harbor_integration.username = 'robot$project+user' + + expect(harbor_integration.ci_variables).to include( + { key: 'HARBOR_USERNAME', value: 'robot$$project+user' } + ) + end + end end describe 'before_validation :reset_username_and_password' do diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb new file mode 100644 index 00000000000..66742fcbeb6 --- /dev/null +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting an work item list for a project' do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, :public, group: group) } + let_it_be(:current_user) { create(:user) } + + let_it_be(:item1) { create(:work_item, project: project, discussion_locked: true, title: 'item1') } + let_it_be(:item2) { create(:work_item, project: project, title: 'item2') } + let_it_be(:confidential_item) { create(:work_item, confidential: true, project: project, title: 'item3') } + let_it_be(:other_item) { create(:work_item) } + + let(:items_data) { graphql_data['project']['workItems']['edges'] } + let(:item_filter_params) { {} } + + let(:fields) do + <<~QUERY + edges { + node { + #{all_graphql_fields_for('workItems'.classify)} + } + } + QUERY + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('workItems', item_filter_params, fields) + ) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + context 'when the user does not have access to the item' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'returns an empty list' do + post_graphql(query) + + expect(items_data).to eq([]) + end + end + + context 'when work_items flag is disabled' do + before do + stub_feature_flags(work_items: false) + end + + it 'returns an empty list' do + post_graphql(query) + + expect(items_data).to eq([]) + end + end + + it 'returns only items visible to user' do + post_graphql(query, current_user: current_user) + + expect(item_ids).to eq([item2.to_global_id.to_s, item1.to_global_id.to_s]) + end + + context 'when the user can see confidential items' do + before do + project.add_developer(current_user) + end + + it 'returns also confidential items' do + post_graphql(query, current_user: current_user) + + expect(item_ids).to eq([confidential_item.to_global_id.to_s, item2.to_global_id.to_s, item1.to_global_id.to_s]) + end + end + + describe 'sorting and pagination' do + let(:data_path) { [:project, :work_items] } + + def pagination_query(params) + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('workItems', params, "#{page_info} nodes { id }") + ) + end + + before do + project.add_developer(current_user) + end + + context 'when sorting by title ascending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :TITLE_ASC } + let(:first_param) { 2 } + let(:all_records) { [item1, item2, confidential_item].map { |item| item.to_global_id.to_s } } + end + end + + context 'when sorting by title descending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :TITLE_DESC } + let(:first_param) { 2 } + let(:all_records) { [confidential_item, item2, item1].map { |item| item.to_global_id.to_s } } + end + end + end + + def item_ids + graphql_dig_at(items_data, :node, :id) + end +end diff --git a/spec/requests/api/integrations_spec.rb b/spec/requests/api/integrations_spec.rb index 96cc101e73a..cd9a0746581 100644 --- a/spec/requests/api/integrations_spec.rb +++ b/spec/requests/api/integrations_spec.rb @@ -55,33 +55,49 @@ RSpec.describe API::Integrations do describe "PUT /projects/:id/#{endpoint}/#{integration.dasherize}" do include_context integration + # NOTE: Some attributes are not supported for PUT requests, even though in most cases they should be. + # For some of them the problem is somewhere else, i.e. most chat integrations don't support the `*_channel` + # fields but they're incorrectly included in `#fields`. + # + # We can fix these manually, or with a generic approach like https://gitlab.com/gitlab-org/gitlab/-/issues/348208 + let(:missing_channel_attributes) { %i[push_channel issue_channel confidential_issue_channel merge_request_channel note_channel confidential_note_channel tag_push_channel pipeline_channel wiki_page_channel] } + let(:missing_attributes) do + { + datadog: %i[archive_trace_events], + discord: missing_channel_attributes + %i[branches_to_be_notified notify_only_broken_pipelines], + hangouts_chat: missing_channel_attributes + %i[notify_only_broken_pipelines], + jira: %i[issues_enabled project_key vulnerabilities_enabled vulnerabilities_issuetype], + mattermost: %i[deployment_channel labels_to_be_notified], + microsoft_teams: missing_channel_attributes, + mock_ci: %i[enable_ssl_verification], + prometheus: %i[manual_configuration], + slack: %i[alert_events alert_channel deployment_channel labels_to_be_notified], + unify_circuit: missing_channel_attributes + %i[branches_to_be_notified notify_only_broken_pipelines], + webex_teams: missing_channel_attributes + %i[branches_to_be_notified notify_only_broken_pipelines] + } + end + it "updates #{integration} settings and returns the correct fields" do - put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user), params: integration_attrs + supported_attrs = integration_attrs.without(missing_attributes.fetch(integration.to_sym, [])) - expect(response).to have_gitlab_http_status(:ok) - - current_integration = project.integrations.first - events = current_integration.event_names.empty? ? ["foo"].freeze : current_integration.event_names - query_strings = [] - events.map(&:to_sym).each do |event| - event_value = !current_integration[event] - query_strings << "#{event}=#{event_value}" - integration_attrs[event] = event_value if integration_attrs[event].present? - end - query_strings = query_strings.join('&') - - put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}?#{query_strings}", user), params: integration_attrs + put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user), params: supported_attrs expect(response).to have_gitlab_http_status(:ok) expect(json_response['slug']).to eq(dashed_integration) - events.each do |event| - next if event == "foo" - expect(project.integrations.first[event]).not_to eq(current_integration[event]), - "expected #{!current_integration[event]} for event #{event} for #{endpoint} #{current_integration.title}, got #{current_integration[event]}" + current_integration = project.integrations.first + expect(current_integration).to have_attributes(supported_attrs) + assert_correct_response_fields(json_response['properties'].keys, current_integration) + + # Flip all booleans and verify that we can set these too + flipped_attrs = supported_attrs.transform_values do |value| + [true, false].include?(value) ? !value : value end - assert_correct_response_fields(json_response['properties'].keys, current_integration) + put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user), params: flipped_attrs + + expect(response).to have_gitlab_http_status(:ok) + expect(project.integrations.first).to have_attributes(flipped_attrs) end it "returns if required fields missing" do diff --git a/spec/services/packages/maven/metadata/append_package_file_service_spec.rb b/spec/services/packages/maven/metadata/append_package_file_service_spec.rb index c406ab93630..f3a90d31158 100644 --- a/spec/services/packages/maven/metadata/append_package_file_service_spec.rb +++ b/spec/services/packages/maven/metadata/append_package_file_service_spec.rb @@ -22,6 +22,18 @@ RSpec.describe ::Packages::Maven::Metadata::AppendPackageFileService do expect_file("#{metadata_file_name}.sha256") expect_file("#{metadata_file_name}.sha512") end + + context 'with FIPS mode', :fips_mode do + it 'does not generate file_md5' do + expect { subject }.to change { package.package_files.count }.by(4) + expect(subject).to be_success + + expect_file(metadata_file_name, with_content: content, with_content_type: 'application/xml', fips: true) + expect_file("#{metadata_file_name}.sha1", fips: true) + expect_file("#{metadata_file_name}.sha256", fips: true) + expect_file("#{metadata_file_name}.sha512", fips: true) + end + end end context 'with nil content' do @@ -36,17 +48,22 @@ RSpec.describe ::Packages::Maven::Metadata::AppendPackageFileService do it_behaves_like 'returning an error service response', message: 'package is not set' end - def expect_file(file_name, with_content: nil, with_content_type: '') + def expect_file(file_name, fips: false, with_content: nil, with_content_type: '') package_file = package.package_files.recent.with_file_name(file_name).first expect(package_file.file).to be_present expect(package_file.file_name).to eq(file_name) expect(package_file.size).to be > 0 - expect(package_file.file_md5).to be_present expect(package_file.file_sha1).to be_present expect(package_file.file_sha256).to be_present expect(package_file.file.content_type).to eq(with_content_type) + if fips + expect(package_file.file_md5).not_to be_present + else + expect(package_file.file_md5).to be_present + end + if with_content expect(package_file.file.read).to eq(with_content) end diff --git a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb index a644e5d5b9c..3d3b8c2207d 100644 --- a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb +++ b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb @@ -8,8 +8,36 @@ Integration.available_integration_names.each do |integration| let(:integration_method) { Project.integration_association_name(integration) } let(:integration_klass) { Integration.integration_name_to_model(integration) } let(:integration_instance) { integration_klass.new } - let(:integration_fields) { integration_instance.fields } - let(:integration_attrs_list) { integration_fields.inject([]) {|arr, hash| arr << hash[:name].to_sym } } + + # Build a list of all attributes that an integration supports. + let(:integration_attrs_list) do + integration_fields + integration_events + custom_attributes.fetch(integration.to_sym, []) + end + + # Attributes defined as fields. + let(:integration_fields) do + integration_instance.fields.map { _1[:name].to_sym } + end + + # Attributes for configurable event triggers. + let(:integration_events) do + integration_instance.configurable_events.map { IntegrationsHelper.integration_event_field_name(_1).to_sym } + end + + # Other special cases, this list might be incomplete. + # + # Some of these won't be needed anymore after we've converted them to use the field DSL + # in https://gitlab.com/gitlab-org/gitlab/-/issues/354899. + # + # Others like `comment_on_event_disabled` are actual columns on `integrations`, maybe we should migrate + # these to fields as well. + let(:custom_attributes) do + { + jira: %i[comment_on_event_enabled jira_issue_transition_automatic jira_issue_transition_id project_key + issues_enabled vulnerabilities_enabled vulnerabilities_issuetype] + } + end + let(:integration_attrs) do integration_attrs_list.inject({}) do |hash, k| if k =~ /^(token*|.*_token|.*_key)/ @@ -32,9 +60,11 @@ Integration.available_integration_names.each do |integration| hash.merge!(k => 1234) elsif integration == 'jira' && k == :jira_issue_transition_id hash.merge!(k => '1,2,3') + elsif integration == 'jira' && k == :jira_issue_transition_automatic + hash.merge!(k => true) elsif integration == 'emails_on_push' && k == :recipients hash.merge!(k => 'foo@bar.com') - elsif integration == 'slack' || integration == 'mattermost' && k == :labels_to_be_notified_behavior + elsif (integration == 'slack' || integration == 'mattermost') && k == :labels_to_be_notified_behavior hash.merge!(k => "match_any") else hash.merge!(k => "someword") diff --git a/spec/tooling/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb index 98034eb4b0a..10afcb18a73 100644 --- a/spec/tooling/quality/test_level_spec.rb +++ b/spec/tooling/quality/test_level_spec.rb @@ -1,8 +1,33 @@ # frozen_string_literal: true +require 'spec_helper' + require_relative '../../../tooling/quality/test_level' RSpec.describe Quality::TestLevel do + describe 'TEST_LEVEL_FOLDERS constant' do + it 'all directories it refers to exists', :aggregate_failures do + ee_only_directories = %w[ + lib/ee/gitlab/background_migration + elastic + elastic_integration + replicators + ] + + described_class::TEST_LEVEL_FOLDERS.values.flatten.each do |dir| + next if ee_only_directories.include?(dir) && !Gitlab.ee? + + spec_directory = if ee_only_directories.include?(dir) + File.join('ee', 'spec', dir) + else + File.join('spec', dir) + end + + expect(File.exist?(spec_directory)).to eq(true), "#{spec_directory} does not exist!" + end + end + end + describe '#pattern' do context 'when level is all' do it 'returns a pattern' do @@ -21,7 +46,7 @@ RSpec.describe Quality::TestLevel do context 'when level is unit' do it 'returns a pattern' do expect(subject.pattern(:unit)) - .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,events,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers,tooling,component}{,/**/}*_spec.rb") + .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,events,factories,finders,frontend,graphql,haml_lint,helpers,initializers,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers,tooling,components}{,/**/}*_spec.rb") end end @@ -89,56 +114,56 @@ RSpec.describe Quality::TestLevel do context 'when level is frontend_fixture' do it 'returns a regexp' do expect(subject.regexp(:frontend_fixture)) - .to eq(%r{spec/(frontend/fixtures)}) + .to eq(%r{spec/(frontend/fixtures)/}) end end context 'when level is unit' do it 'returns a regexp' do expect(subject.regexp(:unit)) - .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|events|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers|tooling|component)}) + .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|events|factories|finders|frontend|graphql|haml_lint|helpers|initializers|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers|tooling|components)/}) end end context 'when level is migration' do it 'returns a regexp' do expect(subject.regexp(:migration)) - .to eq(%r{spec/(migrations|lib/gitlab/background_migration|lib/ee/gitlab/background_migration)}) + .to eq(%r{spec/(migrations|lib/gitlab/background_migration|lib/ee/gitlab/background_migration)/}) end end context 'when level is background_migration' do it 'returns a regexp' do expect(subject.regexp(:background_migration)) - .to eq(%r{spec/(lib/gitlab/background_migration|lib/ee/gitlab/background_migration)}) + .to eq(%r{spec/(lib/gitlab/background_migration|lib/ee/gitlab/background_migration)/}) end end context 'when level is integration' do it 'returns a regexp' do expect(subject.regexp(:integration)) - .to eq(%r{spec/(commands|controllers|mailers|requests)}) + .to eq(%r{spec/(commands|controllers|mailers|requests)/}) end end context 'when level is system' do it 'returns a regexp' do expect(subject.regexp(:system)) - .to eq(%r{spec/(features)}) + .to eq(%r{spec/(features)/}) end end context 'with a prefix' do it 'returns a regexp' do expect(described_class.new('ee/').regexp(:system)) - .to eq(%r{(ee/)spec/(features)}) + .to eq(%r{(ee/)spec/(features)/}) end end context 'with several prefixes' do it 'returns a regexp' do expect(described_class.new(['', 'ee/', 'jh/']).regexp(:system)) - .to eq(%r{(|ee/|jh/)spec/(features)}) + .to eq(%r{(|ee/|jh/)spec/(features)/}) end end diff --git a/spec/uploaders/gitlab_uploader_spec.rb b/spec/uploaders/gitlab_uploader_spec.rb index 4e661e458ad..db70441aaf5 100644 --- a/spec/uploaders/gitlab_uploader_spec.rb +++ b/spec/uploaders/gitlab_uploader_spec.rb @@ -160,4 +160,10 @@ RSpec.describe GitlabUploader do end end end + + describe '.version' do + subject { uploader_class.version } + + it { expect { subject }.to raise_error(RuntimeError, /not supported/) } + end end diff --git a/tooling/quality/test_level.rb b/tooling/quality/test_level.rb index 6e58241bccb..e6945ddb526 100644 --- a/tooling/quality/test_level.rb +++ b/tooling/quality/test_level.rb @@ -32,7 +32,6 @@ module Quality haml_lint helpers initializers - javascripts lib metrics_server models @@ -55,7 +54,7 @@ module Quality views workers tooling - component + components ], integration: %w[ commands @@ -82,6 +81,10 @@ module Quality @regexps[level] ||= Regexp.new("#{prefixes_for_regex}spec/#{folders_regex(level)}").freeze end + def legacy_factories_regexp + @legacy_factories_regexp ||= %r{spec/factories_spec.rb}.freeze + end + def level_for(file_path) case file_path # Detect migration first since some background migration tests are under @@ -97,6 +100,8 @@ module Quality :integration when regexp(:system) :system + when legacy_factories_regexp + :unit else raise UnknownTestLevelError, "Test level for #{file_path} couldn't be set. Please rename the file properly or change the test level detection regexes in #{__FILE__}." end @@ -149,11 +154,11 @@ module Quality def folders_regex(level) case level when :migration - "(#{migration_and_background_migration_folders.join('|')})" + "(#{migration_and_background_migration_folders.join('|')})/" when :all '' else - "(#{TEST_LEVEL_FOLDERS.fetch(level).join('|')})" + "(#{TEST_LEVEL_FOLDERS.fetch(level).join('|')})/" end end end diff --git a/yarn.lock b/yarn.lock index 23f4d6a3a4b..101b7b75046 100644 --- a/yarn.lock +++ b/yarn.lock @@ -987,10 +987,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== -"@graphql-eslint/eslint-plugin@3.10.3": - version "3.10.3" - resolved "https://registry.yarnpkg.com/@graphql-eslint/eslint-plugin/-/eslint-plugin-3.10.3.tgz#5e77f18b18769fe68565a76cc33a094576809b5c" - integrity sha512-QQiTBw5T28INXayJHHp8lsfiJoanJIYuTqqA0axKlSyDMtNlbfsL3Kc+a+hth29Fw7zyi9XMGAhTDmIOkpskhg== +"@graphql-eslint/eslint-plugin@3.10.4": + version "3.10.4" + resolved "https://registry.yarnpkg.com/@graphql-eslint/eslint-plugin/-/eslint-plugin-3.10.4.tgz#fcc3b54ef1d6d38e89daf848d93be2d5359fef98" + integrity sha512-+Vmi1E5uSWOgtylosHS3HYWHF+x0GMgaMOHmSfOflTE51VXnLM2tvGKMd3lpyngCEZG1wjvqdlB5rn2iwhhzqA== dependencies: "@babel/code-frame" "^7.16.7" "@graphql-tools/code-file-loader" "^7.2.14"