From 06bcbc77e472a70b8332150a941539c55953ef2b Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 16 Jun 2022 06:08:59 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- GITLAB_KAS_VERSION | 2 +- .../pajamas/checkbox_component.html.haml | 6 + app/components/pajamas/checkbox_component.rb | 56 ++++ app/components/pajamas/component.rb | 9 + .../checkbox_radio_label_with_help_text.rb | 30 ++ .../concerns/checkbox_radio_options.rb | 11 + .../pajamas/radio_component.html.haml | 5 + app/components/pajamas/radio_component.rb | 51 ++++ .../projects/releases_controller.rb | 12 +- app/helpers/diff_helper.rb | 27 +- app/models/merge_request.rb | 13 + app/models/release.rb | 2 +- .../mergeability/run_checks_service.rb | 12 +- .../groups/_group_admin_settings.html.haml | 12 +- .../20210216180416_i_search_total.yml | 0 config/routes/project.rb | 6 +- doc/api/graphql/reference/index.md | 38 +-- doc/development/api_graphql_styleguide.md | 33 ++- doc/development/fe_guide/haml.md | 47 ++- doc/development/permissions.md | 8 +- doc/user/application_security/sast/index.md | 6 +- lib/api/internal/workhorse.rb | 15 +- .../form_builders/gitlab_ui_form_builder.rb | 80 ++--- lib/gitlab/graphql/deprecation.rb | 28 +- lib/tasks/contracts.rake | 25 +- .../pajamas/checkbox_component_spec.rb | 130 +++++++++ spec/components/pajamas/component_spec.rb | 17 ++ ...heckbox_radio_label_with_help_text_spec.rb | 110 +++++++ .../concerns/checkbox_radio_options_spec.rb | 32 ++ .../pajamas/radio_component_spec.rb | 126 ++++++++ .../endpoints/{ => project}/merge_requests.js | 4 +- .../merge_request/diffs_batch.fixture.js} | 5 +- .../merge_request/diffs_metadata.fixture.js} | 7 +- .../merge_request}/discussions.fixture.js | 1 + spec/contracts/consumer/specs/diffs.spec.js | 37 --- .../consumer/specs/discussions.spec.js | 37 --- .../contracts/consumer/specs/metadata.spec.js | 37 --- .../specs/project/merge_request/show.spec.js | 112 +++++++ ...w-merge_request_diffs_batch_endpoint.json} | 6 +- ...erge_request_diffs_metadata_endpoint.json} | 8 +- ...w-merge_request_discussions_endpoint.json} | 4 +- .../merge_request/diffs_batch_helper.rb | 16 + .../merge_request/diffs_metadata_helper.rb | 16 + .../merge_request/discussions_helper.rb | 16 + spec/contracts/provider/specs/diffs_helper.rb | 16 - .../provider/specs/discussions_helper.rb | 16 - .../provider/specs/metadata_helper.rb | 16 - .../merge_request/diffs_batch_state.rb} | 2 +- .../merge_request/diffs_metadata_state.rb} | 2 +- .../merge_request}/discussions_state.rb | 2 +- .../projects/releases_controller_spec.rb | 14 +- spec/helpers/diff_helper_spec.rb | 21 ++ .../gitlab_ui_form_builder_spec.rb | 275 ++++++++++-------- spec/requests/api/internal/workhorse_spec.rb | 2 +- spec/requests/api/releases_spec.rb | 1 + .../mergeability/run_checks_service_spec.rb | 22 +- spec/support/helpers/form_builder_helpers.rb | 13 + .../components/pajamas_shared_examples.rb | 13 + ...tlab_style_deprecations_shared_examples.rb | 8 +- workhorse/internal/api/api.go | 9 +- 60 files changed, 1211 insertions(+), 476 deletions(-) create mode 100644 app/components/pajamas/checkbox_component.html.haml create mode 100644 app/components/pajamas/checkbox_component.rb create mode 100644 app/components/pajamas/concerns/checkbox_radio_label_with_help_text.rb create mode 100644 app/components/pajamas/concerns/checkbox_radio_options.rb create mode 100644 app/components/pajamas/radio_component.html.haml create mode 100644 app/components/pajamas/radio_component.rb rename config/metrics/{counts_all => counts_7d}/20210216180416_i_search_total.yml (100%) create mode 100644 spec/components/pajamas/checkbox_component_spec.rb create mode 100644 spec/components/pajamas/concerns/checkbox_radio_label_with_help_text_spec.rb create mode 100644 spec/components/pajamas/concerns/checkbox_radio_options_spec.rb create mode 100644 spec/components/pajamas/radio_component_spec.rb rename spec/contracts/consumer/endpoints/{ => project}/merge_requests.js (89%) rename spec/contracts/consumer/fixtures/{diffs.fixture.js => project/merge_request/diffs_batch.fixture.js} (98%) rename spec/contracts/consumer/fixtures/{metadata.fixture.js => project/merge_request/diffs_metadata.fixture.js} (97%) rename spec/contracts/consumer/fixtures/{ => project/merge_request}/discussions.fixture.js (99%) delete mode 100644 spec/contracts/consumer/specs/diffs.spec.js delete mode 100644 spec/contracts/consumer/specs/discussions.spec.js delete mode 100644 spec/contracts/consumer/specs/metadata.spec.js create mode 100644 spec/contracts/consumer/specs/project/merge_request/show.spec.js rename spec/contracts/contracts/{merge_request_page-merge_request_diffs_endpoint.json => project/merge_request/show/mergerequest#show-merge_request_diffs_batch_endpoint.json} (98%) rename spec/contracts/contracts/{merge_request_page-merge_request_metadata_endpoint.json => project/merge_request/show/mergerequest#show-merge_request_diffs_metadata_endpoint.json} (98%) rename spec/contracts/contracts/{merge_request_page-merge_request_discussions_endpoint.json => project/merge_request/show/mergerequest#show-merge_request_discussions_endpoint.json} (99%) create mode 100644 spec/contracts/provider/pact_helpers/project/merge_request/diffs_batch_helper.rb create mode 100644 spec/contracts/provider/pact_helpers/project/merge_request/diffs_metadata_helper.rb create mode 100644 spec/contracts/provider/pact_helpers/project/merge_request/discussions_helper.rb delete mode 100644 spec/contracts/provider/specs/diffs_helper.rb delete mode 100644 spec/contracts/provider/specs/discussions_helper.rb delete mode 100644 spec/contracts/provider/specs/metadata_helper.rb rename spec/contracts/provider/states/{diffs_state.rb => project/merge_request/diffs_batch_state.rb} (93%) rename spec/contracts/provider/states/{metadata_state.rb => project/merge_request/diffs_metadata_state.rb} (92%) rename spec/contracts/provider/states/{ => project/merge_request}/discussions_state.rb (92%) create mode 100644 spec/support/helpers/form_builder_helpers.rb create mode 100644 spec/support/shared_examples/components/pajamas_shared_examples.rb diff --git a/GITLAB_KAS_VERSION b/GITLAB_KAS_VERSION index 94188a74831..d14dfbac369 100644 --- a/GITLAB_KAS_VERSION +++ b/GITLAB_KAS_VERSION @@ -1 +1 @@ -15.0.0 +15.1.0 diff --git a/app/components/pajamas/checkbox_component.html.haml b/app/components/pajamas/checkbox_component.html.haml new file mode 100644 index 00000000000..9e3d4e68a42 --- /dev/null +++ b/app/components/pajamas/checkbox_component.html.haml @@ -0,0 +1,6 @@ +.gl-form-checkbox.custom-control.custom-checkbox + = form.check_box(method, + formatted_input_options, + checked_value, + unchecked_value) + = render_label_with_help_text diff --git a/app/components/pajamas/checkbox_component.rb b/app/components/pajamas/checkbox_component.rb new file mode 100644 index 00000000000..ae78d0453f8 --- /dev/null +++ b/app/components/pajamas/checkbox_component.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +# Renders a Pajamas compliant checkbox element +# Must be used in an instance of `ActionView::Helpers::FormBuilder` +module Pajamas + class CheckboxComponent < Pajamas::Component + include Pajamas::Concerns::CheckboxRadioLabelWithHelpText + include Pajamas::Concerns::CheckboxRadioOptions + + renders_one :label + renders_one :help_text + + def initialize( + form:, + method:, + label: nil, + help_text: nil, + label_options: {}, + checkbox_options: {}, + checked_value: '1', + unchecked_value: '0' + ) + @form = form + @method = method + @label_argument = label + @help_text_argument = help_text + @label_options = label_options + @input_options = checkbox_options + @checked_value = checked_value + @unchecked_value = unchecked_value + @value = checked_value if checkbox_options[:multiple] + end + + attr_reader( + :form, + :method, + :label_argument, + :help_text_argument, + :label_options, + :input_options, + :checked_value, + :unchecked_value, + :value + ) + + private + + def label_content + label? ? label : label_argument + end + + def help_text_content + help_text? ? help_text : help_text_argument + end + end +end diff --git a/app/components/pajamas/component.rb b/app/components/pajamas/component.rb index 70c5f5be596..3b1826a646c 100644 --- a/app/components/pajamas/component.rb +++ b/app/components/pajamas/component.rb @@ -16,5 +16,14 @@ module Pajamas default end + + # Add CSS classes and additional options to an existing options hash + # + # @param [Hash] options + # @param [Array] css_classes + # @param [Hash] additional_option + def format_options(options:, css_classes: [], additional_options: {}) + options.merge({ class: [*css_classes, options[:class]].flatten.compact }, additional_options) + end end end diff --git a/app/components/pajamas/concerns/checkbox_radio_label_with_help_text.rb b/app/components/pajamas/concerns/checkbox_radio_label_with_help_text.rb new file mode 100644 index 00000000000..4ece904fb85 --- /dev/null +++ b/app/components/pajamas/concerns/checkbox_radio_label_with_help_text.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Pajamas + module Concerns + module CheckboxRadioLabelWithHelpText + def render_label_with_help_text + form.label(method, formatted_label_options) { label_entry } + end + + private + + def label_entry + if help_text_content + content_tag(:span, label_content) + + content_tag(:p, help_text_content, class: 'help-text', data: { testid: 'pajamas-component-help-text' }) + else + content_tag(:span, label_content) + end + end + + def formatted_label_options + format_options( + options: label_options, + css_classes: ['custom-control-label'], + additional_options: { value: value } + ) + end + end + end +end diff --git a/app/components/pajamas/concerns/checkbox_radio_options.rb b/app/components/pajamas/concerns/checkbox_radio_options.rb new file mode 100644 index 00000000000..e79fdb7b601 --- /dev/null +++ b/app/components/pajamas/concerns/checkbox_radio_options.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Pajamas + module Concerns + module CheckboxRadioOptions + def formatted_input_options + format_options(options: input_options, css_classes: ['custom-control-input']) + end + end + end +end diff --git a/app/components/pajamas/radio_component.html.haml b/app/components/pajamas/radio_component.html.haml new file mode 100644 index 00000000000..6bf57b0b187 --- /dev/null +++ b/app/components/pajamas/radio_component.html.haml @@ -0,0 +1,5 @@ +.gl-form-radio.custom-control.custom-radio + = form.radio_button(method, + value, + formatted_input_options) + = render_label_with_help_text diff --git a/app/components/pajamas/radio_component.rb b/app/components/pajamas/radio_component.rb new file mode 100644 index 00000000000..52a761b9d7d --- /dev/null +++ b/app/components/pajamas/radio_component.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +# Renders a Pajamas compliant radio button element +# Must be used in an instance of `ActionView::Helpers::FormBuilder` +module Pajamas + class RadioComponent < Pajamas::Component + include Pajamas::Concerns::CheckboxRadioLabelWithHelpText + include Pajamas::Concerns::CheckboxRadioOptions + + renders_one :label + renders_one :help_text + + def initialize( + form:, + method:, + label: nil, + help_text: nil, + label_options: {}, + radio_options: {}, + value: nil + ) + @form = form + @method = method + @label_argument = label + @help_text_argument = help_text + @label_options = label_options + @input_options = radio_options + @value = value + end + + attr_reader( + :form, + :method, + :label_argument, + :help_text_argument, + :label_options, + :input_options, + :value + ) + + private + + def label_content + label? ? label : label_argument + end + + def help_text_content + help_text? ? help_text : help_text_argument + end + end +end diff --git a/app/controllers/projects/releases_controller.rb b/app/controllers/projects/releases_controller.rb index c8f876a2c30..da414d068a6 100644 --- a/app/controllers/projects/releases_controller.rb +++ b/app/controllers/projects/releases_controller.rb @@ -52,19 +52,11 @@ class Projects::ReleasesController < Projects::ApplicationController end def release - @release ||= project.releases.find_by_tag!(sanitized_tag_name) + @release ||= project.releases.find_by_tag!(params[:tag]) end def link - release.links.find_by_filepath!(sanitized_filepath) - end - - def sanitized_filepath - "/#{CGI.unescape(params[:filepath])}" - end - - def sanitized_tag_name - CGI.unescape(params[:tag]) + release.links.find_by_filepath!("/#{params[:filepath]}") end # Default order_by is 'released_at', which is set in ReleasesFinder. diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 522593dd487..71c8296ad2e 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -225,6 +225,23 @@ module DiffHelper end end + def conflicts(allow_tree_conflicts: false) + return unless options[:merge_ref_head_diff] + + conflicts_service = MergeRequests::Conflicts::ListService.new(merge_request, allow_tree_conflicts: allow_tree_conflicts) # rubocop:disable CodeReuse/ServiceClass + + return unless allow_tree_conflicts || conflicts_service.can_be_resolved_in_ui? + + conflicts_service.conflicts.files.index_by(&:path) + rescue Gitlab::Git::Conflict::Resolver::ConflictSideMissing + # This exception is raised when changes on a fork isn't present on canonical repo yet. + # We can't list conflicts until the canonical repo gets the references from the fork + # which happens asynchronously when updating MR. + # + # Return empty hash to indicate that there are no conflicts. + {} + end + private def diff_btn(title, name, selected) @@ -271,16 +288,6 @@ module DiffHelper Gitlab::CodeNavigationPath.new(merge_request.project, merge_request.diff_head_sha) end - def conflicts(allow_tree_conflicts: false) - return unless options[:merge_ref_head_diff] - - conflicts_service = MergeRequests::Conflicts::ListService.new(merge_request, allow_tree_conflicts: allow_tree_conflicts) # rubocop:disable CodeReuse/ServiceClass - - return unless allow_tree_conflicts || conflicts_service.can_be_resolved_in_ui? - - conflicts_service.conflicts.files.index_by(&:path) - end - def log_overflow_limits(diff_files:, collection_overflow:) if diff_files.any?(&:too_large?) Gitlab::Metrics.add_event(:diffs_overflow_single_file_limits) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 74d23a6e815..1a3464d05a2 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1153,6 +1153,19 @@ class MergeRequest < ApplicationRecord can_be_merged? && !should_be_rebased? end + def mergeability_checks + # We want to have the cheapest checks first in the list, that way we can + # fail fast before running the more expensive ones. + # + [ + ::MergeRequests::Mergeability::CheckOpenStatusService, + ::MergeRequests::Mergeability::CheckDraftStatusService, + ::MergeRequests::Mergeability::CheckBrokenStatusService, + ::MergeRequests::Mergeability::CheckDiscussionsStatusService, + ::MergeRequests::Mergeability::CheckCiStatusService + ] + end + # rubocop: disable CodeReuse/ServiceClass def mergeable_state?(skip_ci_check: false, skip_discussions_check: false) if Feature.enabled?(:improved_mergeability_checks, self.project) diff --git a/app/models/release.rb b/app/models/release.rb index c6c0920c4d0..655a2ba4709 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -54,7 +54,7 @@ class Release < ApplicationRecord MAX_NUMBER_TO_DISPLAY = 3 def to_param - CGI.escape(tag) + tag end def commit diff --git a/app/services/merge_requests/mergeability/run_checks_service.rb b/app/services/merge_requests/mergeability/run_checks_service.rb index fd6907c976b..1d4b96b3090 100644 --- a/app/services/merge_requests/mergeability/run_checks_service.rb +++ b/app/services/merge_requests/mergeability/run_checks_service.rb @@ -4,23 +4,13 @@ module MergeRequests class RunChecksService include Gitlab::Utils::StrongMemoize - # We want to have the cheapest checks first in the list, - # that way we can fail fast before running the more expensive ones - CHECKS = [ - CheckOpenStatusService, - CheckDraftStatusService, - CheckBrokenStatusService, - CheckDiscussionsStatusService, - CheckCiStatusService - ].freeze - def initialize(merge_request:, params:) @merge_request = merge_request @params = params end def execute - CHECKS.each_with_object([]) do |check_class, results| + merge_request.mergeability_checks.each_with_object([]) do |check_class, results| check = check_class.new(merge_request: merge_request, params: params) next if check.skip? diff --git a/app/views/groups/_group_admin_settings.html.haml b/app/views/groups/_group_admin_settings.html.haml index 785ca71b371..0a170ebdb24 100644 --- a/app/views/groups/_group_admin_settings.html.haml +++ b/app/views/groups/_group_admin_settings.html.haml @@ -2,12 +2,12 @@ .col-sm-2.col-form-label.pt-0 = f.label :lfs_enabled, _('Large File Storage') .col-sm-10 - - label = _('Allow projects within this group to use Git LFS') - - help_link = link_to sprite_icon('question-o'), help_page_path('topics/git/lfs/index'), class: 'gl-ml-2' - = f.gitlab_ui_checkbox_component :lfs_enabled, - '%{label}%{help_link}'.html_safe % { label: label, help_link: help_link }, - help_text: _('This setting can be overridden in each project.'), - checkbox_options: { checked: @group.lfs_enabled? } + = f.gitlab_ui_checkbox_component :lfs_enabled, checkbox_options: { checked: @group.lfs_enabled? } do |c| + = c.label do + = _('Allow projects within this group to use Git LFS') + = link_to sprite_icon('question-o'), help_page_path('topics/git/lfs/index'), class: 'gl-ml-2' + = c.help_text do + = _('This setting can be overridden in each project.') .form-group.row .col-sm-2.col-form-label = f.label s_('ProjectCreationLevel|Allowed to create projects') diff --git a/config/metrics/counts_all/20210216180416_i_search_total.yml b/config/metrics/counts_7d/20210216180416_i_search_total.yml similarity index 100% rename from config/metrics/counts_all/20210216180416_i_search_total.yml rename to config/metrics/counts_7d/20210216180416_i_search_total.yml diff --git a/config/routes/project.rb b/config/routes/project.rb index a4a3e55f711..afc06780471 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -241,7 +241,9 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end end - resources :releases, only: [:index, :new, :show, :edit], param: :tag, constraints: { tag: %r{[^/]+} } do + get 'releases/permalink/latest(/)(*suffix_path)', to: 'releases#latest_permalink', as: :latest_release_permalink, format: false + + resources :releases, only: [:index, :new, :show, :edit], param: :tag, constraints: { tag: %r{[^\\]+} } do member do get :downloads, path: 'downloads/*filepath', format: false scope module: :releases do @@ -250,8 +252,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end end - get 'releases/permalink/latest(/)(*suffix_path)', to: 'releases#latest_permalink', as: :latest_release_permalink, format: false - resources :logs, only: [:index] do collection do get :k8s diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 1da6beaee9a..15cf2f4d6c0 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -559,8 +559,8 @@ Returns [`Vulnerability`](#vulnerability). Find a work item. Returns `null` if `work_items` feature flag is disabled. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Returns [`WorkItem`](#workitem). @@ -5413,8 +5413,8 @@ Input type: `VulnerabilityRevertToDetectedInput` Creates a work item. Available only when feature flag `work_items` is enabled. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Input type: `WorkItemCreateInput` @@ -5441,8 +5441,8 @@ Input type: `WorkItemCreateInput` Creates a work item from a task in another work item's description. Available only when feature flag `work_items` is enabled. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Input type: `WorkItemCreateFromTaskInput` @@ -5468,8 +5468,8 @@ Input type: `WorkItemCreateFromTaskInput` Deletes a work item. Available only when feature flag `work_items` is enabled. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Input type: `WorkItemDeleteInput` @@ -5493,8 +5493,8 @@ Input type: `WorkItemDeleteInput` Deletes a task in a work item's description. Available only when feature flag `work_items` is enabled. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Input type: `WorkItemDeleteTaskInput` @@ -5520,8 +5520,8 @@ Input type: `WorkItemDeleteTaskInput` Updates a work item by Global ID. Available only when feature flag `work_items` is enabled. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Input type: `WorkItemUpdateInput` @@ -5547,8 +5547,8 @@ Input type: `WorkItemUpdateInput` Updates a work item's task by Global ID. Available only when feature flag `work_items` is enabled. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Input type: `WorkItemUpdateTaskInput` @@ -5574,8 +5574,8 @@ Input type: `WorkItemUpdateTaskInput` Updates the attributes of a work item's widgets by global ID. Available only when feature flag `work_items` is enabled. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Input type: `WorkItemUpdateWidgetsInput` @@ -9806,7 +9806,7 @@ Represents the total number of issues and their weights for a particular day. | `shortSha` | [`String`](#string) | First eight characters of the runner's token used to authenticate new job requests. Used as the runner's unique ID. | | `tagList` | [`[String!]`](#string) | Tags associated with the runner. | | `tokenExpiresAt` | [`Time`](#time) | Runner token expiration time. | -| `upgradeStatus` **{warning-solid}** | [`CiRunnerUpgradeStatusType`](#cirunnerupgradestatustype) | **Deprecated** in 14.10. This feature is in Alpha, and can be removed or changed at any point. | +| `upgradeStatus` **{warning-solid}** | [`CiRunnerUpgradeStatusType`](#cirunnerupgradestatustype) | **Introduced** in 14.10. This feature is in Alpha. It can be changed or removed at any time. | | `userPermissions` | [`RunnerPermissions!`](#runnerpermissions) | Permissions for the current user on the resource. | | `version` | [`String`](#string) | Version of the runner. | @@ -15894,8 +15894,8 @@ four standard [pagination arguments](#connection-pagination-arguments): Work items of the project. WARNING: -**Deprecated** in 15.1. -This feature is in Alpha, and can be removed or changed at any point. +**Introduced** in 15.1. +This feature is in Alpha. It can be changed or removed at any time. Returns [`WorkItemConnection`](#workitemconnection). diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 1660d3d1493..0ad80508bb9 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -94,11 +94,16 @@ discussed in [Nullable fields](#nullable-fields). - Lowering the global limits for query complexity and depth. - Anything else that can result in queries hitting a limit that previously was allowed. -Fields that use the [`feature_flag` property](#feature_flag-property) and the flag is disabled by default are exempt -from the deprecation process, and can be removed at any time without notice. - See the [deprecating schema items](#deprecating-schema-items) section for how to deprecate items. +### Breaking change exemptions + +Two scenarios exist where schema items are exempt from the deprecation process, +and can be removed or changed at any time without notice. These are schema items that either: + +- Use the [`feature_flag` property](#feature_flag-property) _and_ the flag is disabled by default. +- Are [marked as alpha](#marking-schema-items-as-alpha). + ## Global IDs The GitLab GraphQL API uses Global IDs (i.e: `"gid://gitlab/MyObject/123"`) @@ -718,6 +723,28 @@ aware of the support. The documentation will mention that the old Global ID style is now deprecated. +## Marking schema items as Alpha + +Fields, arguments, enum values, and mutations can be marked as being in +[alpha](https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga). + +An item marked as "alpha" is exempt from the deprecation process and can be removed +at any time without notice. + +This leverages GraphQL deprecations to cause the schema item to appear as deprecated, +and will be described as being in "alpha" in our generated docs and its GraphQL description. + +To mark a schema item as being in "alpha", use the `deprecated:` keyword with `reason: :alpha`. +You must provide the `milestone:` that introduced the alpha item. + +For example: + +```ruby +field :token, GraphQL::Types::String, null: true, + deprecated: { reason: :alpha, milestone: '10.0' }, + description: 'Token for login.' +``` + ## Enums GitLab GraphQL enums are defined in `app/graphql/types`. When defining new enums, the diff --git a/doc/development/fe_guide/haml.md b/doc/development/fe_guide/haml.md index 803bb89118c..00096ce7fdc 100644 --- a/doc/development/fe_guide/haml.md +++ b/doc/development/fe_guide/haml.md @@ -39,6 +39,15 @@ For example: %span = s_('GroupSettings|Prevent members from sending invitations to groups outside of %{group} and its subgroups.').html_safe % { group: link_to_group(@group) } %p.help-text= prevent_sharing_groups_outside_hierarchy_help_text(@group) + + .form-group.gl-mb-3 + .gl-form-checkbox.custom-control.custom-checkbox + = f.check_box :lfs_enabled, checked: @group.lfs_enabled?, class: 'custom-control-input' + = f.label :lfs_enabled, class: 'custom-control-label' do + %span + = _('Allow projects within this group to use Git LFS') + = link_to sprite_icon('question-o'), help_page_path('topics/git/lfs/index') + %p.help-text= _('This setting can be overridden in each project.') ``` - After: @@ -50,6 +59,14 @@ For example: s_('GroupSettings|Prevent members from sending invitations to groups outside of %{group} and its subgroups.').html_safe % { group: link_to_group(@group) }, help_text: prevent_sharing_groups_outside_hierarchy_help_text(@group), checkbox_options: { disabled: !can_change_prevent_sharing_groups_outside_hierarchy?(@group) } + + .form-group.gl-mb-3 + = f.gitlab_ui_checkbox_component :lfs_enabled, checkbox_options: { checked: @group.lfs_enabled? } do |c| + = c.label do + = _('Allow projects within this group to use Git LFS') + = link_to sprite_icon('question-o'), help_page_path('topics/git/lfs/index') + = c.help_text do + = _('This setting can be overridden in each project.') ``` ### Available components @@ -67,16 +84,27 @@ Currently only the listed components are available but more components are plann [GitLab UI Docs](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/base-form-form-checkbox--default) +##### Arguments + | Argument | Description | Type | Required (default value) | |---|---|---|---| | `method` | Attribute on the object passed to `gitlab_ui_form_for`. | `Symbol` | `true` | -| `label` | Checkbox label. | `String` | `true` | -| `help_text` | Help text displayed below the checkbox. | `String` | `false` (`nil`) | +| `label` | Checkbox label. `label` slot can be used instead of this argument if HTML is needed. | `String` | `false` (`nil`) | +| `help_text` | Help text displayed below the checkbox. `help_text` slot can be used instead of this argument if HTML is needed. | `String` | `false` (`nil`) | | `checkbox_options` | Options that are passed to [Rails `check_box` method](https://api.rubyonrails.org/classes/ActionView/Helpers/FormBuilder.html#method-i-check_box). | `Hash` | `false` (`{}`) | | `checked_value` | Value when checkbox is checked. | `String` | `false` (`'1'`) | | `unchecked_value` | Value when checkbox is unchecked. | `String` | `false` (`'0'`) | | `label_options` | Options that are passed to [Rails `label` method](https://api.rubyonrails.org/classes/ActionView/Helpers/FormBuilder.html#method-i-label). | `Hash` | `false` (`{}`) | +##### Slots + +This component supports [ViewComponent slots](https://viewcomponent.org/guide/slots.html). + +| Slot | Description +|---|---| +| `label` | Checkbox label content. This slot can be used instead of the `label` argument. | +| `help_text` | Help text content displayed below the checkbox. This slot can be used instead of the `help_text` argument. | + #### gitlab_ui_radio_component @@ -85,11 +113,22 @@ Currently only the listed components are available but more components are plann [GitLab UI Docs](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/base-form-form-radio--default) +##### Arguments + | Argument | Description | Type | Required (default value) | |---|---|---|---| | `method` | Attribute on the object passed to `gitlab_ui_form_for`. | `Symbol` | `true` | | `value` | The value of the radio tag. | `Symbol` | `true` | -| `label` | Radio label. | `String` | `true` | -| `help_text` | Help text displayed below the radio button. | `String` | `false` (`nil`) | +| `label` | Radio label. `label` slot can be used instead of this argument if HTML content is needed inside the label. | `String` | `false` (`nil`) | +| `help_text` | Help text displayed below the radio button. `help_text` slot can be used instead of this argument if HTML content is needed inside the help text. | `String` | `false` (`nil`) | | `radio_options` | Options that are passed to [Rails `radio_button` method](https://api.rubyonrails.org/classes/ActionView/Helpers/FormBuilder.html#method-i-radio_button). | `Hash` | `false` (`{}`) | | `label_options` | Options that are passed to [Rails `label` method](https://api.rubyonrails.org/classes/ActionView/Helpers/FormBuilder.html#method-i-label). | `Hash` | `false` (`{}`) | + +##### Slots + +This component supports [ViewComponent slots](https://viewcomponent.org/guide/slots.html). + +| Slot | Description +|---|---| +| `label` | Checkbox label content. This slot can be used instead of the `label` argument. | +| `help_text` | Help text content displayed below the radio button. This slot can be used instead of the `help_text` argument. | diff --git a/doc/development/permissions.md b/doc/development/permissions.md index f3818e92fec..ed95456c4f9 100644 --- a/doc/development/permissions.md +++ b/doc/development/permissions.md @@ -95,11 +95,9 @@ can still view the groups and their entities (like epics). Project membership (where the group membership is already taken into account) is stored in the `project_authorizations` table. -WARNING: -Due to [an issue](https://gitlab.com/gitlab-org/gitlab/-/issues/219299), -projects in personal namespace do not show owner (`50`) permission in -`project_authorizations` table. Note however that [`user.owned_projects`](https://gitlab.com/gitlab-org/gitlab/-/blob/0d63823b122b11abd2492bca47cc26858eee713d/app/models/user.rb#L906-916) -is calculated properly. +NOTE: +In [GitLab 14.9](https://gitlab.com/gitlab-org/gitlab/-/issues/351211) and later, projects in personal namespaces have a maximum role of Owner. +Because of a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/219299) in GitLab 14.8 and earlier, projects in personal namespaces have a maximum role of Maintainer. ### Confidential issues diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index d86fa2af48a..d4dd8059c6a 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -697,6 +697,10 @@ rules: Vulnerabilities that have been detected and are false positives will be flagged as false positives in the security dashboard. +False positive detection is available in a subset of the [supported languages](#supported-languages-and-frameworks) and [analyzers](analyzers.md): + +- Ruby, in the Brakeman-based analyzer + ### Advanced vulnerability tracking **(ULTIMATE)** > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5144) in GitLab 14.2. @@ -715,7 +719,7 @@ Advanced vulnerability tracking is available in a subset of the [supported langu - Java, in the Semgrep-based analyzer only - JavaScript, in the Semgrep-based analyzer only - Python, in the Semgrep-based analyzer only -- Ruby, in the Brakeman-based analyzers +- Ruby, in the Brakeman-based analyzer Support for more languages and analyzers is tracked in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/5144). diff --git a/lib/api/internal/workhorse.rb b/lib/api/internal/workhorse.rb index 6a7bc335fec..910cf52bc3b 100644 --- a/lib/api/internal/workhorse.rb +++ b/lib/api/internal/workhorse.rb @@ -10,11 +10,22 @@ module API content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE end + helpers do + def request_authenticated? + authenticator = Gitlab::Auth::RequestAuthenticator.new(request) + return true if authenticator.find_authenticated_requester([:api]) + + # Look up user from warden, ignoring the absence of a CSRF token. For + # web users the CSRF token can be in the POST form data but Workhorse + # does not propagate the form data to us. + !!request.env['warden']&.authenticate + end + end + namespace 'internal' do namespace 'workhorse' do post 'authorize_upload' do - authenticator = Gitlab::Auth::RequestAuthenticator.new(request) - unauthorized! unless authenticator.find_authenticated_requester([:api]) + unauthorized! unless request_authenticated? status 200 { TempPath: File.join(::Gitlab.config.uploads.storage_path, 'uploads/tmp') } diff --git a/lib/gitlab/form_builders/gitlab_ui_form_builder.rb b/lib/gitlab/form_builders/gitlab_ui_form_builder.rb index e8e87a864cc..9174ca165cd 100644 --- a/lib/gitlab/form_builders/gitlab_ui_form_builder.rb +++ b/lib/gitlab/form_builders/gitlab_ui_form_builder.rb @@ -5,76 +5,50 @@ module Gitlab class GitlabUiFormBuilder < ActionView::Helpers::FormBuilder def gitlab_ui_checkbox_component( method, - label, + label = nil, help_text: nil, checkbox_options: {}, checked_value: '1', unchecked_value: '0', - label_options: {} + label_options: {}, + &block ) - @template.content_tag( - :div, - class: 'gl-form-checkbox custom-control custom-checkbox' - ) do - value = checkbox_options[:multiple] ? checked_value : nil - - @template.check_box( - @object_name, - method, - format_options(checkbox_options, ['custom-control-input']), - checked_value, - unchecked_value - ) + generic_label(method, label, label_options, help_text: help_text, value: value) - end + Pajamas::CheckboxComponent.new( + form: self, + method: method, + label: label, + help_text: help_text, + checkbox_options: format_options(checkbox_options), + checked_value: checked_value, + unchecked_value: unchecked_value, + label_options: format_options(label_options) + ).render_in(@template, &block) end def gitlab_ui_radio_component( method, value, - label, + label = nil, help_text: nil, radio_options: {}, - label_options: {} + label_options: {}, + &block ) - @template.content_tag( - :div, - class: 'gl-form-radio custom-control custom-radio' - ) do - @template.radio_button( - @object_name, - method, - value, - format_options(radio_options, ['custom-control-input']) - ) + generic_label(method, label, label_options, help_text: help_text, value: value) - end + Pajamas::RadioComponent.new( + form: self, + method: method, + value: value, + label: label, + help_text: help_text, + radio_options: format_options(radio_options), + label_options: format_options(label_options) + ).render_in(@template, &block) end private - def generic_label(method, label, label_options, help_text: nil, value: nil) - @template.label( - @object_name, method, format_options(label_options.merge({ value: value }), ['custom-control-label']) - ) do - if help_text - @template.content_tag( - :span, - label - ) + - @template.content_tag( - :p, - help_text, - class: 'help-text' - ) - else - label - end - end - end - - def format_options(options, classes) - classes << options[:class] - - objectify_options(options.merge({ class: classes.flatten.compact })) + def format_options(options) + objectify_options(options) end end end diff --git a/lib/gitlab/graphql/deprecation.rb b/lib/gitlab/graphql/deprecation.rb index 3335e511714..d30751fe46e 100644 --- a/lib/gitlab/graphql/deprecation.rb +++ b/lib/gitlab/graphql/deprecation.rb @@ -3,9 +3,12 @@ module Gitlab module Graphql class Deprecation + REASON_RENAMED = :renamed + REASON_ALPHA = :alpha + REASONS = { - renamed: 'This was renamed.', - alpha: 'This feature is in Alpha, and can be removed or changed at any point.' + REASON_RENAMED => 'This was renamed.', + REASON_ALPHA => 'This feature is in Alpha. It can be changed or removed at any time.' }.freeze include ActiveModel::Validations @@ -39,7 +42,7 @@ module Gitlab def markdown(context: :inline) parts = [ - "#{deprecated_in(format: :markdown)}.", + "#{changed_in_milestone(format: :markdown)}.", reason_text, replacement_markdown.then { |r| "Use: #{r}." if r } ].compact @@ -77,7 +80,7 @@ module Gitlab [ reason_text, replacement && "Please use `#{replacement}`.", - "#{deprecated_in}." + "#{changed_in_milestone}." ].compact.join(' ') end @@ -107,15 +110,24 @@ module Gitlab end def description_suffix - " #{deprecated_in}: #{reason_text}" + " #{changed_in_milestone}: #{reason_text}" end - def deprecated_in(format: :plain) + # Returns 'Deprecated in ' for proper deprecations. + # Retruns 'Introduced in ' for :alpha deprecations. + # Formatted to markdown or plain format. + def changed_in_milestone(format: :plain) + verb = if reason == REASON_ALPHA + 'Introduced' + else + 'Deprecated' + end + case format when :plain - "Deprecated in #{milestone}" + "#{verb} in #{milestone}" when :markdown - "**Deprecated** in #{milestone}" + "**#{verb}** in #{milestone}" end end end diff --git a/lib/tasks/contracts.rake b/lib/tasks/contracts.rake index 75c350f5be5..6bb7f30ad57 100644 --- a/lib/tasks/contracts.rake +++ b/lib/tasks/contracts.rake @@ -10,24 +10,25 @@ provider = File.expand_path('provider', contracts) # rubocop:disable Rails/RakeEnvironment namespace :contracts do namespace :mr do - Pact::VerificationTask.new(:metadata) do |pact| + Pact::VerificationTask.new(:diffs_batch) do |pact| pact.uri( - "#{contracts}/contracts/merge_request_page-merge_request_metadata_endpoint.json", - pact_helper: "#{provider}/specs/metadata_helper.rb" + "#{contracts}/contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_batch_endpoint.json", + pact_helper: "#{provider}/pact_helpers/project/merge_request/diffs_batch_helper.rb" + ) + end + + Pact::VerificationTask.new(:diffs_metadata) do |pact| + pact.uri( + "#{contracts}/contracts/project/merge_request/show/" \ + "mergerequest#show-merge_request_diffs_metadata_endpoint.json", + pact_helper: "#{provider}/pact_helpers/project/merge_request/diffs_metadata_helper.rb" ) end Pact::VerificationTask.new(:discussions) do |pact| pact.uri( - "#{contracts}/contracts/merge_request_page-merge_request_discussions_endpoint.json", - pact_helper: "#{provider}/specs/discussions_helper.rb" - ) - end - - Pact::VerificationTask.new(:diffs) do |pact| - pact.uri( - "#{contracts}/contracts/merge_request_page-merge_request_diffs_endpoint.json", - pact_helper: "#{provider}/specs/diffs_helper.rb" + "#{contracts}/contracts/project/merge_request/show/mergerequest#show-merge_request_discussions_endpoint.json", + pact_helper: "#{provider}/pact_helpers/project/merge_request/discussions_helper.rb" ) end diff --git a/spec/components/pajamas/checkbox_component_spec.rb b/spec/components/pajamas/checkbox_component_spec.rb new file mode 100644 index 00000000000..b2f3a84fbfe --- /dev/null +++ b/spec/components/pajamas/checkbox_component_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true +require "spec_helper" + +RSpec.describe Pajamas::CheckboxComponent, :aggregate_failures, type: :component do + include FormBuilderHelpers + + let_it_be(:method) { :view_diffs_file_by_file } + let_it_be(:label) { "Show one file at a time on merge request's Changes tab" } + let_it_be(:help_text) { 'Instead of all the files changed, show only one file at a time.' } + + RSpec.shared_examples 'it renders unchecked checkbox with value of `1`' do + it 'renders unchecked checkbox with value of `1`' do + expect(rendered_component).to have_unchecked_field(label, with: '1') + end + end + + context 'with default options' do + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method, + label: label + ) + ) + end + end + + include_examples 'it renders unchecked checkbox with value of `1`' + include_examples 'it does not render help text' + + it 'renders hidden input with value of `0`' do + expect(rendered_component).to have_field('user[view_diffs_file_by_file]', type: 'hidden', with: '0') + end + end + + context 'with custom options' do + let_it_be(:checked_value) { 'yes' } + let_it_be(:unchecked_value) { 'no' } + let_it_be(:checkbox_options) { { class: 'checkbox-foo-bar', checked: true } } + let_it_be(:label_options) { { class: 'label-foo-bar' } } + + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method, + label: label, + help_text: help_text, + checked_value: checked_value, + unchecked_value: unchecked_value, + checkbox_options: checkbox_options, + label_options: label_options + ) + ) + end + end + + include_examples 'it renders help text' + + it 'renders checked checkbox with value of `yes`' do + expect(rendered_component).to have_checked_field(label, with: checked_value, class: checkbox_options[:class]) + end + + it 'adds CSS class to label' do + expect(rendered_component).to have_selector('label.label-foo-bar') + end + + it 'renders hidden input with value of `no`' do + expect(rendered_component).to have_field('user[view_diffs_file_by_file]', type: 'hidden', with: unchecked_value) + end + end + + context 'with `label` slot' do + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method + ) + ) do |c| + c.label { label } + end + end + end + + include_examples 'it renders unchecked checkbox with value of `1`' + end + + context 'with `help_text` slot' do + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method, + label: label + ) + ) do |c| + c.help_text { help_text } + end + end + end + + include_examples 'it renders unchecked checkbox with value of `1`' + include_examples 'it renders help text' + end + + context 'with `label` and `help_text` slots' do + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method + ) + ) do |c| + c.label { label } + c.help_text { help_text } + end + end + end + + include_examples 'it renders unchecked checkbox with value of `1`' + include_examples 'it renders help text' + end +end diff --git a/spec/components/pajamas/component_spec.rb b/spec/components/pajamas/component_spec.rb index 96f6b43bac1..7385519b468 100644 --- a/spec/components/pajamas/component_spec.rb +++ b/spec/components/pajamas/component_spec.rb @@ -23,4 +23,21 @@ RSpec.describe Pajamas::Component do expect(value).to eq('something') end end + + describe '#format_options' do + it 'merges CSS classes and additional options' do + expect( + subject.send( + :format_options, + options: { foo: 'bar', class: 'gl-display-flex gl-py-5' }, + css_classes: %w(gl-px-5 gl-mt-5), + additional_options: { baz: 'bax' } + ) + ).to match({ + foo: 'bar', + baz: 'bax', + class: ['gl-px-5', 'gl-mt-5', 'gl-display-flex gl-py-5'] + }) + end + end end diff --git a/spec/components/pajamas/concerns/checkbox_radio_label_with_help_text_spec.rb b/spec/components/pajamas/concerns/checkbox_radio_label_with_help_text_spec.rb new file mode 100644 index 00000000000..7a792592b3c --- /dev/null +++ b/spec/components/pajamas/concerns/checkbox_radio_label_with_help_text_spec.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true +require "spec_helper" + +RSpec.describe Pajamas::Concerns::CheckboxRadioLabelWithHelpText do + let(:form) { instance_double('ActionView::Helpers::FormBuilder') } + let(:component_class) do + Class.new do + attr_reader( + :form, + :method, + :label_argument, + :help_text_argument, + :label_options, + :input_options, + :value + ) + + def initialize( + form:, + method:, + label: nil, + help_text: nil, + label_options: {}, + radio_options: {}, + value: nil + ) + @form = form + @method = method + @label_argument = label + @help_text_argument = help_text + @label_options = label_options + @input_options = radio_options + @value = value + end + + def label_content + @label_argument + end + + def help_text_content + @help_text_argument + end + + def format_options(options:, css_classes: [], additional_options: {}) + {} + end + + include Pajamas::Concerns::CheckboxRadioLabelWithHelpText + include ActionView::Helpers::TagHelper + end + end + + let_it_be(:method) { 'username' } + let_it_be(:label_options) { { class: 'foo-bar' } } + let_it_be(:value) { 'Foo bar' } + + describe '#render_label_with_help_text' do + it 'calls `#format_options` with correct arguments' do + allow(form).to receive(:label) + + component = component_class.new(form: form, method: method, label_options: label_options, value: value) + + expect(component).to receive(:format_options).with( + options: label_options, + css_classes: ['custom-control-label'], + additional_options: { value: value } + ) + + component.render_label_with_help_text + end + + context 'when `help_text` argument is passed' do + it 'calls `form.label` with `label` and `help_text` arguments used in the block' do + component = component_class.new( + form: form, + method: method, + label: 'Label argument', + help_text: 'Help text argument' + ) + + expected_label_entry = 'Label argument

Help text argument

' + + expect(form).to receive(:label).with(method, {}) do |&block| + expect(block.call).to eq(expected_label_entry) + end + + component.render_label_with_help_text + end + end + + context 'when `help_text` argument is not passed' do + it 'calls `form.label` with `label` argument used in the block' do + component = component_class.new( + form: form, + method: method, + label: 'Label argument' + ) + + expected_label_entry = 'Label argument' + + expect(form).to receive(:label).with(method, {}) do |&block| + expect(block.call).to eq(expected_label_entry) + end + + component.render_label_with_help_text + end + end + end +end diff --git a/spec/components/pajamas/concerns/checkbox_radio_options_spec.rb b/spec/components/pajamas/concerns/checkbox_radio_options_spec.rb new file mode 100644 index 00000000000..3eb888e5f3b --- /dev/null +++ b/spec/components/pajamas/concerns/checkbox_radio_options_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true +require "spec_helper" + +RSpec.describe Pajamas::Concerns::CheckboxRadioOptions do + let(:component_class) do + Class.new do + include Pajamas::Concerns::CheckboxRadioOptions + + attr_reader(:input_options) + + def initialize(input_options: {}) + @input_options = input_options + end + + def format_options(options:, css_classes: [], additional_options: {}) + {} + end + end + end + + describe '#formatted_input_options' do + let_it_be(:input_options) { { class: 'foo-bar' } } + + it 'calls `#format_options` with correct arguments' do + component = component_class.new(input_options: input_options) + + expect(component).to receive(:format_options).with(options: input_options, css_classes: ['custom-control-input']) + + component.formatted_input_options + end + end +end diff --git a/spec/components/pajamas/radio_component_spec.rb b/spec/components/pajamas/radio_component_spec.rb new file mode 100644 index 00000000000..3885d101c7a --- /dev/null +++ b/spec/components/pajamas/radio_component_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true +require "spec_helper" + +RSpec.describe Pajamas::RadioComponent, :aggregate_failures, type: :component do + include FormBuilderHelpers + + let_it_be(:method) { :access_level } + let_it_be(:label) { "Access Level" } + let_it_be(:value) { :regular } + let_it_be(:help_text) do + 'Administrators have access to all groups, projects, and users and can manage all features in this installation' + end + + RSpec.shared_examples 'it renders unchecked radio' do + it 'renders unchecked radio' do + expect(rendered_component).to have_unchecked_field(label) + end + end + + context 'with default options' do + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method, + value: value, + label: label + ) + ) + end + end + + include_examples 'it renders unchecked radio' + include_examples 'it does not render help text' + end + + context 'with custom options' do + let_it_be(:radio_options) { { class: 'radio-foo-bar', checked: true } } + let_it_be(:label_options) { { class: 'label-foo-bar' } } + + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method, + value: method, + label: label, + help_text: help_text, + radio_options: radio_options, + label_options: label_options + ) + ) + end + end + + include_examples 'it renders help text' + + it 'renders checked radio' do + expect(rendered_component).to have_checked_field(label, class: radio_options[:class]) + end + + it 'adds CSS class to label' do + expect(rendered_component).to have_selector('label.label-foo-bar') + end + end + + context 'with `label` slot' do + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method, + value: value + ) + ) do |c| + c.label { label } + end + end + end + + include_examples 'it renders unchecked radio' + end + + context 'with `help_text` slot' do + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method, + value: value, + label: label + ) + ) do |c| + c.help_text { help_text } + end + end + end + + include_examples 'it renders unchecked radio' + include_examples 'it renders help text' + end + + context 'with `label` and `help_text` slots' do + before do + fake_form_for do |form| + render_inline( + described_class.new( + form: form, + method: method, + value: value + ) + ) do |c| + c.label { label } + c.help_text { help_text } + end + end + end + + include_examples 'it renders unchecked radio' + include_examples 'it renders help text' + end +end diff --git a/spec/contracts/consumer/endpoints/merge_requests.js b/spec/contracts/consumer/endpoints/project/merge_requests.js similarity index 89% rename from spec/contracts/consumer/endpoints/merge_requests.js rename to spec/contracts/consumer/endpoints/project/merge_requests.js index ae4d5544df6..38773e5fb10 100644 --- a/spec/contracts/consumer/endpoints/merge_requests.js +++ b/spec/contracts/consumer/endpoints/project/merge_requests.js @@ -1,6 +1,6 @@ import { request } from 'axios'; -export function getMetadata(endpoint) { +export function getDiffsMetadata(endpoint) { const { url } = endpoint; return request({ @@ -22,7 +22,7 @@ export function getDiscussions(endpoint) { }).then((response) => response.data); } -export function getDiffs(endpoint) { +export function getDiffsBatch(endpoint) { const { url } = endpoint; return request({ diff --git a/spec/contracts/consumer/fixtures/diffs.fixture.js b/spec/contracts/consumer/fixtures/project/merge_request/diffs_batch.fixture.js similarity index 98% rename from spec/contracts/consumer/fixtures/diffs.fixture.js rename to spec/contracts/consumer/fixtures/project/merge_request/diffs_batch.fixture.js index cc2c054b08f..b53e4bb335d 100644 --- a/spec/contracts/consumer/fixtures/diffs.fixture.js +++ b/spec/contracts/consumer/fixtures/project/merge_request/diffs_batch.fixture.js @@ -62,7 +62,7 @@ const body = { }, }; -const Diffs = { +const DiffsBatch = { body: Matchers.extractPayload(body), success: { @@ -86,5 +86,6 @@ const Diffs = { }, }; -export { Diffs }; +export { DiffsBatch }; + /* eslint-enable @gitlab/require-i18n-strings */ diff --git a/spec/contracts/consumer/fixtures/metadata.fixture.js b/spec/contracts/consumer/fixtures/project/merge_request/diffs_metadata.fixture.js similarity index 97% rename from spec/contracts/consumer/fixtures/metadata.fixture.js rename to spec/contracts/consumer/fixtures/project/merge_request/diffs_metadata.fixture.js index c19ca2175b3..39dbcf78ee7 100644 --- a/spec/contracts/consumer/fixtures/metadata.fixture.js +++ b/spec/contracts/consumer/fixtures/project/merge_request/diffs_metadata.fixture.js @@ -70,7 +70,7 @@ const body = { project_name: Matchers.string('contract-testing'), }; -const Metadata = { +const DiffsMetadata = { body: Matchers.extractPayload(body), success: { @@ -82,7 +82,7 @@ const Metadata = { }, request: { - uponReceiving: 'a request for Metadata', + uponReceiving: 'a request for Diffs Metadata', withRequest: { method: 'GET', path: '/gitlab-org/gitlab-qa/-/merge_requests/1/diffs_metadata.json', @@ -93,5 +93,6 @@ const Metadata = { }, }; -export { Metadata }; +export { DiffsMetadata }; + /* eslint-enable @gitlab/require-i18n-strings */ diff --git a/spec/contracts/consumer/fixtures/discussions.fixture.js b/spec/contracts/consumer/fixtures/project/merge_request/discussions.fixture.js similarity index 99% rename from spec/contracts/consumer/fixtures/discussions.fixture.js rename to spec/contracts/consumer/fixtures/project/merge_request/discussions.fixture.js index 26f1d65f663..af0962a01cb 100644 --- a/spec/contracts/consumer/fixtures/discussions.fixture.js +++ b/spec/contracts/consumer/fixtures/project/merge_request/discussions.fixture.js @@ -83,4 +83,5 @@ const Discussions = { }; export { Discussions }; + /* eslint-enable @gitlab/require-i18n-strings */ diff --git a/spec/contracts/consumer/specs/diffs.spec.js b/spec/contracts/consumer/specs/diffs.spec.js deleted file mode 100644 index 6b1cefdbdbc..00000000000 --- a/spec/contracts/consumer/specs/diffs.spec.js +++ /dev/null @@ -1,37 +0,0 @@ -/* eslint-disable @gitlab/require-i18n-strings */ - -import { pactWith } from 'jest-pact'; - -import { Diffs } from '../fixtures/diffs.fixture'; -import { getDiffs } from '../endpoints/merge_requests'; - -pactWith( - { - consumer: 'Merge Request Page', - provider: 'Merge Request Diffs Endpoint', - log: '../logs/consumer.log', - dir: '../contracts', - }, - - (provider) => { - describe('Diffs Endpoint', () => { - beforeEach(() => { - const interaction = { - state: 'a merge request with diffs exists', - ...Diffs.request, - willRespondWith: Diffs.success, - }; - provider.addInteraction(interaction); - }); - - it('return a successful body', () => { - return getDiffs({ - url: provider.mockService.baseUrl, - }).then((diffs) => { - expect(diffs).toEqual(Diffs.body); - }); - }); - }); - }, -); -/* eslint-enable @gitlab/require-i18n-strings */ diff --git a/spec/contracts/consumer/specs/discussions.spec.js b/spec/contracts/consumer/specs/discussions.spec.js deleted file mode 100644 index 2a5d0ba6267..00000000000 --- a/spec/contracts/consumer/specs/discussions.spec.js +++ /dev/null @@ -1,37 +0,0 @@ -/* eslint-disable @gitlab/require-i18n-strings */ - -import { pactWith } from 'jest-pact'; - -import { Discussions } from '../fixtures/discussions.fixture'; -import { getDiscussions } from '../endpoints/merge_requests'; - -pactWith( - { - consumer: 'Merge Request Page', - provider: 'Merge Request Discussions Endpoint', - log: '../logs/consumer.log', - dir: '../contracts', - }, - - (provider) => { - describe('Discussions Endpoint', () => { - beforeEach(() => { - const interaction = { - state: 'a merge request with discussions exists', - ...Discussions.request, - willRespondWith: Discussions.success, - }; - provider.addInteraction(interaction); - }); - - it('return a successful body', () => { - return getDiscussions({ - url: provider.mockService.baseUrl, - }).then((discussions) => { - expect(discussions).toEqual(Discussions.body); - }); - }); - }); - }, -); -/* eslint-enable @gitlab/require-i18n-strings */ diff --git a/spec/contracts/consumer/specs/metadata.spec.js b/spec/contracts/consumer/specs/metadata.spec.js deleted file mode 100644 index fc082cb6a46..00000000000 --- a/spec/contracts/consumer/specs/metadata.spec.js +++ /dev/null @@ -1,37 +0,0 @@ -/* eslint-disable @gitlab/require-i18n-strings */ - -import { pactWith } from 'jest-pact'; - -import { Metadata } from '../fixtures/metadata.fixture'; -import { getMetadata } from '../endpoints/merge_requests'; - -pactWith( - { - consumer: 'Merge Request Page', - provider: 'Merge Request Metadata Endpoint', - log: '../logs/consumer.log', - dir: '../contracts', - }, - - (provider) => { - describe('Metadata Endpoint', () => { - beforeEach(() => { - const interaction = { - state: 'a merge request exists', - ...Metadata.request, - willRespondWith: Metadata.success, - }; - provider.addInteraction(interaction); - }); - - it('return a successful body', () => { - return getMetadata({ - url: provider.mockService.baseUrl, - }).then((metadata) => { - expect(metadata).toEqual(Metadata.body); - }); - }); - }); - }, -); -/* eslint-enable @gitlab/require-i18n-strings */ diff --git a/spec/contracts/consumer/specs/project/merge_request/show.spec.js b/spec/contracts/consumer/specs/project/merge_request/show.spec.js new file mode 100644 index 00000000000..8c6e029cb12 --- /dev/null +++ b/spec/contracts/consumer/specs/project/merge_request/show.spec.js @@ -0,0 +1,112 @@ +/* eslint-disable @gitlab/require-i18n-strings */ + +import { pactWith } from 'jest-pact'; + +import { DiffsBatch } from '../../../fixtures/project/merge_request/diffs_batch.fixture'; +import { Discussions } from '../../../fixtures/project/merge_request/discussions.fixture'; +import { DiffsMetadata } from '../../../fixtures/project/merge_request/diffs_metadata.fixture'; +import { + getDiffsBatch, + getDiffsMetadata, + getDiscussions, +} from '../../../endpoints/project/merge_requests'; + +const CONSUMER_NAME = 'MergeRequest#show'; +const CONSUMER_LOG = '../logs/consumer.log'; +const CONTRACT_DIR = '../contracts/project/merge_request/show'; +const DIFFS_BATCH_PROVIDER_NAME = 'Merge Request Diffs Batch Endpoint'; +const DISCUSSIONS_PROVIDER_NAME = 'Merge Request Discussions Endpoint'; +const DIFFS_METADATA_PROVIDER_NAME = 'Merge Request Diffs Metadata Endpoint'; + +// API endpoint: /merge_requests/:id/diffs_batch.json +pactWith( + { + consumer: CONSUMER_NAME, + provider: DIFFS_BATCH_PROVIDER_NAME, + log: CONSUMER_LOG, + dir: CONTRACT_DIR, + }, + + (provider) => { + describe(DIFFS_BATCH_PROVIDER_NAME, () => { + beforeEach(() => { + const interaction = { + state: 'a merge request with diffs exists', + ...DiffsBatch.request, + willRespondWith: DiffsBatch.success, + }; + provider.addInteraction(interaction); + }); + + it('returns a successful body', () => { + return getDiffsBatch({ + url: provider.mockService.baseUrl, + }).then((diffsBatch) => { + expect(diffsBatch).toEqual(DiffsBatch.body); + }); + }); + }); + }, +); + +pactWith( + { + consumer: CONSUMER_NAME, + provider: DISCUSSIONS_PROVIDER_NAME, + log: CONSUMER_LOG, + dir: CONTRACT_DIR, + }, + + (provider) => { + describe(DISCUSSIONS_PROVIDER_NAME, () => { + beforeEach(() => { + const interaction = { + state: 'a merge request with discussions exists', + ...Discussions.request, + willRespondWith: Discussions.success, + }; + provider.addInteraction(interaction); + }); + + it('return a successful body', () => { + return getDiscussions({ + url: provider.mockService.baseUrl, + }).then((discussions) => { + expect(discussions).toEqual(Discussions.body); + }); + }); + }); + }, +); + +pactWith( + { + consumer: CONSUMER_NAME, + provider: DIFFS_METADATA_PROVIDER_NAME, + log: CONSUMER_LOG, + dir: CONTRACT_DIR, + }, + + (provider) => { + describe(DIFFS_METADATA_PROVIDER_NAME, () => { + beforeEach(() => { + const interaction = { + state: 'a merge request exists', + ...DiffsMetadata.request, + willRespondWith: DiffsMetadata.success, + }; + provider.addInteraction(interaction); + }); + + it('return a successful body', () => { + return getDiffsMetadata({ + url: provider.mockService.baseUrl, + }).then((diffsMetadata) => { + expect(diffsMetadata).toEqual(DiffsMetadata.body); + }); + }); + }); + }, +); + +/* eslint-enable @gitlab/require-i18n-strings */ diff --git a/spec/contracts/contracts/merge_request_page-merge_request_diffs_endpoint.json b/spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_batch_endpoint.json similarity index 98% rename from spec/contracts/contracts/merge_request_page-merge_request_diffs_endpoint.json rename to spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_batch_endpoint.json index 2f097d8eb24..3fa13766766 100644 --- a/spec/contracts/contracts/merge_request_page-merge_request_diffs_endpoint.json +++ b/spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_batch_endpoint.json @@ -1,9 +1,9 @@ { "consumer": { - "name": "Merge Request Page" + "name": "MergeRequest#show" }, "provider": { - "name": "Merge Request Diffs Endpoint" + "name": "Merge Request Diffs Batch Endpoint" }, "interactions": [ { @@ -226,4 +226,4 @@ "version": "2.0.0" } } -} +} \ No newline at end of file diff --git a/spec/contracts/contracts/merge_request_page-merge_request_metadata_endpoint.json b/spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_metadata_endpoint.json similarity index 98% rename from spec/contracts/contracts/merge_request_page-merge_request_metadata_endpoint.json rename to spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_metadata_endpoint.json index eb22b7d2e3c..b98a0127e54 100644 --- a/spec/contracts/contracts/merge_request_page-merge_request_metadata_endpoint.json +++ b/spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_metadata_endpoint.json @@ -1,13 +1,13 @@ { "consumer": { - "name": "Merge Request Page" + "name": "MergeRequest#show" }, "provider": { - "name": "Merge Request Metadata Endpoint" + "name": "Merge Request Diffs Metadata Endpoint" }, "interactions": [ { - "description": "a request for Metadata", + "description": "a request for Diffs Metadata", "providerState": "a merge request exists", "request": { "method": "GET", @@ -220,4 +220,4 @@ "version": "2.0.0" } } -} +} \ No newline at end of file diff --git a/spec/contracts/contracts/merge_request_page-merge_request_discussions_endpoint.json b/spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_discussions_endpoint.json similarity index 99% rename from spec/contracts/contracts/merge_request_page-merge_request_discussions_endpoint.json rename to spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_discussions_endpoint.json index 819d95276b3..ecaf9c123af 100644 --- a/spec/contracts/contracts/merge_request_page-merge_request_discussions_endpoint.json +++ b/spec/contracts/contracts/project/merge_request/show/mergerequest#show-merge_request_discussions_endpoint.json @@ -1,6 +1,6 @@ { "consumer": { - "name": "Merge Request Page" + "name": "MergeRequest#show" }, "provider": { "name": "Merge Request Discussions Endpoint" @@ -233,4 +233,4 @@ "version": "2.0.0" } } -} +} \ No newline at end of file diff --git a/spec/contracts/provider/pact_helpers/project/merge_request/diffs_batch_helper.rb b/spec/contracts/provider/pact_helpers/project/merge_request/diffs_batch_helper.rb new file mode 100644 index 00000000000..7d1fbe91e86 --- /dev/null +++ b/spec/contracts/provider/pact_helpers/project/merge_request/diffs_batch_helper.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require_relative '../../../spec_helper' +require_relative '../../../states/project/merge_request/diffs_batch_state' + +module Provider + module DiffsBatchHelper + Pact.service_provider "Merge Request Diffs Batch Endpoint" do + app { Environments::Test.app } + + honours_pact_with 'MergeRequest#show' do + pact_uri '../contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_batch_endpoint.json' + end + end + end +end diff --git a/spec/contracts/provider/pact_helpers/project/merge_request/diffs_metadata_helper.rb b/spec/contracts/provider/pact_helpers/project/merge_request/diffs_metadata_helper.rb new file mode 100644 index 00000000000..5f0c58d18d4 --- /dev/null +++ b/spec/contracts/provider/pact_helpers/project/merge_request/diffs_metadata_helper.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require_relative '../../../spec_helper' +require_relative '../../../states/project/merge_request/diffs_metadata_state' + +module Provider + module DiffsMetadataHelper + Pact.service_provider "Merge Request Diffs Metadata Endpoint" do + app { Environments::Test.app } + + honours_pact_with 'MergeRequest#show' do + pact_uri '../contracts/project/merge_request/show/mergerequest#show-merge_request_diffs_metadata_endpoint.json' + end + end + end +end diff --git a/spec/contracts/provider/pact_helpers/project/merge_request/discussions_helper.rb b/spec/contracts/provider/pact_helpers/project/merge_request/discussions_helper.rb new file mode 100644 index 00000000000..0f4244ba40a --- /dev/null +++ b/spec/contracts/provider/pact_helpers/project/merge_request/discussions_helper.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require_relative '../../../spec_helper' +require_relative '../../../states/project/merge_request/discussions_state' + +module Provider + module DiscussionsHelper + Pact.service_provider "Merge Request Discussions Endpoint" do + app { Environments::Test.app } + + honours_pact_with 'MergeRequest#show' do + pact_uri '../contracts/project/merge_request/show/mergerequest#show-merge_request_discussions_endpoint.json' + end + end + end +end diff --git a/spec/contracts/provider/specs/diffs_helper.rb b/spec/contracts/provider/specs/diffs_helper.rb deleted file mode 100644 index 24bdd00dbae..00000000000 --- a/spec/contracts/provider/specs/diffs_helper.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -require_relative '../spec_helper' -require_relative '../states/diffs_state' - -module Provider - module DiffsHelper - Pact.service_provider "Merge Request Diffs Endpoint" do - app { Environments::Test.app } - - honours_pact_with 'Merge Request Page' do - pact_uri '../contracts/merge_request_page-merge_request_diffs_endpoint.json' - end - end - end -end diff --git a/spec/contracts/provider/specs/discussions_helper.rb b/spec/contracts/provider/specs/discussions_helper.rb deleted file mode 100644 index 135ccf48276..00000000000 --- a/spec/contracts/provider/specs/discussions_helper.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -require_relative '../spec_helper' -require_relative '../states/discussions_state' - -module Provider - module DiscussionsHelper - Pact.service_provider "Merge Request Discussions Endpoint" do - app { Environments::Test.app } - - honours_pact_with 'Merge Request Page' do - pact_uri '../contracts/merge_request_page-merge_request_discussions_endpoint.json' - end - end - end -end diff --git a/spec/contracts/provider/specs/metadata_helper.rb b/spec/contracts/provider/specs/metadata_helper.rb deleted file mode 100644 index e73b993a31a..00000000000 --- a/spec/contracts/provider/specs/metadata_helper.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -require_relative '../spec_helper' -require_relative '../states/metadata_state' - -module Provider - module MetadataHelper - Pact.service_provider "Merge Request Metadata Endpoint" do - app { Environments::Test.app } - - honours_pact_with 'Merge Request Page' do - pact_uri '../contracts/merge_request_page-merge_request_metadata_endpoint.json' - end - end - end -end diff --git a/spec/contracts/provider/states/diffs_state.rb b/spec/contracts/provider/states/project/merge_request/diffs_batch_state.rb similarity index 93% rename from spec/contracts/provider/states/diffs_state.rb rename to spec/contracts/provider/states/project/merge_request/diffs_batch_state.rb index d959cde5f5e..ac20c17c187 100644 --- a/spec/contracts/provider/states/diffs_state.rb +++ b/spec/contracts/provider/states/project/merge_request/diffs_batch_state.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -Pact.provider_states_for "Merge Request Page" do +Pact.provider_states_for "MergeRequest#show" do provider_state "a merge request with diffs exists" do set_up do user = User.find_by(name: Provider::UsersHelper::CONTRACT_USER_NAME) diff --git a/spec/contracts/provider/states/metadata_state.rb b/spec/contracts/provider/states/project/merge_request/diffs_metadata_state.rb similarity index 92% rename from spec/contracts/provider/states/metadata_state.rb rename to spec/contracts/provider/states/project/merge_request/diffs_metadata_state.rb index 59b290ce2fe..8754232690c 100644 --- a/spec/contracts/provider/states/metadata_state.rb +++ b/spec/contracts/provider/states/project/merge_request/diffs_metadata_state.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -Pact.provider_states_for "Merge Request Page" do +Pact.provider_states_for "MergeRequest#show" do provider_state "a merge request exists" do set_up do user = User.find_by(name: Provider::UsersHelper::CONTRACT_USER_NAME) diff --git a/spec/contracts/provider/states/discussions_state.rb b/spec/contracts/provider/states/project/merge_request/discussions_state.rb similarity index 92% rename from spec/contracts/provider/states/discussions_state.rb rename to spec/contracts/provider/states/project/merge_request/discussions_state.rb index ddbcf80f2c8..2d64f85eedf 100644 --- a/spec/contracts/provider/states/discussions_state.rb +++ b/spec/contracts/provider/states/project/merge_request/discussions_state.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -Pact.provider_states_for "Merge Request Page" do +Pact.provider_states_for "MergeRequest#show" do provider_state "a merge request with discussions exists" do set_up do user = User.find_by(name: Provider::UsersHelper::CONTRACT_USER_NAME) diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb index 1e77da64663..ad6682601f3 100644 --- a/spec/controllers/projects/releases_controller_spec.rb +++ b/spec/controllers/projects/releases_controller_spec.rb @@ -148,19 +148,19 @@ RSpec.describe Projects::ReleasesController do end let(:release) { create(:release, project: project) } - let(:tag) { CGI.escape(release.tag) } + let(:tag) { release.tag } it_behaves_like 'successful request' context 'when tag name contains slash' do let(:release) { create(:release, project: project, tag: 'awesome/v1.0') } - let(:tag) { CGI.escape(release.tag) } + let(:tag) { release.tag } it_behaves_like 'successful request' it 'is accesible at a URL encoded path' do expect(edit_project_release_path(project, release)) - .to eq("/#{project.namespace.path}/#{project.name}/-/releases/awesome%252Fv1.0/edit") + .to eq("/#{project.namespace.path}/#{project.name}/-/releases/awesome%2Fv1.0/edit") end end @@ -187,19 +187,19 @@ RSpec.describe Projects::ReleasesController do end let(:release) { create(:release, project: project) } - let(:tag) { CGI.escape(release.tag) } + let(:tag) { release.tag } it_behaves_like 'successful request' context 'when tag name contains slash' do let(:release) { create(:release, project: project, tag: 'awesome/v1.0') } - let(:tag) { CGI.escape(release.tag) } + let(:tag) { release.tag } it_behaves_like 'successful request' it 'is accesible at a URL encoded path' do expect(project_release_path(project, release)) - .to eq("/#{project.namespace.path}/#{project.name}/-/releases/awesome%252Fv1.0") + .to eq("/#{project.namespace.path}/#{project.name}/-/releases/awesome%2Fv1.0") end end @@ -239,7 +239,7 @@ RSpec.describe Projects::ReleasesController do end let(:release) { create(:release, project: project) } - let(:tag) { CGI.escape(release.tag) } + let(:tag) { release.tag } context 'when user is a guest' do let(:project) { private_project } diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 84e702cd6a9..cf16807723b 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -468,4 +468,25 @@ RSpec.describe DiffHelper do it { is_expected.to be_nil } end end + + describe '#conflicts' do + let(:merge_request) { instance_double(MergeRequest) } + + before do + allow(helper).to receive(:merge_request).and_return(merge_request) + allow(helper).to receive(:options).and_return(merge_ref_head_diff: true) + end + + context 'when Gitlab::Git::Conflict::Resolver::ConflictSideMissing exception is raised' do + before do + allow_next_instance_of(MergeRequests::Conflicts::ListService, merge_request, allow_tree_conflicts: true) do |svc| + allow(svc).to receive_message_chain(:conflicts, :files).and_raise(Gitlab::Git::Conflict::Resolver::ConflictSideMissing) + end + end + + it 'returns an empty hash' do + expect(helper.conflicts(allow_tree_conflicts: true)).to eq({}) + end + end + end end diff --git a/spec/lib/gitlab/form_builders/gitlab_ui_form_builder_spec.rb b/spec/lib/gitlab/form_builders/gitlab_ui_form_builder_spec.rb index a5f26a212ab..2b1fcac9257 100644 --- a/spec/lib/gitlab/form_builders/gitlab_ui_form_builder_spec.rb +++ b/spec/lib/gitlab/form_builders/gitlab_ui_form_builder_spec.rb @@ -3,163 +3,194 @@ require 'spec_helper' RSpec.describe Gitlab::FormBuilders::GitlabUiFormBuilder do - let_it_be(:user) { build(:user) } - let_it_be(:fake_template) do - Object.new.tap do |template| - template.extend ActionView::Helpers::FormHelper - template.extend ActionView::Helpers::FormOptionsHelper - template.extend ActionView::Helpers::TagHelper - template.extend ActionView::Context - end - end + include FormBuilderHelpers - let_it_be(:form_builder) { described_class.new(:user, user, fake_template, {}) } + let_it_be(:user) { build(:user, :admin) } + + let_it_be(:form_builder) { described_class.new(:user, user, fake_action_view_base, {}) } describe '#gitlab_ui_checkbox_component' do - let(:optional_args) { {} } + context 'when not using slots' do + let(:optional_args) { {} } - subject(:checkbox_html) { form_builder.gitlab_ui_checkbox_component(:view_diffs_file_by_file, "Show one file at a time on merge request's Changes tab", **optional_args) } + subject(:checkbox_html) do + form_builder.gitlab_ui_checkbox_component( + :view_diffs_file_by_file, + "Show one file at a time on merge request's Changes tab", + **optional_args + ) + end + + context 'without optional arguments' do + it 'renders correct html' do + expected_html = <<~EOS +
+ + + +
+ EOS + + expect(html_strip_whitespace(checkbox_html)).to eq(html_strip_whitespace(expected_html)) + end + end + + context 'with optional arguments' do + let(:optional_args) do + { + help_text: 'Instead of all the files changed, show only one file at a time.', + checkbox_options: { class: 'checkbox-foo-bar' }, + label_options: { class: 'label-foo-bar' }, + checked_value: '3', + unchecked_value: '1' + } + end + + it 'renders help text' do + expected_html = <<~EOS +
+ + + +
+ EOS + + expect(html_strip_whitespace(checkbox_html)).to eq(html_strip_whitespace(expected_html)) + end + end + + context 'with checkbox_options: { multiple: true }' do + let(:optional_args) do + { + checkbox_options: { multiple: true }, + checked_value: 'one', + unchecked_value: false + } + end + + it 'renders labels with correct for attributes' do + expected_html = <<~EOS +
+ + +
+ EOS + + expect(html_strip_whitespace(checkbox_html)).to eq(html_strip_whitespace(expected_html)) + end + end + end + + context 'when using slots' do + subject(:checkbox_html) do + form_builder.gitlab_ui_checkbox_component( + :view_diffs_file_by_file + ) do |c| + c.label { "Show one file at a time on merge request's Changes tab" } + c.help_text { 'Instead of all the files changed, show only one file at a time.' } + end + end - context 'without optional arguments' do it 'renders correct html' do expected_html = <<~EOS
-
- EOS - - expect(checkbox_html).to eq(html_strip_whitespace(expected_html)) - end - end - - context 'with optional arguments' do - let(:optional_args) do - { - help_text: 'Instead of all the files changed, show only one file at a time.', - checkbox_options: { class: 'checkbox-foo-bar' }, - label_options: { class: 'label-foo-bar' }, - checked_value: '3', - unchecked_value: '1' - } - end - - it 'renders help text' do - expected_html = <<~EOS -
- - -
EOS - expect(checkbox_html).to eq(html_strip_whitespace(expected_html)) - end - - it 'passes arguments to `check_box` method' do - allow(fake_template).to receive(:check_box).and_return('') - - checkbox_html - - expect(fake_template).to have_received(:check_box).with(:user, :view_diffs_file_by_file, { class: %w(custom-control-input checkbox-foo-bar), object: user }, '3', '1') - end - - it 'passes arguments to `label` method' do - allow(fake_template).to receive(:label).and_return('') - - checkbox_html - - expect(fake_template).to have_received(:label).with(:user, :view_diffs_file_by_file, { class: %w(custom-control-label label-foo-bar), object: user, value: nil }) - end - end - - context 'with checkbox_options: { multiple: true }' do - let(:optional_args) do - { - checkbox_options: { multiple: true }, - checked_value: 'one', - unchecked_value: false - } - end - - it 'renders labels with correct for attributes' do - expected_html = <<~EOS -
- - -
- EOS - - expect(checkbox_html).to eq(html_strip_whitespace(expected_html)) + expect(html_strip_whitespace(checkbox_html)).to eq(html_strip_whitespace(expected_html)) end end end describe '#gitlab_ui_radio_component' do - let(:optional_args) { {} } + context 'when not using slots' do + let(:optional_args) { {} } - subject(:radio_html) { form_builder.gitlab_ui_radio_component(:access_level, :admin, "Access Level", **optional_args) } + subject(:radio_html) do + form_builder.gitlab_ui_radio_component( + :access_level, + :admin, + "Admin", + **optional_args + ) + end - context 'without optional arguments' do - it 'renders correct html' do - expected_html = <<~EOS -
- - -
- EOS + context 'without optional arguments' do + it 'renders correct html' do + expected_html = <<~EOS +
+ + +
+ EOS - expect(radio_html).to eq(html_strip_whitespace(expected_html)) + expect(html_strip_whitespace(radio_html)).to eq(html_strip_whitespace(expected_html)) + end + end + + context 'with optional arguments' do + let(:optional_args) do + { + help_text: 'Administrators have access to all groups, projects, and users and can manage all features in this installation', + radio_options: { class: 'radio-foo-bar' }, + label_options: { class: 'label-foo-bar' } + } + end + + it 'renders help text' do + expected_html = <<~EOS +
+ + +
+ EOS + + expect(html_strip_whitespace(radio_html)).to eq(html_strip_whitespace(expected_html)) + end end end - context 'with optional arguments' do - let(:optional_args) do - { - help_text: 'Administrators have access to all groups, projects, and users and can manage all features in this installation', - radio_options: { class: 'radio-foo-bar' }, - label_options: { class: 'label-foo-bar' } - } + context 'when using slots' do + subject(:radio_html) do + form_builder.gitlab_ui_radio_component( + :access_level, + :admin + ) do |c| + c.label { "Admin" } + c.help_text { 'Administrators have access to all groups, projects, and users and can manage all features in this installation' } + end end - it 'renders help text' do + it 'renders correct html' do expected_html = <<~EOS
- -
EOS - expect(radio_html).to eq(html_strip_whitespace(expected_html)) - end - - it 'passes arguments to `radio_button` method' do - allow(fake_template).to receive(:radio_button).and_return('') - - radio_html - - expect(fake_template).to have_received(:radio_button).with(:user, :access_level, :admin, { class: %w(custom-control-input radio-foo-bar), object: user }) - end - - it 'passes arguments to `label` method' do - allow(fake_template).to receive(:label).and_return('') - - radio_html - - expect(fake_template).to have_received(:label).with(:user, :access_level, { class: %w(custom-control-label label-foo-bar), object: user, value: :admin }) + expect(html_strip_whitespace(radio_html)).to eq(html_strip_whitespace(expected_html)) end end end diff --git a/spec/requests/api/internal/workhorse_spec.rb b/spec/requests/api/internal/workhorse_spec.rb index e6836d8fef5..d40c14cc0fd 100644 --- a/spec/requests/api/internal/workhorse_spec.rb +++ b/spec/requests/api/internal/workhorse_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Internal::Workhorse do +RSpec.describe API::Internal::Workhorse, :allow_forgery_protection do include WorkhorseHelpers context '/authorize_upload' do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 3108197f662..c050214ff50 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -227,6 +227,7 @@ RSpec.describe API::Releases do get api("/projects/#{project.id}/releases", maintainer) expect(response).to have_gitlab_http_status(:ok) + expect(json_response[0]['tag_path']).to include('%2F') # properly escape the slash end end diff --git a/spec/services/merge_requests/mergeability/run_checks_service_spec.rb b/spec/services/merge_requests/mergeability/run_checks_service_spec.rb index d4ee4afd71d..2bb7dc3eef7 100644 --- a/spec/services/merge_requests/mergeability/run_checks_service_spec.rb +++ b/spec/services/merge_requests/mergeability/run_checks_service_spec.rb @@ -7,12 +7,6 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do let_it_be(:merge_request) { create(:merge_request) } - describe '#CHECKS' do - it 'contains every subclass of the base checks service', :eager_load do - expect(described_class::CHECKS).to contain_exactly(*MergeRequests::Mergeability::CheckBaseService.subclasses) - end - end - describe '#execute' do subject(:execute) { run_checks.execute } @@ -22,8 +16,8 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do context 'when every check is skipped', :eager_load do before do MergeRequests::Mergeability::CheckBaseService.subclasses.each do |subclass| - expect_next_instance_of(subclass) do |service| - expect(service).to receive(:skip?).and_return(true) + allow_next_instance_of(subclass) do |service| + allow(service).to receive(:skip?).and_return(true) end end end @@ -35,7 +29,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do context 'when a check is skipped' do it 'does not execute the check' do - described_class::CHECKS.each do |check| + merge_request.mergeability_checks.each do |check| allow_next_instance_of(check) do |service| allow(service).to receive(:skip?).and_return(false) allow(service).to receive(:execute).and_return(success_result) @@ -47,7 +41,13 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do expect(service).not_to receive(:execute) end - expect(execute).to match_array([success_result, success_result, success_result, success_result]) + # Since we're only marking one check to be skipped, we expect to receive + # `# of checks - 1` success result objects in return + # + check_count = merge_request.mergeability_checks.count - 1 + success_array = (1..check_count).each_with_object([]) { |_, array| array << success_result } + + expect(execute).to match_array(success_array) end end @@ -56,7 +56,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do let(:merge_check) { instance_double(MergeRequests::Mergeability::CheckCiStatusService) } before do - described_class::CHECKS.each do |check| + merge_request.mergeability_checks.each do |check| allow_next_instance_of(check) do |service| allow(service).to receive(:skip?).and_return(true) end diff --git a/spec/support/helpers/form_builder_helpers.rb b/spec/support/helpers/form_builder_helpers.rb new file mode 100644 index 00000000000..4bae7421c4d --- /dev/null +++ b/spec/support/helpers/form_builder_helpers.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module FormBuilderHelpers + def fake_action_view_base + lookup_context = ActionView::LookupContext.new(ActionController::Base.view_paths) + + ActionView::Base.new(lookup_context, {}, ApplicationController.new) + end + + def fake_form_for(&block) + fake_action_view_base.form_for :user, url: '/user', &block + end +end diff --git a/spec/support/shared_examples/components/pajamas_shared_examples.rb b/spec/support/shared_examples/components/pajamas_shared_examples.rb new file mode 100644 index 00000000000..5c0ad1a1bc9 --- /dev/null +++ b/spec/support/shared_examples/components/pajamas_shared_examples.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'it renders help text' do + it 'renders help text' do + expect(rendered_component).to have_selector('[data-testid="pajamas-component-help-text"]', text: help_text) + end +end + +RSpec.shared_examples 'it does not render help text' do + it 'does not render help text' do + expect(rendered_component).not_to have_selector('[data-testid="pajamas-component-help-text"]') + end +end diff --git a/spec/support/shared_examples/graphql/types/gitlab_style_deprecations_shared_examples.rb b/spec/support/shared_examples/graphql/types/gitlab_style_deprecations_shared_examples.rb index cf9c36fafe8..7fd54408b11 100644 --- a/spec/support/shared_examples/graphql/types/gitlab_style_deprecations_shared_examples.rb +++ b/spec/support/shared_examples/graphql/types/gitlab_style_deprecations_shared_examples.rb @@ -53,18 +53,20 @@ RSpec.shared_examples 'Gitlab-style deprecations' do it 'adds information about the replacement if provided' do deprecable = subject(deprecated: { milestone: '1.10', reason: :renamed, replacement: 'Foo.bar' }) - expect(deprecable.deprecation_reason).to include 'Please use `Foo.bar`' + expect(deprecable.deprecation_reason).to include('Please use `Foo.bar`') end it 'supports named reasons: renamed' do deprecable = subject(deprecated: { milestone: '1.10', reason: :renamed }) - expect(deprecable.deprecation_reason).to include 'This was renamed.' + expect(deprecable.deprecation_reason).to eq('This was renamed. Deprecated in 1.10.') end it 'supports named reasons: alpha' do deprecable = subject(deprecated: { milestone: '1.10', reason: :alpha }) - expect(deprecable.deprecation_reason).to include 'This feature is in Alpha' + expect(deprecable.deprecation_reason).to eq( + 'This feature is in Alpha. It can be changed or removed at any time. Introduced in 1.10.' + ) end end diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go index a00cff34b89..3abc735dc90 100644 --- a/workhorse/internal/api/api.go +++ b/workhorse/internal/api/api.go @@ -3,7 +3,6 @@ package api import ( "bytes" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -310,18 +309,18 @@ func (api *API) PreAuthorizeFixedPath(r *http.Request, method string, path strin } authReq.Header = helper.HeaderClone(r.Header) - ignoredResponse, apiResponse, err := api.PreAuthorize(path, authReq) + failureResponse, apiResponse, err := api.PreAuthorize(path, authReq) if err != nil { return nil, fmt.Errorf("PreAuthorize: %w", err) } - // We don't need the contents of ignoredResponse but we are responsible + // We don't need the contents of failureResponse but we are responsible // for closing it. Part of the reason PreAuthorizeFixedPath exists is to // hide this awkwardness. - ignoredResponse.Body.Close() + failureResponse.Body.Close() if apiResponse == nil { - return nil, errors.New("no api response on fixed path") + return nil, fmt.Errorf("no api response: status %d", failureResponse.StatusCode) } return apiResponse, nil