diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-03-02 00:13:45 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-03-02 00:13:45 +0000 |
| commit | 90c386a7b0f2701abeb1e86517fc1d5dea231c09 (patch) | |
| tree | bd31e90b87c7986276daee9020a065313db7c3ba | |
| parent | 70fe7ce74ba4a8430c88ec6e3f4c60475a69fe21 (diff) | |
| download | gitlab-ce-90c386a7b0f2701abeb1e86517fc1d5dea231c09.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
52 files changed, 641 insertions, 357 deletions
diff --git a/.gitlab/issue_templates/QA Failure.md b/.gitlab/issue_templates/QA Failure.md index 957aac299b6..3171923d8c5 100644 --- a/.gitlab/issue_templates/QA Failure.md +++ b/.gitlab/issue_templates/QA Failure.md @@ -57,7 +57,7 @@ If you include multiple screenshots it can be helpful to hide all but the first /label ~Quality ~QA ~test <!-- Test failure type label, please use just one.--> -/label ~"failure::broken-test" ~"failure::flaky-test" ~"failure::stale-test" ~"failure::test-environment" ~"failure::investigating" +/label ~"failure::broken-test" ~"failure::flaky-test" ~"failure::stale-test" ~"failure::test-environment" ~"failure::investigating" ~"failure::new" <!-- Choose the stage that appears in the test path, e.g. ~"devops::create" for diff --git a/app/assets/javascripts/releases/components/app_index.vue b/app/assets/javascripts/releases/components/app_index.vue index c2c91f406a1..e53bfea7389 100644 --- a/app/assets/javascripts/releases/components/app_index.vue +++ b/app/assets/javascripts/releases/components/app_index.vue @@ -68,7 +68,7 @@ export default { :href="newReleasePath" :aria-describedby="shouldRenderEmptyState && 'releases-description'" category="primary" - variant="success" + variant="confirm" data-testid="new-release-button" > {{ __('New release') }} diff --git a/app/graphql/types/alert_management/alert_type.rb b/app/graphql/types/alert_management/alert_type.rb index 7495d46179c..43b7bbb419f 100644 --- a/app/graphql/types/alert_management/alert_type.rb +++ b/app/graphql/types/alert_management/alert_type.rb @@ -9,6 +9,7 @@ module Types present_using ::AlertManagement::AlertPresenter implements(Types::Notes::NoteableInterface) + implements(Types::TodoableInterface) authorize :read_alert_management_alert @@ -127,6 +128,12 @@ module Types null: true, description: 'Alert condition for Prometheus.' + field :web_url, + GraphQL::Types::String, + method: :details_url, + null: false, + description: 'URL of the alert.' + def notes object.ordered_notes end diff --git a/app/graphql/types/commit_type.rb b/app/graphql/types/commit_type.rb index 221f215f6ff..c3a6d6f7faa 100644 --- a/app/graphql/types/commit_type.rb +++ b/app/graphql/types/commit_type.rb @@ -8,6 +8,8 @@ module Types present_using CommitPresenter + implements(Types::TodoableInterface) + field :id, type: GraphQL::Types::ID, null: false, description: 'ID (global ID) of the commit.' diff --git a/app/graphql/types/design_management/design_type.rb b/app/graphql/types/design_management/design_type.rb index 2f40bf5ebfd..4c0b1162306 100644 --- a/app/graphql/types/design_management/design_type.rb +++ b/app/graphql/types/design_management/design_type.rb @@ -13,6 +13,12 @@ module Types implements(Types::Notes::NoteableInterface) implements(Types::DesignManagement::DesignFields) implements(Types::CurrentUserTodos) + implements(Types::TodoableInterface) + + field :web_url, + GraphQL::Types::String, + null: false, + description: 'URL of the design.' field :versions, Types::DesignManagement::VersionType.connection_type, @@ -40,6 +46,10 @@ module Types def request_cache_base_key self.class.name end + + def web_url + Gitlab::UrlBuilder.build(object) + end end end end diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb index 6167b661caa..07450c38616 100644 --- a/app/graphql/types/issue_type.rb +++ b/app/graphql/types/issue_type.rb @@ -8,6 +8,7 @@ module Types implements(Types::Notes::NoteableInterface) implements(Types::CurrentUserTodos) + implements(Types::TodoableInterface) authorize :read_issue diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index ea05671c79c..d573df21587 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -8,6 +8,7 @@ module Types implements(Types::Notes::NoteableInterface) implements(Types::CurrentUserTodos) + implements(Types::TodoableInterface) authorize :read_merge_request diff --git a/app/graphql/types/todo_type.rb b/app/graphql/types/todo_type.rb index 34ba2c75b5f..62e2d5996aa 100644 --- a/app/graphql/types/todo_type.rb +++ b/app/graphql/types/todo_type.rb @@ -31,6 +31,11 @@ module Types description: 'Action of the to-do item.', null: false + field :target, Types::TodoableInterface, + description: 'Target of the to-do item.', + calls_gitaly: true, + null: false + field :target_type, Types::TodoTargetEnum, description: 'Target type of the to-do item.', null: false @@ -59,5 +64,28 @@ module Types def author Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find end + + def target + if object.for_commit? + Gitlab::Graphql::Loaders::BatchCommitLoader.new( + container_class: Project, + container_id: object.project_id, + oid: object.commit_id + ).find + else + Gitlab::Graphql::Loaders::BatchModelLoader.new(target_type_class, object.target_id).find + end + end + + private + + def target_type_class + klass = object.target_type.safe_constantize + raise "Invalid target type \"#{object.target_type}\"" unless klass < Todoable + + klass + end end end + +Types::TodoType.prepend_mod diff --git a/app/graphql/types/todoable_interface.rb b/app/graphql/types/todoable_interface.rb new file mode 100644 index 00000000000..7d437973c12 --- /dev/null +++ b/app/graphql/types/todoable_interface.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Types + module TodoableInterface + include Types::BaseInterface + + graphql_name 'Todoable' + + field :web_url, GraphQL::Types::String, null: true, description: 'URL of this object.' + + def self.resolve_type(object, context) + case object + when Issue + Types::IssueType + when MergeRequest + Types::MergeRequestType + when ::DesignManagement::Design + Types::DesignManagement::DesignType + when ::AlertManagement::Alert + Types::AlertManagement::AlertType + when Commit + Types::CommitType + else + raise "Unknown GraphQL type for #{object}" + end + end + end +end + +Types::TodoableInterface.prepend_mod diff --git a/app/services/spam/spam_action_service.rb b/app/services/spam/spam_action_service.rb index 2a28b66f09b..4fa9c0e4993 100644 --- a/app/services/spam/spam_action_service.rb +++ b/app/services/spam/spam_action_service.rb @@ -65,22 +65,19 @@ module Spam # ask the SpamVerdictService what to do with the target. spam_verdict_service.execute.tap do |result| case result - when CONDITIONAL_ALLOW - # at the moment, this means "ask for reCAPTCHA" - create_spam_log - - break if target.allow_possible_spam? - - target.needs_recaptcha! - when DISALLOW - # TODO: remove `unless target.allow_possible_spam?` once this flag has been passed to `SpamVerdictService` - # https://gitlab.com/gitlab-org/gitlab/-/issues/214739 - target.spam! unless target.allow_possible_spam? - create_spam_log when BLOCK_USER # TODO: improve BLOCK_USER handling, non-existent until now # https://gitlab.com/gitlab-org/gitlab/-/issues/329666 - target.spam! unless target.allow_possible_spam? + target.spam! + create_spam_log + when DISALLOW + target.spam! + create_spam_log + when CONDITIONAL_ALLOW + # This means "require a CAPTCHA to be solved" + target.needs_recaptcha! + create_spam_log + when OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM create_spam_log when ALLOW target.clear_spam_flags! diff --git a/app/services/spam/spam_constants.rb b/app/services/spam/spam_constants.rb index b654fbbbcc8..d300525710c 100644 --- a/app/services/spam/spam_constants.rb +++ b/app/services/spam/spam_constants.rb @@ -2,11 +2,12 @@ module Spam module SpamConstants - CONDITIONAL_ALLOW = "conditional_allow" - DISALLOW = "disallow" - ALLOW = "allow" - BLOCK_USER = "block" - NOOP = "noop" + BLOCK_USER = 'block' + DISALLOW = 'disallow' + CONDITIONAL_ALLOW = 'conditional_allow' + OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM = 'override_via_allow_possible_spam' + ALLOW = 'allow' + NOOP = 'noop' SUPPORTED_VERDICTS = { BLOCK_USER => { @@ -18,11 +19,14 @@ module Spam CONDITIONAL_ALLOW => { priority: 3 }, - ALLOW => { + OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM => { priority: 4 }, - NOOP => { + ALLOW => { priority: 5 + }, + NOOP => { + priority: 6 } }.freeze end diff --git a/app/services/spam/spam_verdict_service.rb b/app/services/spam/spam_verdict_service.rb index c8bdcf4310b..e73b2666c02 100644 --- a/app/services/spam/spam_verdict_service.rb +++ b/app/services/spam/spam_verdict_service.rb @@ -39,21 +39,24 @@ module Spam return ALLOW unless valid_results.any? # Favour the most restrictive result. - final_verdict = valid_results.min_by { |v| SUPPORTED_VERDICTS[v][:priority] } + verdict = valid_results.min_by { |v| SUPPORTED_VERDICTS[v][:priority] } + + # The target can override the verdict via the `allow_possible_spam` feature flag + verdict = OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM if override_via_allow_possible_spam?(verdict: verdict) logger.info(class: self.class.name, akismet_verdict: akismet_verdict, spam_check_verdict: original_spamcheck_result, extra_attributes: spamcheck_attribs, spam_check_rtt: external_spam_check_round_trip_time.real, - final_verdict: final_verdict, + final_verdict: verdict, username: user.username, user_id: user.id, target_type: target.class.to_s, project_id: target.project_id ) - final_verdict + verdict end private @@ -87,6 +90,14 @@ module Spam end end + def override_via_allow_possible_spam?(verdict:) + # If the verdict is already going to allow (because current verdict's priority value is greater + # than the override verdict's priority value), then we don't need to override it. + return false if SUPPORTED_VERDICTS[verdict][:priority] > SUPPORTED_VERDICTS[OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM][:priority] + + target.allow_possible_spam? + end + def spamcheck_client @spamcheck_client ||= Gitlab::Spamcheck::Client.new end diff --git a/app/views/admin/application_settings/appearances/_form.html.haml b/app/views/admin/application_settings/appearances/_form.html.haml index 0f7f0109a54..84c26da8772 100644 --- a/app/views/admin/application_settings/appearances/_form.html.haml +++ b/app/views/admin/application_settings/appearances/_form.html.haml @@ -16,7 +16,7 @@ = image_tag @appearance.header_logo_path, class: 'appearance-light-logo-preview' - if @appearance.persisted? %br - = link_to _('Remove header logo'), header_logos_admin_application_settings_appearances_path, data: { confirm: _("Header logo will be removed. Are you sure?") }, method: :delete, class: "btn gl-button btn-danger btn-danger-secondary btn-sm" + = link_to _('Remove header logo'), header_logos_admin_application_settings_appearances_path, data: { confirm: _("Header logo will be removed. Are you sure?"), confirm_btn_variant: "danger" }, aria: { label: _('Remove header logo') }, method: :delete, class: "btn gl-button btn-danger btn-danger-secondary btn-sm" %hr = f.hidden_field :header_logo_cache = f.file_field :header_logo, class: "", accept: 'image/*' @@ -35,7 +35,7 @@ = image_tag @appearance.favicon_path, class: 'appearance-light-logo-preview' - if @appearance.persisted? %br - = link_to _('Remove favicon'), favicon_admin_application_settings_appearances_path, data: { confirm: _("Favicon will be removed. Are you sure?") }, method: :delete, class: "btn gl-button btn-danger btn-danger-secondary btn-sm" + = link_to _('Remove favicon'), favicon_admin_application_settings_appearances_path, data: { confirm: _("Favicon will be removed. Are you sure?"), confirm_btn_variant: "danger" }, aria: { label: _('Remove favicon') }, method: :delete, class: "btn gl-button btn-danger btn-danger-secondary btn-sm" %hr = f.hidden_field :favicon_cache = f.file_field :favicon, class: '', accept: 'image/*' @@ -67,7 +67,7 @@ = image_tag @appearance.logo_path, class: 'appearance-logo-preview' - if @appearance.persisted? %br - = link_to _('Remove logo'), logo_admin_application_settings_appearances_path, data: { confirm: _("Logo will be removed. Are you sure?") }, method: :delete, class: "btn gl-button btn-danger btn-danger-secondary btn-sm remove-logo" + = link_to _('Remove logo'), logo_admin_application_settings_appearances_path, data: { confirm: _("Logo will be removed. Are you sure?"), confirm_btn_variant: "danger" }, aria: { label: _('Remove logo') }, method: :delete, class: "btn gl-button btn-danger btn-danger-secondary btn-sm remove-logo" %hr = f.hidden_field :logo_cache = f.file_field :logo, class: "", accept: 'image/*' diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index f46e005dacf..7f9539c3604 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -78,9 +78,6 @@ ci_pipelines: - table: merge_requests column: merge_request_id on_delete: async_delete - - table: external_pull_requests - column: external_pull_request_id - on_delete: async_nullify - table: users column: user_id on_delete: async_nullify diff --git a/db/post_migrate/20220216201949_remove_package_files_limit_from_application_settings.rb b/db/post_migrate/20220216201949_remove_package_files_limit_from_application_settings.rb new file mode 100644 index 00000000000..589b75dd918 --- /dev/null +++ b/db/post_migrate/20220216201949_remove_package_files_limit_from_application_settings.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class RemovePackageFilesLimitFromApplicationSettings < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + return unless column_exists?(:application_settings, :max_package_files_for_package_destruction) + + remove_column :application_settings, :max_package_files_for_package_destruction, :smallint + end + + def down + add_column :application_settings, :max_package_files_for_package_destruction, :smallint, default: 100, null: false + add_check_constraint :application_settings, + 'max_package_files_for_package_destruction > 0', + 'app_settings_max_package_files_for_package_destruction_positive' + end +end diff --git a/db/schema_migrations/20220216201949 b/db/schema_migrations/20220216201949 new file mode 100644 index 00000000000..466da69ad0e --- /dev/null +++ b/db/schema_migrations/20220216201949 @@ -0,0 +1 @@ +481bc7b167ddf46bd11322e4458e48de10483bf34d0e393f7e76a3572c28e09f
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e1e6a79bccd..8437f9bb2b5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11243,7 +11243,6 @@ CREATE TABLE application_settings ( container_registry_import_max_step_duration integer DEFAULT 300 NOT NULL, container_registry_import_target_plan text DEFAULT 'free'::text NOT NULL, container_registry_import_created_before timestamp with time zone DEFAULT '2022-01-23 00:00:00+00'::timestamp with time zone NOT NULL, - max_package_files_for_package_destruction smallint DEFAULT 100 NOT NULL, runner_token_expiration_interval integer, group_runner_token_expiration_interval integer, project_runner_token_expiration_interval integer, @@ -11254,7 +11253,6 @@ CREATE TABLE application_settings ( CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), - CONSTRAINT app_settings_max_package_files_for_package_destruction_positive CHECK ((max_package_files_for_package_destruction > 0)), CONSTRAINT app_settings_p_cleanup_package_file_worker_capacity_positive CHECK ((packages_cleanup_package_file_worker_capacity >= 0)), CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)), CONSTRAINT app_settings_yaml_max_depth_positive CHECK ((max_yaml_depth > 0)), diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index e0209b15453..e9a7982ebd5 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -8595,6 +8595,7 @@ Describes an alert from the project's Alert Management. | <a id="alertmanagementalertstatus"></a>`status` | [`AlertManagementStatus`](#alertmanagementstatus) | Status of the alert. | | <a id="alertmanagementalerttitle"></a>`title` | [`String`](#string) | Title of the alert. | | <a id="alertmanagementalertupdatedat"></a>`updatedAt` | [`Time`](#time) | Timestamp the alert was last updated. | +| <a id="alertmanagementalertweburl"></a>`webUrl` | [`String!`](#string) | URL of the alert. | #### Fields with arguments @@ -9991,6 +9992,7 @@ A single design. | <a id="designnotes"></a>`notes` | [`NoteConnection!`](#noteconnection) | All notes on this noteable. (see [Connections](#connections)) | | <a id="designnotescount"></a>`notesCount` | [`Int!`](#int) | Total count of user-created notes for this design. | | <a id="designproject"></a>`project` | [`Project!`](#project) | Project the design belongs to. | +| <a id="designweburl"></a>`webUrl` | [`String!`](#string) | URL of the design. | #### Fields with arguments @@ -15901,6 +15903,7 @@ Representing a to-do entry. | <a id="todoid"></a>`id` | [`ID!`](#id) | ID of the to-do item. | | <a id="todoproject"></a>`project` | [`Project`](#project) | Project this to-do item is associated with. | | <a id="todostate"></a>`state` | [`TodoStateEnum!`](#todostateenum) | State of the to-do item. | +| <a id="todotarget"></a>`target` | [`Todoable!`](#todoable) | Target of the to-do item. | | <a id="todotargettype"></a>`targetType` | [`TodoTargetEnum!`](#todotargetenum) | Target type of the to-do item. | ### `Topic` @@ -19414,6 +19417,25 @@ Returns [`TimeboxReport`](#timeboxreport). | ---- | ---- | ----------- | | <a id="timeboxreportinterfacereportfullpath"></a>`fullPath` | [`String`](#string) | Full path of the project or group used as a scope for report. For example, `gitlab-org` or `gitlab-org/gitlab`. | +#### `Todoable` + +Implementations: + +- [`AlertManagementAlert`](#alertmanagementalert) +- [`BoardEpic`](#boardepic) +- [`Commit`](#commit) +- [`Design`](#design) +- [`Epic`](#epic) +- [`EpicIssue`](#epicissue) +- [`Issue`](#issue) +- [`MergeRequest`](#mergerequest) + +##### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="todoableweburl"></a>`webUrl` | [`String`](#string) | URL of this object. | + #### `User` Representation of a GitLab user. diff --git a/doc/user/compliance/license_compliance/index.md b/doc/user/compliance/license_compliance/index.md index 504f798c439..a2172b72572 100644 --- a/doc/user/compliance/license_compliance/index.md +++ b/doc/user/compliance/license_compliance/index.md @@ -55,7 +55,7 @@ You can view and modify existing policies from the [policies](#policies) tab. ## License expressions -GitLab has limited support for [composite licenses](https://spdx.github.io/spdx-spec/appendix-IV-SPDX-license-expressions/). +GitLab has limited support for [composite licenses](https://spdx.github.io/spdx-spec/SPDX-license-expressions/). License compliance can read multiple licenses, but always considers them combined using the `AND` operator. For example, if a dependency has two licenses, and one of them is allowed and the other is denied by the project [policy](#policies), GitLab evaluates the composite license as _denied_, as this is the safer option. @@ -90,7 +90,7 @@ The reported licenses might be incomplete or inaccurate. | Objective-C, Swift | [Carthage](https://github.com/Carthage/Carthage), [CocoaPods](https://cocoapods.org/) v0.39 and below | | Elixir | [Mix](https://elixir-lang.org/getting-started/mix-otp/introduction-to-mix.html) | | C++/C | [Conan](https://conan.io/) | -| Rust | [Cargo](https://crates.io) | +| Rust | [Cargo](https://crates.io/) | | PHP | [Composer](https://getcomposer.org/) | ## Enable License Compliance @@ -219,7 +219,7 @@ license_scanning: MAVEN_CLI_OPTS: --debug ``` -`mvn install` runs through all of the [build life cycle](http://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html) +`mvn install` runs through all of the [build life cycle](https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html) stages prior to `install`, including `test`. Running unit tests is not directly necessary for the license scanning purposes and consumes time, so it's skipped by having the default value of `MAVEN_CLI_OPTS` as `-DskipTests`. If you want @@ -249,7 +249,7 @@ license_scanning: Alternatively, you can use a Java key store to verify the TLS connection. For instructions on how to generate a key store file, see the -[Maven Guide to Remote repository access through authenticated HTTPS](http://maven.apache.org/guides/mini/guide-repository-ssl.html). +[Maven Guide to Remote repository access through authenticated HTTPS](https://maven.apache.org/guides/mini/guide-repository-ssl.html). ### Selecting the version of Java diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index d88453fb7d6..9fddb80d8bf 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -191,7 +191,7 @@ GitLab.com uses the IP ranges `34.74.90.64/28` and `34.74.226.0/24` for traffic fleet. This whole range is solely allocated to GitLab. You can expect connections from webhooks or repository mirroring to come from those IPs and allow them. -GitLab.com is fronted by Cloudflare. For incoming connections to GitLab.com, you might need to allow CIDR blocks of Cloudflare ([IPv4](https://www.cloudflare.com/ips-v4) and [IPv6](https://www.cloudflare.com/ips-v6)). +GitLab.com is fronted by Cloudflare. For incoming connections to GitLab.com, you might need to allow CIDR blocks of Cloudflare ([IPv4](https://www.cloudflare.com/ips-v4/) and [IPv6](https://www.cloudflare.com/ips-v6/)). For outgoing connections from CI/CD runners, we are not providing static IP addresses. All GitLab.com shared runners are deployed into Google Cloud Platform (GCP). Any diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index d1834659204..8ebcd9f62d0 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -176,7 +176,7 @@ See the [troubleshooting page](../../../administration/troubleshooting/group_sam ### Okta setup notes -Please follow the Okta documentation on [setting up a SAML application in Okta](https://developer.okta.com/docs/guides/build-sso-integration/saml2/overview/) with the notes below for consideration. +Please follow the Okta documentation on [setting up a SAML application in Okta](https://developer.okta.com/docs/guides/build-sso-integration/saml2/main/) with the notes below for consideration. <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> For a demo of the Okta SAML setup including SCIM, see [Demo: Okta Group SAML & SCIM setup](https://youtu.be/0ES9HsZq0AQ). diff --git a/doc/user/group/saml_sso/scim_setup.md b/doc/user/group/saml_sso/scim_setup.md index f2bf27d0633..2e853452015 100644 --- a/doc/user/group/saml_sso/scim_setup.md +++ b/doc/user/group/saml_sso/scim_setup.md @@ -101,7 +101,7 @@ Once synchronized, changing the field mapped to `id` and `externalId` may cause ### Okta configuration steps Before you start this section, complete the [GitLab configuration](#gitlab-configuration) process. -Make sure that you've also set up a SAML application for [Okta](https://developer.okta.com/docs/guides/build-sso-integration/saml2/overview/), +Make sure that you've also set up a SAML application for [Okta](https://developer.okta.com/docs/guides/build-sso-integration/saml2/main/), as described in the [Okta setup notes](index.md#okta-setup-notes) Make sure that the Okta setup matches our documentation exactly, especially the NameID diff --git a/doc/user/group/value_stream_analytics/index.md b/doc/user/group/value_stream_analytics/index.md index be9b502a58b..6942b9da393 100644 --- a/doc/user/group/value_stream_analytics/index.md +++ b/doc/user/group/value_stream_analytics/index.md @@ -24,6 +24,7 @@ Value stream analytics is also available for [projects](../../analytics/value_st > - Date range filter [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13216) in GitLab 12.4 > - Filtering [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13216) in GitLab 13.3 +> - Horizontal stage path [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12196) in 13.0 and [feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/323982) in 13.12 You must have at least the Reporter role to view value stream analytics for groups. @@ -45,6 +46,9 @@ To view value stream analytics for your group: header. The header name differs based on the stage you select. - To sort by most or least amount of time spent in each stage, select the **Time** header. +A badge next to the workflow items table header shows the number of workflow items that +completed during the selected stage. + The table shows a list of related workflow items for the selected stage. Based on the stage you select, this can be: - CI/CD jobs @@ -52,9 +56,6 @@ The table shows a list of related workflow items for the selected stage. Based o - Merge requests - Pipelines -A badge next to the workflow items table header shows the number of workflow items that -completed the selected stage. - ## View metrics for each development stage > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/210315) in GitLab 13.0. @@ -244,7 +245,8 @@ You can change the name of a project environment in your GitLab CI/CD configurat > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12196) in GitLab 12.9. Use custom value streams to create custom stages that align with your own development processes, -and hide default stages. +and hide default stages. The dashboard depicts stages as a horizontal process +flow. ### Create a value stream @@ -263,12 +265,12 @@ To create a value stream: 1. On the left sidebar, select **Analytics > Value Stream**. 1. In the top right, select the dropdown list and then **Create new Value Stream**. 1. Enter a name for the new Value Stream. - - You can [customize the stages](#creating-a-value-stream-with-stages). + - You can [customize the stages](#create-a-value-stream-with-stages). 1. Select **Create Value Stream**.  -### Creating a value stream with stages +### Create a value stream with stages > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50229) in GitLab 13.7. > - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55572) in GitLab 13.10. @@ -320,7 +322,7 @@ After you create a value stream, you can customize it to suit your purposes. To 1. On the top bar, select **Menu > Groups** and find your group. 1. On the left sidebar, select **Analytics > Value Stream**. 1. In the top right, select the dropdown list and then select the relevant value stream. -1. Next to the value stream dropdown, select **Edit**. +1. Next to the value stream dropdown list, select **Edit**. The edit form is populated with the value stream details. 1. Optional: - Rename the value stream. @@ -345,20 +347,27 @@ To delete a custom value stream:  -## Total time chart +## View number of days for a cycle to complete > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/21631) in GitLab 12.6. > - Chart median line [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/235455) in GitLab 13.4. > - Totals [replaced](https://gitlab.com/gitlab-org/gitlab/-/issues/262070) with averages in GitLab 13.12. -This chart visually depicts the average number of days it takes for cycles to be completed. +The **Total time chart** shows the average number of days it takes for development cycles to complete. +The chart shows data for the last 500 workflow items. -This chart uses the global page filters for displaying data based on the selected -group, projects, and time frame. - -When a stage is selected the chart only displays data relevant to the selected stage. On the overview the chart displays a sum of the times for all stages in the value stream. - -The chart data is limited to the last 500 items. +1. On the top bar, select **Menu > Groups** and find your group. +1. On the left sidebar, select **Analytics > Value stream**. +1. Above the **Filter results** box, select a stage: + - To view a summary of the cycle time for all stages, select **Overview**. + - To view the cycle time for specific stage, select a stage. +1. Optional. Filter the results: + 1. Select the **Filter results** text box. + 1. Select a parameter. + 1. Select a value or enter text to refine the results. + 1. To adjust the date range: + - In the **From** field, select a start date. + - In the **To** field, select an end date. ## Type of work - Tasks by type chart diff --git a/doc/user/infrastructure/iac/index.md b/doc/user/infrastructure/iac/index.md index 8ccd4eb45fd..90014d81848 100644 --- a/doc/user/infrastructure/iac/index.md +++ b/doc/user/infrastructure/iac/index.md @@ -60,9 +60,9 @@ This video from January 2021 walks you through all the GitLab Terraform integrat ## GitLab-managed Terraform state -[Terraform remote backends](https://www.terraform.io/docs/language/settings/backends/index.html) +[Terraform remote backends](https://www.terraform.io/language/settings/backends) enable you to store the state file in a remote, shared store. GitLab uses the -[Terraform HTTP backend](https://www.terraform.io/docs/language/settings/backends/http.html) +[Terraform HTTP backend](https://www.terraform.io/language/settings/backends/http) to securely store the state files in local storage (the default) or [the remote store of your choice](../../../administration/terraform_state.md). diff --git a/doc/user/infrastructure/iac/terraform_state.md b/doc/user/infrastructure/iac/terraform_state.md index 8fd38215b04..39a57b60787 100644 --- a/doc/user/infrastructure/iac/terraform_state.md +++ b/doc/user/infrastructure/iac/terraform_state.md @@ -8,9 +8,9 @@ info: To determine the technical writer assigned to the Stage/Group associated w > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2673) in GitLab 13.0. -[Terraform remote backends](https://www.terraform.io/docs/language/settings/backends/index.html) +[Terraform remote backends](https://www.terraform.io/language/settings/backends) enable you to store the state file in a remote, shared store. GitLab uses the -[Terraform HTTP backend](https://www.terraform.io/docs/language/settings/backends/http.html) +[Terraform HTTP backend](https://www.terraform.io/language/settings/backends/http) to securely store the state files in local storage (the default) or [the remote store of your choice](../../../administration/terraform_state.md). @@ -222,9 +222,9 @@ See [this reference project](https://gitlab.com/gitlab-org/configure/examples/gi ## Using a GitLab-managed Terraform state backend as a remote data source You can use a GitLab-managed Terraform state as a -[Terraform data source](https://www.terraform.io/docs/language/state/remote-state-data.html). +[Terraform data source](https://www.terraform.io/language/state/remote-state-data). To use your existing Terraform state backend as a data source, provide the following details -as [Terraform input variables](https://www.terraform.io/docs/language/values/variables.html): +as [Terraform input variables](https://www.terraform.io/language/values/variables): - **address**: The URL of the remote state backend you want to use as a data source. For example, `https://gitlab.com/api/v4/projects/<TARGET-PROJECT-ID>/terraform/state/<TARGET-STATE-NAME>`. diff --git a/doc/user/markdown.md b/doc/user/markdown.md index fa739843e3b..c81fdc275d9 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -30,7 +30,7 @@ and the [main GitLab website](https://about.gitlab.com) use [Kramdown](https://k You should not view this page in the documentation, but instead [view these styles as they appear on GitLab](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/user/markdown.md). GitLab Flavored Markdown extends the [CommonMark specification](https://spec.commonmark.org/current/). -It was inspired by [GitHub Flavored Markdown](https://docs.github.com/en/github/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax). +It was inspired by [GitHub Flavored Markdown](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax). ## Where you can use GitLab Flavored Markdown diff --git a/doc/user/project/import/clearcase.md b/doc/user/project/import/clearcase.md index 120c64e00f2..867477c83b2 100644 --- a/doc/user/project/import/clearcase.md +++ b/doc/user/project/import/clearcase.md @@ -51,4 +51,4 @@ are some useful links to get you started: - [ClearCase to Git](https://therub.org/2013/07/19/clearcase-to-git/) - [Dual syncing ClearCase to Git](https://therub.org/2013/10/22/dual-syncing-clearcase-and-git/) - [Moving to Git from ClearCase](https://sateeshkumarb.wordpress.com/2011/01/15/moving-to-git-from-clearcase/) -- [ClearCase to Git webinar](https://www.brighttalk.com/webcast/11817/162473/clearcase-to-git) +- [ClearCase to Git webinar](https://www.brighttalk.com/webcast/11817/162473) diff --git a/doc/user/project/import/cvs.md b/doc/user/project/import/cvs.md index 55c5feff1f0..8bb716d8122 100644 --- a/doc/user/project/import/cvs.md +++ b/doc/user/project/import/cvs.md @@ -71,6 +71,6 @@ Here's a few links to get you started with the migration: - [Migrate using the `cvs-fast-export` tool](https://gitlab.com/esr/cvs-fast-export) - [Stack Overflow post on importing the CVS repository](https://stackoverflow.com/a/11490134/974710) -- [Convert a CVS repository to Git](https://www.techrepublic.com/blog/linux-and-open-source/convert-cvs-repositories-to-git/) +- [Convert a CVS repository to Git](http://www.techrepublic.com/article/convert-cvs-repositories-to-git/) - [Man page of the `git-cvsimport` tool](https://mirrors.edge.kernel.org/pub/software/scm/git/docs/git-cvsimport.html) - [Migrate using `reposurgeon`](http://www.catb.org/~esr/reposurgeon/repository-editing.html#conversion) diff --git a/doc/user/project/integrations/mattermost_slash_commands.md b/doc/user/project/integrations/mattermost_slash_commands.md index 0e972f0aa63..768acb02ee6 100644 --- a/doc/user/project/integrations/mattermost_slash_commands.md +++ b/doc/user/project/integrations/mattermost_slash_commands.md @@ -165,4 +165,4 @@ different types of notifications. All events are sent to the specified channel. ## Further reading - [Mattermost slash commands documentation](https://docs.mattermost.com/developer/slash-commands.html) -- [Omnibus GitLab Mattermost](https://docs.gitlab.com/omnibus/gitlab-mattermost/) +- [Omnibus GitLab Mattermost](../../../integration/mattermost/) diff --git a/doc/user/project/pages/custom_domains_ssl_tls_certification/dns_concepts.md b/doc/user/project/pages/custom_domains_ssl_tls_certification/dns_concepts.md index 165131d50a4..3491346f7d9 100644 --- a/doc/user/project/pages/custom_domains_ssl_tls_certification/dns_concepts.md +++ b/doc/user/project/pages/custom_domains_ssl_tls_certification/dns_concepts.md @@ -37,7 +37,7 @@ for the most popular hosting services: - [Cloudflare](https://support.cloudflare.com/hc/en-us/articles/201720164-Creating-a-Cloudflare-account-and-adding-a-website) - [cPanel](https://documentation.cpanel.net/display/84Docs/Edit+DNS+Zone) - [DigitalOcean](https://docs.digitalocean.com/products/networking/dns/how-to/manage-records/) -- [DreamHost](https://help.dreamhost.com/hc/en-us/articles/215414867-How-do-I-add-custom-DNS-records-) +- [DreamHost](https://help.dreamhost.com/hc/en-us/articles/360035516812) - [Gandi](https://docs.gandi.net/en/domain_names/faq/dns_records.html) - [Go Daddy](https://www.godaddy.com/help/add-an-a-record-19238) - [Hostgator](https://www.hostgator.com/help/article/changing-dns-records) diff --git a/doc/user/project/pages/custom_domains_ssl_tls_certification/index.md b/doc/user/project/pages/custom_domains_ssl_tls_certification/index.md index ec66dce41f9..4d8919090a2 100644 --- a/doc/user/project/pages/custom_domains_ssl_tls_certification/index.md +++ b/doc/user/project/pages/custom_domains_ssl_tls_certification/index.md @@ -315,7 +315,7 @@ To enable this setting: 1. Tick the checkbox **Force HTTPS (requires valid certificates)**. If you use Cloudflare CDN in front of GitLab Pages, make sure to set the SSL connection setting to -`full` instead of `flexible`. For more details, see the [Cloudflare CDN directions](https://support.cloudflare.com/hc/en-us/articles/200170416-End-to-end-HTTPS-with-Cloudflare-Part-3-SSL-options#h_4e0d1a7c-eb71-4204-9e22-9d3ef9ef7fef). +`full` instead of `flexible`. For more details, see the [Cloudflare CDN directions](https://developers.cloudflare.com/ssl/origin-configuration/ssl-modes#h_4e0d1a7c-eb71-4204-9e22-9d3ef9ef7fef). <!-- ## Troubleshooting diff --git a/doc/user/project/releases/index.md b/doc/user/project/releases/index.md index 8049ee9726d..124191cc5af 100644 --- a/doc/user/project/releases/index.md +++ b/doc/user/project/releases/index.md @@ -42,7 +42,7 @@ To view a list of releases: - On the left sidebar, select **Deployments > Releases**, or -- On the project's overview page, if at least one release exists, click the number of releases. +- On the project's overview page, if at least one release exists, select the number of releases.  @@ -52,10 +52,10 @@ To view a list of releases: ### Sort releases -To sort releases by **Released date** or **Created date**, select from the sort order dropdown. To +To sort releases by **Released date** or **Created date**, select from the sort order dropdown list. To switch between ascending or descending order, select **Sort order**. - + ## Create a release @@ -307,9 +307,9 @@ Read more about [Release permissions](#release-permissions). To edit the details of a release: 1. On the left sidebar, select **Deployments > Releases**. -1. In the top-right corner of the release you want to modify, click **Edit this release** (the pencil icon). +1. In the top-right corner of the release you want to modify, select **Edit this release** (the pencil icon). 1. On the **Edit Release** page, change the release's details. -1. Click **Save changes**. +1. Select **Save changes**. You can edit the release title, notes, associated milestones, and asset links. To change the release date use the @@ -330,16 +330,16 @@ the [Releases API](../../../api/releases/index.md#create-a-release). In the user interface, to associate milestones to a release: 1. On the left sidebar, select **Deployments > Releases**. -1. In the top-right corner of the release you want to modify, click **Edit this release** (the pencil icon). +1. In the top-right corner of the release you want to modify, select **Edit this release** (the pencil icon). 1. From the **Milestones** list, select each milestone you want to associate. You can select multiple milestones. -1. Click **Save changes**. +1. Select **Save changes**. On the **Deployments > Releases** page, the **Milestone** is listed in the top section, along with statistics about the issues in the milestones.  -Releases are also visible on the **Issues > Milestones** page, and when you click a milestone +Releases are also visible on the **Issues > Milestones** page, and when you select a milestone on this page. Here is an example of milestones with no releases, one release, and two releases, respectively. @@ -360,8 +360,8 @@ You can be notified by email when a new release is created for your project. To subscribe to notifications for releases: 1. On the left sidebar, select **Project information**. -1. Click **Notification setting** (the bell icon). -1. In the list, click **Custom**. +1. Select **Notification setting** (the bell icon). +1. In the list, select **Custom**. 1. Select the **New release** checkbox. 1. Close the dialog box to save. @@ -395,12 +395,12 @@ To set a deploy freeze window in the UI, complete these steps: 1. Sign in to GitLab as a user with the Maintainer role. 1. On the left sidebar, select **Project information**. -1. In the left navigation menu, navigate to **Settings > CI/CD**. +1. In the left navigation menu, go to **Settings > CI/CD**. 1. Scroll to **Deploy freezes**. -1. Click **Expand** to see the deploy freeze table. -1. Click **Add deploy freeze** to open the deploy freeze modal. +1. Select **Expand** to see the deploy freeze table. +1. Select **Add deploy freeze** to open the deploy freeze modal. 1. Enter the start time, end time, and time zone of the desired deploy freeze period. -1. Click **Add deploy freeze** in the modal. +1. Select **Add deploy freeze** in the modal. 1. After the deploy freeze is saved, you can edit it by selecting the edit button (**{pencil}**) and remove it by selecting the delete button (**{remove}**).  @@ -611,7 +611,7 @@ On [GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/releases), you can view t  The totals are displayed on [shields](https://shields.io/) and are generated per release by -[a Rake task in the `www-gitlab-com` repo](https://gitlab.com/gitlab-com/www-gitlab-com/-/blob/master/lib/tasks/update_gitlab_project_releases_page.rake). +[a Rake task in the `www-gitlab-com` repository](https://gitlab.com/gitlab-com/www-gitlab-com/-/blob/master/lib/tasks/update_gitlab_project_releases_page.rake). | Item | Formula | | ------ | ------ | @@ -634,7 +634,7 @@ This data is saved in a JSON file and called *release evidence*. The feature includes test artifacts and linked milestones to facilitate internal processes, like external audits. -To access the release evidence, on the Releases page, click the link to the JSON file that's listed +To access the release evidence, on the Releases page, select the link to the JSON file that's listed under the **Evidence collection** heading. You can also [use the API](../../../api/releases/index.md#collect-release-evidence) to @@ -720,7 +720,7 @@ When you create a release, if [job artifacts](../../../ci/yaml/index.md#artifact Although job artifacts normally expire, artifacts included in release evidence do not expire. -To enable job artifact collection you need to specify both: +To enable job artifact collection you must specify both: 1. [`artifacts:paths`](../../../ci/yaml/index.md#artifactspaths) 1. [`artifacts:reports`](../../../ci/yaml/index.md#artifactsreports) diff --git a/doc/user/project/repository/mirror/push.md b/doc/user/project/repository/mirror/push.md index 221616bd41c..ebc4430ac53 100644 --- a/doc/user/project/repository/mirror/push.md +++ b/doc/user/project/repository/mirror/push.md @@ -76,7 +76,7 @@ through the [remote mirrors API](../../../../api/remote_mirrors.md). To configure a mirror from GitLab to GitHub: -1. Create a [GitHub personal access token](https://docs.github.com/en/github/authenticating-to-github/keeping-your-account-and-data-secure/creating-a-personal-access-token) +1. Create a [GitHub personal access token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token) with `public_repo` selected. 1. Enter a **Git repository URL** with this format: `https://<your_access_token>@github.com/<github_group>/<github_project>.git`. diff --git a/doc/user/project/repository/x509_signed_commits/index.md b/doc/user/project/repository/x509_signed_commits/index.md index 27ef14d31c5..f319c1d8fd3 100644 --- a/doc/user/project/repository/x509_signed_commits/index.md +++ b/doc/user/project/repository/x509_signed_commits/index.md @@ -24,11 +24,11 @@ The main difference is the way GitLab determines whether or not the developer's to their account. GitLab uses its own certificate store and therefore defines the -[trust chain](https://www.ssl.com/faqs/what-is-a-chain-of-trust/). +[trust chain](https://www.ssl.com/faqs/what-is-a-certificate-authority/). For a commit or tag to be *verified* by GitLab: - The signing certificate email must match a verified email address in GitLab. -- The GitLab instance must be able to establish a full [trust chain](https://www.ssl.com/faqs/what-is-a-chain-of-trust/) +- The GitLab instance must be able to establish a full trust chain from the certificate in the signature to a trusted certificate in the GitLab certificate store. This chain may include intermediate certificates supplied in the signature. You may need to add certificates, such as Certificate Authority root certificates, diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 8dd1f686132..06c81fd65dd 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -37,10 +37,12 @@ module Gitlab next unless dependencies.present? next unless needs_value.present? - missing_needs = dependencies - needs_value[:job].pluck(:name) # rubocop:disable CodeReuse/ActiveRecord (Array#pluck) + if needs_value[:job].nil? && needs_value[:cross_dependency].present? + errors.add(:needs, "corresponding to dependencies must be from the same pipeline") + else + missing_needs = dependencies - needs_value[:job].pluck(:name) # rubocop:disable CodeReuse/ActiveRecord (Array#pluck) - if missing_needs.any? - errors.add(:dependencies, "the #{missing_needs.join(", ")} should be part of needs") + errors.add(:dependencies, "the #{missing_needs.join(", ")} should be part of needs") if missing_needs.any? end end end diff --git a/lib/gitlab/graphql/loaders/batch_commit_loader.rb b/lib/gitlab/graphql/loaders/batch_commit_loader.rb new file mode 100644 index 00000000000..26c1f61c567 --- /dev/null +++ b/lib/gitlab/graphql/loaders/batch_commit_loader.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Loaders + class BatchCommitLoader + def initialize(container_class:, container_id:, oid:) + @container_class = container_class + @container_id = container_id + @oid = oid + end + + def find + Gitlab::Graphql::Lazy.with_value(find_containers) do |container| + BatchLoader::GraphQL.for(oid).batch(key: container) do |oids, loader, args| + container = args[:key] + + container.repository.commits_by(oids: oids).each do |commit| + loader.call(commit.id, commit) if commit + end + end + end + end + + private + + def find_containers + BatchLoader::GraphQL.for(container_id.to_i).batch(key: container_class) do |ids, loader, args| + model = args[:key] + results = model.includes(:route).id_in(ids) # rubocop: disable CodeReuse/ActiveRecord + + results.each { |record| loader.call(record.id, record) } + end + end + + attr_reader :container_class, :container_id, :oid + end + end + end +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index c159768690e..00985dbd77f 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -619,11 +619,11 @@ RSpec.describe Projects::IssuesController do end end - context 'when the SpamVerdictService disallows' do + context 'when an issue is identified as spam' do before do stub_application_setting(recaptcha_enabled: true) - expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service| - expect(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW) + allow_next_instance_of(Spam::AkismetService) do |akismet_service| + allow(akismet_service).to receive(:spam?).and_return(true) end end @@ -940,8 +940,8 @@ RSpec.describe Projects::IssuesController do context 'when an issue is identified as spam' do context 'when recaptcha is not verified' do before do - expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service| - expect(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW) + allow_next_instance_of(Spam::AkismetService) do |akismet_service| + allow(akismet_service).to receive(:spam?).and_return(true) end end @@ -1274,11 +1274,11 @@ RSpec.describe Projects::IssuesController do end end - context 'when SpamVerdictService requires recaptcha' do + context 'when an issue is identified as spam and requires recaptcha' do context 'when captcha is not verified' do before do - expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service| - expect(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW) + allow_next_instance_of(Spam::AkismetService) do |akismet_service| + allow(akismet_service).to receive(:spam?).and_return(true) end end diff --git a/spec/features/issues/spam_akismet_issue_creation_spec.rb b/spec/features/issues/spam_akismet_issue_creation_spec.rb new file mode 100644 index 00000000000..4cc4c4cf607 --- /dev/null +++ b/spec/features/issues/spam_akismet_issue_creation_spec.rb @@ -0,0 +1,178 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Spam detection on issue creation', :js do + include StubENV + + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + + include_context 'includes Spam constants' + + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + + Gitlab::CurrentSettings.update!( + akismet_enabled: true, + akismet_api_key: 'testkey', + spam_check_api_key: 'testkey', + recaptcha_enabled: true, + recaptcha_site_key: 'test site key', + recaptcha_private_key: 'test private key' + ) + + project.add_maintainer(user) + sign_in(user) + visit new_project_issue_path(project) + + fill_in 'issue_title', with: 'issue title' + fill_in 'issue_description', with: 'issue description' + end + + shared_examples 'disallows issue creation' do + it 'disallows issue creation' do + click_button 'Create issue' + + expect(page).to have_content('discarded') + expect(page).not_to have_css('.recaptcha') + expect(page).not_to have_content('issue title') + end + end + + shared_examples 'allows issue creation with CAPTCHA' do + it 'allows issue creation' do + click_button 'Create issue' + + # it is impossible to test reCAPTCHA automatically and there is no possibility to fill in recaptcha + # reCAPTCHA verification is skipped in test environment and it always returns true + expect(page).not_to have_content('issue title') + expect(page).to have_css('.recaptcha') + + click_button 'Create issue' + + expect(page.find('.issue-details h1.title')).to have_content('issue title') + expect(page.find('.issue-details .description')).to have_content('issue description') + end + end + + shared_examples 'allows issue creation without CAPTCHA' do + it 'allows issue creation without need to solve CAPTCHA' do + click_button 'Create issue' + + expect(page).not_to have_css('.recaptcha') + expect(page.find('.issue-details h1.title')).to have_content('issue title') + expect(page.find('.issue-details .description')).to have_content('issue description') + end + end + + shared_examples 'creates a spam_log record' do + it 'creates a spam_log record' do + expect { click_button 'Create issue' } + .to log_spam(title: 'issue title', description: 'issue description', user_id: user.id, noteable_type: 'Issue') + end + end + + shared_examples 'does not create a spam_log record' do + it 'does not creates a spam_log record' do + expect { click_button 'Create issue' } + .not_to log_spam(title: 'issue title', description: 'issue description', user_id: user.id, noteable_type: 'Issue') + end + end + + shared_context 'when spammable is identified as possible spam' do + before do + allow_next_instance_of(Spam::AkismetService) do |akismet_service| + allow(akismet_service).to receive(:spam?).and_return(true) + end + end + end + + shared_context 'when spammable is not identified as possible spam' do + before do + allow_next_instance_of(Spam::AkismetService) do |akismet_service| + allow(akismet_service).to receive(:spam?).and_return(false) + end + end + end + + shared_context 'when CAPTCHA is enabled' do + before do + stub_application_setting(recaptcha_enabled: true) + end + end + + shared_context 'when CAPTCHA is not enabled' do + before do + stub_application_setting(recaptcha_enabled: false) + end + end + + shared_context 'when allow_possible_spam feature flag is true' do + before do + stub_feature_flags(allow_possible_spam: true) + end + end + + shared_context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end + end + + describe 'spam handling' do + # verdict, spam_flagged, captcha_enabled, allow_possible_spam_flag, creates_spam_log + # TODO: Add example for BLOCK_USER verdict when we add support for testing SpamCheck - see https://gitlab.com/groups/gitlab-org/-/epics/5527#lacking-coverage-for-spamcheck-vs-akismet + # DISALLOW, true, false, false, true + # CONDITIONAL_ALLOW, true, true, false, true + # OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM, true, true, true, true + # OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM, true, false, true, true + # ALLOW, false, true, false, false + # TODO: Add example for NOOP verdict when we add support for testing SpamCheck - see https://gitlab.com/groups/gitlab-org/-/epics/5527#lacking-coverage-for-spamcheck-vs-akismet + + context 'DISALLOW: spam_flagged=true, captcha_enabled=true, allow_possible_spam=true' do + include_context 'when spammable is identified as possible spam' + include_context 'when CAPTCHA is enabled' + include_context 'when allow_possible_spam feature flag is true' + + it_behaves_like 'allows issue creation without CAPTCHA' + it_behaves_like 'creates a spam_log record' + end + + context 'CONDITIONAL_ALLOW: spam_flagged=true, captcha_enabled=true, allow_possible_spam=false' do + include_context 'when spammable is identified as possible spam' + include_context 'when CAPTCHA is enabled' + include_context 'when allow_possible_spam feature flag is false' + + it_behaves_like 'allows issue creation with CAPTCHA' + it_behaves_like 'creates a spam_log record' + end + + context 'OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM: spam_flagged=true, captcha_enabled=true, allow_possible_spam=true' do + include_context 'when spammable is identified as possible spam' + include_context 'when CAPTCHA is enabled' + include_context 'when allow_possible_spam feature flag is true' + + it_behaves_like 'allows issue creation without CAPTCHA' + it_behaves_like 'creates a spam_log record' + end + + context 'OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM: spam_flagged=true, captcha_enabled=false, allow_possible_spam=true' do + include_context 'when spammable is identified as possible spam' + include_context 'when CAPTCHA is not enabled' + include_context 'when allow_possible_spam feature flag is true' + + it_behaves_like 'allows issue creation without CAPTCHA' + it_behaves_like 'creates a spam_log record' + end + + context 'ALLOW: spam_flagged=false, captcha_enabled=true, allow_possible_spam=false' do + include_context 'when spammable is not identified as possible spam' + include_context 'when CAPTCHA is not enabled' + include_context 'when allow_possible_spam feature flag is false' + + it_behaves_like 'allows issue creation without CAPTCHA' + it_behaves_like 'does not create a spam_log record' + end + end +end diff --git a/spec/features/issues/spam_issues_spec.rb b/spec/features/issues/spam_issues_spec.rb deleted file mode 100644 index de9888295b9..00000000000 --- a/spec/features/issues/spam_issues_spec.rb +++ /dev/null @@ -1,188 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'New issue', :js do - include StubENV - - let(:project) { create(:project, :public) } - let(:user) { create(:user)} - - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - - Gitlab::CurrentSettings.update!( - akismet_enabled: true, - akismet_api_key: 'testkey', - spam_check_api_key: 'testkey', - recaptcha_enabled: true, - recaptcha_site_key: 'test site key', - recaptcha_private_key: 'test private key' - ) - - project.add_maintainer(user) - sign_in(user) - end - - context 'when SpamVerdictService disallows' do - include_context 'includes Spam constants' - - before do - allow_next_instance_of(Spam::SpamVerdictService) do |verdict_service| - allow(verdict_service).to receive(:execute).and_return(DISALLOW) - end - - visit new_project_issue_path(project) - end - - context 'when allow_possible_spam feature flag is false' do - before do - stub_feature_flags(allow_possible_spam: false) - - fill_in 'issue_title', with: 'issue title' - fill_in 'issue_description', with: 'issue description' - end - - it 'rejects issue creation' do - click_button 'Create issue' - - expect(page).to have_content('discarded') - expect(page).not_to have_content('potential spam') - expect(page).not_to have_content('issue title') - end - - it 'creates a spam log record' do - expect { click_button 'Create issue' } - .to log_spam(title: 'issue title', description: 'issue description', user_id: user.id, noteable_type: 'Issue') - end - end - - context 'when allow_possible_spam feature flag is true' do - before do - fill_in 'issue_title', with: 'issue title' - fill_in 'issue_description', with: 'issue description' - end - - it 'allows issue creation' do - click_button 'Create issue' - - expect(page.find('.issue-details h1.title')).to have_content('issue title') - expect(page.find('.issue-details .description')).to have_content('issue description') - end - - it 'creates a spam log record' do - expect { click_button 'Create issue' } - .to log_spam(title: 'issue title', description: 'issue description', user_id: user.id, noteable_type: 'Issue') - end - end - end - - context 'when SpamVerdictService requires recaptcha' do - include_context 'includes Spam constants' - - before do - allow_next_instance_of(Spam::SpamVerdictService) do |verdict_service| - allow(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW) - end - - visit new_project_issue_path(project) - end - - context 'when recaptcha is enabled' do - before do - stub_application_setting(recaptcha_enabled: true) - end - - context 'when allow_possible_spam feature flag is false' do - before do - stub_feature_flags(allow_possible_spam: false) - end - - it 'creates an issue after solving reCaptcha' do - fill_in 'issue_title', with: 'issue title' - fill_in 'issue_description', with: 'issue description' - - click_button 'Create issue' - - # it is impossible to test reCAPTCHA automatically and there is no possibility to fill in recaptcha - # reCAPTCHA verification is skipped in test environment and it always returns true - expect(page).not_to have_content('issue title') - expect(page).to have_css('.recaptcha') - - click_button 'Create issue' - - expect(page.find('.issue-details h1.title')).to have_content('issue title') - expect(page.find('.issue-details .description')).to have_content('issue description') - end - end - - context 'when allow_possible_spam feature flag is true' do - before do - fill_in 'issue_title', with: 'issue title' - fill_in 'issue_description', with: 'issue description' - end - - it 'creates an issue without a need to solve reCAPTCHA' do - click_button 'Create issue' - - expect(page).not_to have_css('.recaptcha') - expect(page.find('.issue-details h1.title')).to have_content('issue title') - expect(page.find('.issue-details .description')).to have_content('issue description') - end - - it 'creates a spam log record' do - expect { click_button 'Create issue' } - .to log_spam(title: 'issue title', description: 'issue description', user_id: user.id, noteable_type: 'Issue') - end - end - end - - context 'when reCAPTCHA is not enabled' do - before do - stub_application_setting(recaptcha_enabled: false) - end - - context 'when allow_possible_spam feature flag is true' do - before do - fill_in 'issue_title', with: 'issue title' - fill_in 'issue_description', with: 'issue description' - end - - it 'creates an issue without a need to solve reCaptcha' do - click_button 'Create issue' - - expect(page).not_to have_css('.recaptcha') - expect(page.find('.issue-details h1.title')).to have_content('issue title') - expect(page.find('.issue-details .description')).to have_content('issue description') - end - - it 'creates a spam log record' do - expect { click_button 'Create issue' } - .to log_spam(title: 'issue title', description: 'issue description', user_id: user.id, noteable_type: 'Issue') - end - end - end - end - - context 'when the SpamVerdictService allows' do - include_context 'includes Spam constants' - - before do - allow_next_instance_of(Spam::SpamVerdictService) do |verdict_service| - allow(verdict_service).to receive(:execute).and_return(ALLOW) - end - - visit new_project_issue_path(project) - end - - it 'creates an issue' do - fill_in 'issue_title', with: 'issue title' - fill_in 'issue_description', with: 'issue description' - - click_button 'Create issue' - - expect(page.find('.issue-details h1.title')).to have_content('issue title') - expect(page.find('.issue-details .description')).to have_content('issue description') - end - end -end diff --git a/spec/graphql/types/alert_management/alert_type_spec.rb b/spec/graphql/types/alert_management/alert_type_spec.rb index 9ff01418c9a..69cbdb998eb 100644 --- a/spec/graphql/types/alert_management/alert_type_spec.rb +++ b/spec/graphql/types/alert_management/alert_type_spec.rb @@ -7,6 +7,8 @@ RSpec.describe GitlabSchema.types['AlertManagementAlert'] do specify { expect(described_class).to require_graphql_authorizations(:read_alert_management_alert) } + specify { expect(described_class.interfaces).to include(Types::TodoableInterface) } + it 'exposes the expected fields' do expected_fields = %i[ iid @@ -34,6 +36,7 @@ RSpec.describe GitlabSchema.types['AlertManagementAlert'] do details_url prometheus_alert environment + web_url ] expect(described_class).to have_graphql_fields(*expected_fields) diff --git a/spec/graphql/types/commit_type_spec.rb b/spec/graphql/types/commit_type_spec.rb index c1d838c3117..fe8df15028d 100644 --- a/spec/graphql/types/commit_type_spec.rb +++ b/spec/graphql/types/commit_type_spec.rb @@ -7,6 +7,8 @@ RSpec.describe GitlabSchema.types['Commit'] do specify { expect(described_class).to require_graphql_authorizations(:download_code) } + specify { expect(described_class).to include(Types::TodoableInterface) } + it 'contains attributes related to commit' do expect(described_class).to have_graphql_fields( :id, :sha, :short_id, :title, :full_title, :full_title_html, :description, :description_html, :message, :title_html, :authored_date, diff --git a/spec/graphql/types/design_management/design_type_spec.rb b/spec/graphql/types/design_management/design_type_spec.rb index cae98a013e1..9c460e9058a 100644 --- a/spec/graphql/types/design_management/design_type_spec.rb +++ b/spec/graphql/types/design_management/design_type_spec.rb @@ -5,8 +5,10 @@ require 'spec_helper' RSpec.describe GitlabSchema.types['Design'] do specify { expect(described_class.interfaces).to include(Types::CurrentUserTodos) } + specify { expect(described_class.interfaces).to include(Types::TodoableInterface) } + it_behaves_like 'a GraphQL type with design fields' do - let(:extra_design_fields) { %i[notes current_user_todos discussions versions] } + let(:extra_design_fields) { %i[notes current_user_todos discussions versions web_url] } let_it_be(:design) { create(:design, :with_versions) } let(:object_id) { GitlabSchema.id_from_object(design) } let_it_be(:object_id_b) { GitlabSchema.id_from_object(create(:design, :with_versions)) } diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb index 5ab8845246a..dbd5ab98383 100644 --- a/spec/graphql/types/merge_request_type_spec.rb +++ b/spec/graphql/types/merge_request_type_spec.rb @@ -13,6 +13,8 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do specify { expect(described_class.interfaces).to include(Types::CurrentUserTodos) } + specify { expect(described_class.interfaces).to include(Types::TodoableInterface) } + it 'has the expected fields' do expected_fields = %w[ notes discussions user_permissions id iid title title_html description diff --git a/spec/graphql/types/todo_type_spec.rb b/spec/graphql/types/todo_type_spec.rb index 15b6195ec5c..8de63ebfda5 100644 --- a/spec/graphql/types/todo_type_spec.rb +++ b/spec/graphql/types/todo_type_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe GitlabSchema.types['Todo'] do it 'has the correct fields' do - expected_fields = [:id, :project, :group, :author, :action, :target_type, :body, :state, :created_at] + expected_fields = [:id, :project, :group, :author, :action, :target, :target_type, :body, :state, :created_at] expect(described_class).to have_graphql_fields(*expected_fields) end diff --git a/spec/graphql/types/todoable_interface_spec.rb b/spec/graphql/types/todoable_interface_spec.rb new file mode 100644 index 00000000000..bafd89fbf59 --- /dev/null +++ b/spec/graphql/types/todoable_interface_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Types::TodoableInterface do + it 'exposes the expected fields' do + expected_fields = %i[ + web_url + ] + + expect(described_class).to have_graphql_fields(*expected_fields) + end + + describe ".resolve_type" do + it 'knows the correct type for objects' do + expect(described_class.resolve_type(build(:issue), {})).to eq(Types::IssueType) + expect(described_class.resolve_type(build(:merge_request), {})).to eq(Types::MergeRequestType) + expect(described_class.resolve_type(build(:design), {})).to eq(Types::DesignManagement::DesignType) + expect(described_class.resolve_type(build(:alert_management_alert), {})).to eq(Types::AlertManagement::AlertType) + expect(described_class.resolve_type(build(:commit), {})).to eq(Types::CommitType) + end + + it 'raises an error for an unknown type' do + project = build(:project) + + expect { described_class.resolve_type(project, {}) }.to raise_error("Unknown GraphQL type for #{project}") + end + end +end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 885f3eaff79..97691504abd 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -420,7 +420,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do end end - context 'when has dependencies' do + context 'when it has dependencies' do context 'that are not a array of strings' do let(:config) do { script: 'echo', dependencies: 'build-job' } @@ -433,8 +433,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do end end - context 'when has needs' do - context 'when have dependencies that are not subset of needs' do + context 'when the job has needs' do + context 'and there are dependencies that are not included in needs' do let(:config) do { stage: 'test', @@ -448,6 +448,24 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do expect(entry).not_to be_valid expect(entry.errors).to include 'job dependencies the another-job should be part of needs' end + + context 'and they are only cross pipeline needs' do + let(:config) do + { + script: 'echo', + dependencies: ['rspec'], + needs: [{ + job: 'rspec', + pipeline: 'other' + }] + } + end + + it 'adds an error for dependency keyword usage' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'job needs corresponding to dependencies must be from the same pipeline' + end + end end end diff --git a/spec/lib/gitlab/graphql/loaders/batch_commit_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/batch_commit_loader_spec.rb new file mode 100644 index 00000000000..cace2597633 --- /dev/null +++ b/spec/lib/gitlab/graphql/loaders/batch_commit_loader_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Graphql::Loaders::BatchCommitLoader do + include RepoHelpers + + describe '#find' do + let_it_be(:first_project) { create(:project, :repository) } + let_it_be(:second_project) { create(:project, :repository) } + + let_it_be(:first_commit) { first_project.commit(sample_commit.id) } + let_it_be(:second_commit) { first_project.commit(another_sample_commit.id) } + let_it_be(:third_commit) { second_project.commit(sample_big_commit.id) } + + it 'finds a commit by id' do + result = described_class.new( + container_class: Project, + container_id: first_project.id, + oid: first_commit.id + ).find + + expect(result.force).to eq(first_commit) + end + + before do + ActiveSupport::Notifications.subscribe('sql.active_record') do |event| + raise "no!" if event.payload[:sql].include?('SELECT "namespaces".') + end + end + + it 'only queries once' do + expect do + [ + described_class.new( + container_class: Project, + container_id: first_project.id, + oid: first_commit.id + ).find, + described_class.new( + container_class: Project, + container_id: first_project.id, + oid: second_commit.id + ).find, + described_class.new( + container_class: Project, + container_id: second_project.id, + oid: third_commit.id + ).find + ].map(&:force) + end.not_to exceed_query_limit(2) + end + end +end diff --git a/spec/models/external_pull_request_spec.rb b/spec/models/external_pull_request_spec.rb index 82da7cdf34b..10136dd0bdb 100644 --- a/spec/models/external_pull_request_spec.rb +++ b/spec/models/external_pull_request_spec.rb @@ -233,10 +233,6 @@ RSpec.describe ExternalPullRequest do end end - it_behaves_like 'it has loose foreign keys' do - let(:factory_name) { :external_pull_request } - end - context 'loose foreign key on external_pull_requests.project_id' do it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index dac721cbea0..925b74c5545 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -199,8 +199,8 @@ RSpec.describe API::Issues do expect(spam_service).to receive_messages(check_for_spam?: true) end - expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service| - expect(verdict_service).to receive(:execute).and_return(DISALLOW) + allow_next_instance_of(Spam::AkismetService) do |akismet_service| + allow(akismet_service).to receive(:spam?).and_return(true) end end diff --git a/spec/services/spam/spam_action_service_spec.rb b/spec/services/spam/spam_action_service_spec.rb index 8ddfa7ed3a0..bd8418d7092 100644 --- a/spec/services/spam/spam_action_service_spec.rb +++ b/spec/services/spam/spam_action_service_spec.rb @@ -170,26 +170,13 @@ RSpec.describe Spam::SpamActionService do allow(fake_verdict_service).to receive(:execute).and_return(DISALLOW) end - context 'when allow_possible_spam feature flag is false' do - before do - stub_feature_flags(allow_possible_spam: false) - end + it_behaves_like 'creates a spam log' - it 'marks as spam' do - response = subject - - expect(response.message).to match(expected_service_check_response_message) - expect(issue).to be_spam - end - end - - context 'when allow_possible_spam feature flag is true' do - it 'does not mark as spam' do - response = subject + it 'marks as spam' do + response = subject - expect(response.message).to match(expected_service_check_response_message) - expect(issue).not_to be_spam - end + expect(response.message).to match(expected_service_check_response_message) + expect(issue).to be_spam end end @@ -198,26 +185,13 @@ RSpec.describe Spam::SpamActionService do allow(fake_verdict_service).to receive(:execute).and_return(BLOCK_USER) end - context 'when allow_possible_spam feature flag is false' do - before do - stub_feature_flags(allow_possible_spam: false) - end - - it 'marks as spam' do - response = subject + it_behaves_like 'creates a spam log' - expect(response.message).to match(expected_service_check_response_message) - expect(issue).to be_spam - end - end - - context 'when allow_possible_spam feature flag is true' do - it 'does not mark as spam' do - response = subject + it 'marks as spam' do + response = subject - expect(response.message).to match(expected_service_check_response_message) - expect(issue).not_to be_spam - end + expect(response.message).to match(expected_service_check_response_message) + expect(issue).to be_spam end end @@ -226,37 +200,42 @@ RSpec.describe Spam::SpamActionService do allow(fake_verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW) end - context 'when allow_possible_spam feature flag is false' do - before do - stub_feature_flags(allow_possible_spam: false) - end + it_behaves_like 'creates a spam log' - it_behaves_like 'creates a spam log' + it 'does not mark as spam' do + response = subject - it 'does not mark as spam' do - response = subject + expect(response.message).to match(expected_service_check_response_message) + expect(issue).not_to be_spam + end - expect(response.message).to match(expected_service_check_response_message) - expect(issue).not_to be_spam - end + it 'marks as needing reCAPTCHA' do + response = subject - it 'marks as needing reCAPTCHA' do - response = subject + expect(response.message).to match(expected_service_check_response_message) + expect(issue).to be_needs_recaptcha + end + end - expect(response.message).to match(expected_service_check_response_message) - expect(issue).to be_needs_recaptcha - end + context 'when spam verdict service returns OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM' do + before do + allow(fake_verdict_service).to receive(:execute).and_return(OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM) end - context 'when allow_possible_spam feature flag is true' do - it_behaves_like 'creates a spam log' + it_behaves_like 'creates a spam log' - it 'does not mark as needing reCAPTCHA' do - response = subject + it 'does not mark as spam' do + response = subject + + expect(response.message).to match(expected_service_check_response_message) + expect(issue).not_to be_spam + end + + it 'does not mark as needing CAPTCHA' do + response = subject - expect(response.message).to match(expected_service_check_response_message) - expect(issue.needs_recaptcha).to be_falsey - end + expect(response.message).to match(expected_service_check_response_message) + expect(issue).not_to be_needs_recaptcha end end diff --git a/spec/services/spam/spam_verdict_service_spec.rb b/spec/services/spam/spam_verdict_service_spec.rb index 99047f3233b..082b8f909f9 100644 --- a/spec/services/spam/spam_verdict_service_spec.rb +++ b/spec/services/spam/spam_verdict_service_spec.rb @@ -27,6 +27,10 @@ RSpec.describe Spam::SpamVerdictService do extra_attributes end + before do + stub_feature_flags(allow_possible_spam: false) + end + describe '#execute' do subject { service.execute } @@ -114,6 +118,32 @@ RSpec.describe Spam::SpamVerdictService do end end + context 'if allow_possible_spam flag is true' do + before do + stub_feature_flags(allow_possible_spam: true) + end + + context 'and a service returns a verdict that should be overridden' do + before do + allow(service).to receive(:spamcheck_verdict).and_return([BLOCK_USER, attribs]) + end + + it 'overrides and renders the override verdict' do + expect(subject).to eq OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM + end + end + + context 'and a service returns a verdict that does not need to be overridden' do + before do + allow(service).to receive(:spamcheck_verdict).and_return([ALLOW, attribs]) + end + + it 'does not override and renders the original verdict' do + expect(subject).to eq ALLOW + end + end + end + context 'records metrics' do let(:histogram) { instance_double(Prometheus::Client::Histogram) } diff --git a/spec/support/shared_contexts/spam_constants.rb b/spec/support/shared_contexts/spam_constants.rb index e88a7c1b0df..03c5caa13b2 100644 --- a/spec/support/shared_contexts/spam_constants.rb +++ b/spec/support/shared_contexts/spam_constants.rb @@ -2,10 +2,11 @@ RSpec.shared_context 'includes Spam constants' do before do - stub_const('CONDITIONAL_ALLOW', Spam::SpamConstants::CONDITIONAL_ALLOW) + stub_const('BLOCK_USER', Spam::SpamConstants::BLOCK_USER) stub_const('DISALLOW', Spam::SpamConstants::DISALLOW) + stub_const('CONDITIONAL_ALLOW', Spam::SpamConstants::CONDITIONAL_ALLOW) + stub_const('OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM', Spam::SpamConstants::OVERRIDE_VIA_ALLOW_POSSIBLE_SPAM) stub_const('ALLOW', Spam::SpamConstants::ALLOW) - stub_const('BLOCK_USER', Spam::SpamConstants::BLOCK_USER) stub_const('NOOP', Spam::SpamConstants::NOOP) end end |