diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-23 18:08:46 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-23 18:08:46 +0300 |
commit | 3f9e1b261121f4dbd045341241f81b47356c99cf (patch) | |
tree | 32be23bd7fda0c3f891182f220f6d0399a1b41dd | |
parent | 5ad0cf26551baff8f08af8562a8d45e6ec14d71a (diff) |
Add latest changes from gitlab-org/gitlab@master
50 files changed, 457 insertions, 316 deletions
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6d1c25836b0..a2a208ea6b5 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -172,12 +172,6 @@ Lint/DuplicateMethods: - 'lib/gitlab/git/tree.rb' - 'lib/gitlab/git/wiki_page.rb' -# Offense count: 3 -Lint/InterpolationCheck: - Exclude: - - 'spec/features/issues/filtered_search/filter_issues_spec.rb' - - 'spec/services/quick_actions/interpret_service_spec.rb' - # Offense count: 122 # Configuration parameters: MaximumRangeSize. Lint/MissingCopEnableDirective: @@ -275,13 +269,6 @@ RSpec/ItBehavesLike: - 'spec/lib/gitlab/git/repository_spec.rb' - 'spec/services/notification_service_spec.rb' -# Offense count: 3 -RSpec/IteratedExpectation: - Exclude: - - 'spec/features/admin/admin_settings_spec.rb' - - 'spec/lib/gitlab/gitlab_import/client_spec.rb' - - 'spec/lib/gitlab/legacy_github_import/client_spec.rb' - # Offense count: 68 # Cop supports --auto-correct. RSpec/LetBeforeExamples: @@ -303,13 +290,6 @@ RSpec/MultipleSubjects: Exclude: - 'spec/services/merge_requests/create_from_issue_service_spec.rb' -# Offense count: 3 -RSpec/OverwritingSetup: - Exclude: - - 'spec/models/email_spec.rb' - - 'spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb' - - 'spec/services/notes/quick_actions_service_spec.rb' - # Offense count: 2018 # Cop supports --auto-correct. # Configuration parameters: Strict, EnforcedStyle, AllowedExplicitMatchers. @@ -556,13 +536,6 @@ Style/IfUnlessModifier: Style/Lambda: Enabled: false -# Offense count: 3 -# Cop supports --auto-correct. -Style/LineEndConcatenation: - Exclude: - - 'spec/lib/gitlab/gfm/reference_rewriter_spec.rb' - - 'spec/lib/gitlab/incoming_email_spec.rb' - # Offense count: 17 Style/MethodMissingSuper: Enabled: false @@ -659,14 +632,6 @@ Style/PerlBackrefs: Style/RaiseArgs: Enabled: false -# Offense count: 3 -# Cop supports --auto-correct. -Style/RedundantBegin: - Exclude: - - 'app/models/merge_request.rb' - - 'app/services/projects/import_service.rb' - - 'lib/gitlab/health_checks/base_abstract_check.rb' - # Offense count: 1 # Cop supports --auto-correct. Style/RedundantConditional: @@ -771,14 +736,6 @@ Style/TernaryParentheses: - 'spec/requests/api/pipeline_schedules_spec.rb' - 'spec/support/capybara.rb' -# Offense count: 3 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyleForMultiline. -# SupportedStylesForMultiline: comma, consistent_comma, no_comma -Style/TrailingCommaInArguments: - Exclude: - - 'spec/features/markdown/copy_as_gfm_spec.rb' - # Offense count: 2 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyleForMultiline. @@ -302,7 +302,7 @@ gem 'sentry-raven', '~> 2.9' gem 'premailer-rails', '~> 1.10.3' # LabKit: Tracing and Correlation -gem 'gitlab-labkit', '0.8.0' +gem 'gitlab-labkit', '0.9.1' # I18n gem 'ruby_parser', '~> 3.8', require: false diff --git a/Gemfile.lock b/Gemfile.lock index af8941611e4..8c4b1660e6b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -367,7 +367,7 @@ GEM github-markup (1.7.0) gitlab-chronic (0.10.5) numerizer (~> 0.2) - gitlab-labkit (0.8.0) + gitlab-labkit (0.9.1) actionpack (>= 5.0.0, < 6.1.0) activesupport (>= 5.0.0, < 6.1.0) grpc (~> 1.19) @@ -1215,7 +1215,7 @@ DEPENDENCIES gitaly (~> 1.81.0) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) - gitlab-labkit (= 0.8.0) + gitlab-labkit (= 0.9.1) gitlab-license (~> 1.0) gitlab-markup (~> 1.7.0) gitlab-net-dns (~> 0.9.1) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 60b5d9b6da8..c6d91308123 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -454,6 +454,7 @@ class ApplicationController < ActionController::Base user: -> { auth_user }, project: -> { @project }, namespace: -> { @group }, + caller_id: full_action_name, &block) end @@ -551,6 +552,10 @@ class ApplicationController < ActionController::Base end end + def full_action_name + "#{self.class.name}##{action_name}" + end + # A user requires a role and have the setup_for_company attribute set when they are part of the experimental signup # flow (executed by the Growth team). Users are redirected to the welcome page when their role is required and the # experiment is enabled for the current user. diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 7042bc7074f..50346d97d8b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -821,7 +821,7 @@ class MergeRequest < ApplicationRecord end def check_mergeability(async: false) - return if Feature.enabled?(:merge_requests_conditional_mergeability_check, default_enabled: true) && !recheck_merge_status? + return unless recheck_merge_status? check_service = MergeRequests::MergeabilityCheckService.new(self) @@ -1201,12 +1201,10 @@ class MergeRequest < ApplicationRecord end def in_locked_state - begin - lock_mr - yield - ensure - unlock_mr - end + lock_mr + yield + ensure + unlock_mr end def diverged_commits_count diff --git a/app/models/spam_log.rb b/app/models/spam_log.rb index 5b9ece8373f..2ec53f58e5f 100644 --- a/app/models/spam_log.rb +++ b/app/models/spam_log.rb @@ -12,4 +12,8 @@ class SpamLog < ApplicationRecord def text [title, description].join("\n") end + + def self.verify_recaptcha!(id:, user_id:) + find_by(id: id, user_id: user_id)&.update!(recaptcha_verified: true) + end end diff --git a/app/services/concerns/spam_check_methods.rb b/app/services/concerns/spam_check_methods.rb index 75d9759f1d1..5eb663c97ff 100644 --- a/app/services/concerns/spam_check_methods.rb +++ b/app/services/concerns/spam_check_methods.rb @@ -22,14 +22,15 @@ module SpamCheckMethods # a dirty instance, which means it should be already assigned with the new # attribute values. # rubocop:disable Gitlab/ModuleWithInstanceVariables - # rubocop: disable CodeReuse/ActiveRecord def spam_check(spammable, user) - spam_service = SpamService.new(spammable: spammable, request: @request) - - spam_service.when_recaptcha_verified(@recaptcha_verified, @api) do - user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true) - end + SpamCheckService.new( + spammable: spammable, + request: @request + ).execute( + api: @api, + recaptcha_verified: @recaptcha_verified, + spam_log_id: @spam_log_id, + user_id: user.id) end - # rubocop: enable CodeReuse/ActiveRecord # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index cc12aacaf02..a4771e864d4 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -66,23 +66,21 @@ module Projects end def import_repository - begin - refmap = importer_class.try(:refmap) if has_importer? - - if refmap - project.ensure_repository - project.repository.fetch_as_mirror(project.import_url, refmap: refmap) - else - gitlab_shell.import_project_repository(project) - end - rescue Gitlab::Shell::Error => e - # Expire cache to prevent scenarios such as: - # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true - # 2. Retried import, repo is broken or not imported but +exists?+ still returns true - project.repository.expire_content_cache if project.repository_exists? + refmap = importer_class.try(:refmap) if has_importer? - raise Error, e.message + if refmap + project.ensure_repository + project.repository.fetch_as_mirror(project.import_url, refmap: refmap) + else + gitlab_shell.import_project_repository(project) end + rescue Gitlab::Shell::Error => e + # Expire cache to prevent scenarios such as: + # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true + # 2. Retried import, repo is broken or not imported but +exists?+ still returns true + project.repository.expire_content_cache if project.repository_exists? + + raise Error, e.message end def download_lfs_objects diff --git a/app/services/spam_service.rb b/app/services/spam_check_service.rb index 242c0ede57a..e1f5efabcaf 100644 --- a/app/services/spam_service.rb +++ b/app/services/spam_check_service.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class SpamService +class SpamCheckService include AkismetMethods attr_accessor :spammable, :request, :options @@ -21,14 +21,14 @@ class SpamService end end - def when_recaptcha_verified(recaptcha_verified, api = false) - # In case it's a request which is already verified through recaptcha, yield - # block. + def execute(api: false, recaptcha_verified:, spam_log_id:, user_id:) if recaptcha_verified - yield + # If it's a request which is already verified through recaptcha, + # update the spam log accordingly. + SpamLog.verify_recaptcha!(user_id: user_id, id: spam_log_id) else - # Otherwise, it goes to Akismet and check if it's a spam. If that's the - # case, it assigns spammable record as "spam" and create a SpamLog record. + # Otherwise, it goes to Akismet for spam check. + # If so, it assigns spammable object as "spam" and creates a SpamLog record. possible_spam = check(api) spammable.spam = possible_spam unless spammable.allow_possible_spam? spammable.spam_log = spam_log @@ -38,9 +38,9 @@ class SpamService private def check(api) - return false unless request && check_for_spam? - - return false unless akismet.spam? + return unless request + return unless check_for_spam? + return unless akismet.spam? create_spam_log(api) true diff --git a/changelogs/unreleased/add-extra-fields-to-the-application-context.yml b/changelogs/unreleased/add-extra-fields-to-the-application-context.yml new file mode 100644 index 00000000000..780168ef6c3 --- /dev/null +++ b/changelogs/unreleased/add-extra-fields-to-the-application-context.yml @@ -0,0 +1,5 @@ +--- +title: Add extra fields to the application context +merge_request: 22792 +author: +type: added diff --git a/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md b/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md index baadc2c6c56..c4b7324ce05 100644 --- a/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md +++ b/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md @@ -542,6 +542,20 @@ group = Group.find_by_full_path 'group' user.max_member_access_for_group group.id ``` +### Change user password + +```ruby +password = "your password" +user = User.find_by_username('your username') +password_attributes = { + password: password, + password_confirmation: password, + password_automatically_set: false +} + +result = Users::UpdateService.new(user, password_attributes.merge(user: user)).execute +``` + ## Groups ### Count unique users in a group and sub-groups diff --git a/lib/api/api.rb b/lib/api/api.rb index 1aee4fd30ee..83439e914cd 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -47,7 +47,8 @@ module API Gitlab::ApplicationContext.push( user: -> { current_user }, project: -> { @project }, - namespace: -> { @group } + namespace: -> { @group }, + caller_id: route.origin ) end diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index d64de2bb465..9de3644daa8 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -9,7 +9,8 @@ module API before do Gitlab::ApplicationContext.push( user: -> { actor&.user }, - project: -> { project } + project: -> { project }, + caller_id: route.origin ) end diff --git a/lib/declarative_policy/runner.rb b/lib/declarative_policy/runner.rb index f739fe5e16e..70666dc7924 100644 --- a/lib/declarative_policy/runner.rb +++ b/lib/declarative_policy/runner.rb @@ -33,6 +33,7 @@ module DeclarativePolicy attr_reader :steps def initialize(steps) @steps = steps + @state = nil end # We make sure only to run any given Runner once, diff --git a/lib/feature/gitaly.rb b/lib/feature/gitaly.rb index 2bd55c36a03..c225ca56c55 100644 --- a/lib/feature/gitaly.rb +++ b/lib/feature/gitaly.rb @@ -11,6 +11,7 @@ class Feature inforef_uploadpack_cache get_tag_messages_go filter_shas_with_signatures_go + commit_without_batch_check ].freeze DEFAULT_ON_FLAGS = Set.new([]).freeze diff --git a/lib/gitlab/application_context.rb b/lib/gitlab/application_context.rb index 71dbfea70e8..5a9a99423c4 100644 --- a/lib/gitlab/application_context.rb +++ b/lib/gitlab/application_context.rb @@ -5,12 +5,13 @@ module Gitlab class ApplicationContext include Gitlab::Utils::LazyAttributes - Attribute = Struct.new(:name, :type) + Attribute = Struct.new(:name, :type, :evaluation) APPLICATION_ATTRIBUTES = [ Attribute.new(:project, Project), Attribute.new(:namespace, Namespace), - Attribute.new(:user, User) + Attribute.new(:user, User), + Attribute.new(:caller_id, String) ].freeze def self.with_context(args, &block) @@ -37,6 +38,7 @@ module Gitlab hash[:user] = -> { username } if set_values.include?(:user) hash[:project] = -> { project_path } if set_values.include?(:project) hash[:root_namespace] = -> { root_namespace_path } if include_namespace? + hash[:caller_id] = caller_id if set_values.include?(:caller_id) end end @@ -75,3 +77,5 @@ module Gitlab end end end + +Gitlab::ApplicationContext.prepend_if_ee('EE::Gitlab::ApplicationContext') diff --git a/lib/gitlab/health_checks/base_abstract_check.rb b/lib/gitlab/health_checks/base_abstract_check.rb index 199cd2f9b2d..7e1c5331b07 100644 --- a/lib/gitlab/health_checks/base_abstract_check.rb +++ b/lib/gitlab/health_checks/base_abstract_check.rb @@ -32,11 +32,9 @@ module Gitlab end def catch_timeout(seconds, &block) - begin - Timeout.timeout(seconds.to_i, &block) - rescue Timeout::Error => ex - ex - end + Timeout.timeout(seconds.to_i, &block) + rescue Timeout::Error => ex + ex end end end diff --git a/lib/gitlab/marginalia.rb b/lib/gitlab/marginalia.rb index 2be96cecae3..b4b9896e6b9 100644 --- a/lib/gitlab/marginalia.rb +++ b/lib/gitlab/marginalia.rb @@ -2,6 +2,8 @@ module Gitlab module Marginalia + cattr_accessor :enabled, default: false + MARGINALIA_FEATURE_FLAG = :marginalia def self.set_application_name @@ -15,14 +17,14 @@ module Gitlab end def self.cached_feature_enabled? - !!@enabled + enabled end def self.set_feature_cache # During db:create and db:bootstrap skip feature query as DB is not available yet. return false unless ActiveRecord::Base.connected? && Gitlab::Database.cached_table_exists?('features') - @enabled = Feature.enabled?(MARGINALIA_FEATURE_FLAG) + self.enabled = Feature.enabled?(MARGINALIA_FEATURE_FLAG) end end end diff --git a/lib/gitlab/sidekiq_middleware/client_metrics.rb b/lib/gitlab/sidekiq_middleware/client_metrics.rb index cd11415b55e..245a1b5e024 100644 --- a/lib/gitlab/sidekiq_middleware/client_metrics.rb +++ b/lib/gitlab/sidekiq_middleware/client_metrics.rb @@ -9,8 +9,10 @@ module Gitlab @metrics = init_metrics end - def call(worker, _job, queue, _redis_pool) - labels = create_labels(worker.class, queue) + def call(worker_class, _job, queue, _redis_pool) + # worker_class can either be the string or class of the worker being enqueued. + worker_class = worker_class.safe_constantize if worker_class.respond_to?(:safe_constantize) + labels = create_labels(worker_class, queue) @metrics.fetch(ENQUEUED).increment(labels, 1) diff --git a/lib/gitlab/sidekiq_middleware/metrics.rb b/lib/gitlab/sidekiq_middleware/metrics.rb index 9588e9ef19a..fbc34357323 100644 --- a/lib/gitlab/sidekiq_middleware/metrics.rb +++ b/lib/gitlab/sidekiq_middleware/metrics.rb @@ -10,7 +10,7 @@ module Gitlab def create_labels(worker_class, queue) labels = { queue: queue.to_s, latency_sensitive: FALSE_LABEL, external_dependencies: FALSE_LABEL, feature_category: "", boundary: "" } - return labels unless worker_class.include? WorkerAttributes + return labels unless worker_class && worker_class.include?(WorkerAttributes) labels[:latency_sensitive] = bool_as_label(worker_class.latency_sensitive_worker?) labels[:external_dependencies] = bool_as_label(worker_class.worker_has_external_dependencies?) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5398c2c32fa..cd984e981a1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3341,6 +3341,9 @@ msgstr "" msgid "Checkout|$%{selectedPlanPrice} per user per year" msgstr "" +msgid "Checkout|%{cardType} ending in %{lastFourDigits}" +msgstr "" + msgid "Checkout|%{name}'s GitLab subscription" msgstr "" @@ -3362,15 +3365,57 @@ msgstr "" msgid "Checkout|3. Your GitLab group" msgstr "" +msgid "Checkout|Billing address" +msgstr "" + msgid "Checkout|Checkout" msgstr "" +msgid "Checkout|City" +msgstr "" + +msgid "Checkout|Confirm purchase" +msgstr "" + +msgid "Checkout|Confirming..." +msgstr "" + msgid "Checkout|Continue to billing" msgstr "" +msgid "Checkout|Continue to payment" +msgstr "" + +msgid "Checkout|Country" +msgstr "" + +msgid "Checkout|Credit card form failed to load. Please try again." +msgstr "" + +msgid "Checkout|Credit card form failed to load: %{message}" +msgstr "" + msgid "Checkout|Edit" msgstr "" +msgid "Checkout|Exp %{expirationMonth}/%{expirationYear}" +msgstr "" + +msgid "Checkout|Failed to confirm your order! Please try again." +msgstr "" + +msgid "Checkout|Failed to confirm your order: %{message}. Please try again." +msgstr "" + +msgid "Checkout|Failed to load countries. Please try again." +msgstr "" + +msgid "Checkout|Failed to load states. Please try again." +msgstr "" + +msgid "Checkout|Failed to register credit card. Please try again." +msgstr "" + msgid "Checkout|GitLab plan" msgstr "" @@ -3386,6 +3431,24 @@ msgstr "" msgid "Checkout|Number of users" msgstr "" +msgid "Checkout|Payment method" +msgstr "" + +msgid "Checkout|Please select a country" +msgstr "" + +msgid "Checkout|Please select a state" +msgstr "" + +msgid "Checkout|State" +msgstr "" + +msgid "Checkout|Street address" +msgstr "" + +msgid "Checkout|Submitting the credit card form failed with code %{errorCode}: %{errorMessage}" +msgstr "" + msgid "Checkout|Subscription details" msgstr "" @@ -3404,6 +3467,9 @@ msgstr "" msgid "Checkout|Your organization" msgstr "" +msgid "Checkout|Zip code" +msgstr "" + msgid "Checkout|company or team" msgstr "" diff --git a/package.json b/package.json index c303d375d36..85f054ea3dc 100644 --- a/package.json +++ b/package.json @@ -39,8 +39,8 @@ "@babel/plugin-syntax-dynamic-import": "^7.2.0", "@babel/plugin-syntax-import-meta": "^7.2.0", "@babel/preset-env": "^7.6.2", - "@gitlab/svgs": "^1.89.0", - "@gitlab/ui": "^8.18.0", + "@gitlab/svgs": "^1.90.0", + "@gitlab/ui": "^8.20.0", "@gitlab/visual-review-tools": "1.5.1", "@sentry/browser": "^5.10.2", "@sourcegraph/code-host-integration": "0.0.21", diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 0c299dcda34..ae7b5784302 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -924,7 +924,7 @@ describe ApplicationController do end it 'sets the group if it was available' do - group = build_stubbed(:group) + group = build(:group) controller.instance_variable_set(:@group, group) get :index, format: :json @@ -933,12 +933,18 @@ describe ApplicationController do end it 'sets the project if one was available' do - project = build_stubbed(:project) + project = build(:project) controller.instance_variable_set(:@project, project) get :index, format: :json expect(json_response['meta.project']).to eq(project.full_path) end + + it 'sets the caller_id as controller#action' do + get :index, format: :json + + expect(json_response['meta.caller_id']).to eq('AnonymousController#index') + end end end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index b31c5e30fc0..0168c2118c7 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -228,9 +228,7 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc click_link 'Slack notifications' - page.all('input[type=checkbox]').each do |checkbox| - expect(checkbox).to be_checked - end + expect(page.all('input[type=checkbox]')).to all(be_checked) expect(find_field('Webhook').value).to eq 'http://localhost' expect(find_field('Username').value).to eq 'test_user' expect(find('#service_push_channel').value).to eq '#test_channel' diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index c99c205d5da..ee5773f1484 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -192,7 +192,7 @@ describe 'Filter issues', :js do end it 'filters issues by label containing special characters' do - special_label = create(:label, project: project, title: '!@#{$%^&*()-+[]<>?/:{}|\}') + special_label = create(:label, project: project, title: '!@#$%^&*()-+[]<>?/:{}|\\') special_issue = create(:issue, title: "Issue with special character label", project: project) special_issue.labels << special_label @@ -204,7 +204,7 @@ describe 'Filter issues', :js do end it 'filters issues by label not containing special characters' do - special_label = create(:label, project: project, title: '!@#{$%^&*()-+[]<>?/:{}|\}') + special_label = create(:label, project: project, title: '!@#$%^&*()-+[]<>?/:{}|\\') special_issue = create(:issue, title: "Issue with special character label", project: project) special_issue.labels << special_label diff --git a/spec/features/markdown/copy_as_gfm_spec.rb b/spec/features/markdown/copy_as_gfm_spec.rb index 84221f5555a..ef3a36f3128 100644 --- a/spec/features/markdown/copy_as_gfm_spec.rb +++ b/spec/features/markdown/copy_as_gfm_spec.rb @@ -624,7 +624,7 @@ describe 'Copy as GFM', :js do GFM # table with empty heading - <<~GFM, + <<~GFM | | x | y | |--|---|---| | a | 1 | 0 | @@ -784,7 +784,7 @@ describe 'Copy as GFM', :js do verify( '.line[id="LC9"], .line[id="LC10"]', - <<~GFM, + <<~GFM ```ruby raise RuntimeError, "System commands must be given as an array of strings" end @@ -826,7 +826,7 @@ describe 'Copy as GFM', :js do verify( '.line[id="LC27"], .line[id="LC28"]', - <<~GFM, + <<~GFM ```json "bio": null, "skype": "", diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 5f4f92e547c..5a8db3c070d 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -152,7 +152,7 @@ describe 'Login' do end end - describe 'with two-factor authentication' do + describe 'with two-factor authentication', :js do def enter_code(code) fill_in 'user_otp_attempt', with: code click_button 'Verify code' diff --git a/spec/lib/gitlab/application_context_spec.rb b/spec/lib/gitlab/application_context_spec.rb index 482bf0dc192..1fd400294b7 100644 --- a/spec/lib/gitlab/application_context_spec.rb +++ b/spec/lib/gitlab/application_context_spec.rb @@ -43,11 +43,11 @@ describe Gitlab::ApplicationContext do describe '#to_lazy_hash' do let(:user) { build(:user) } let(:project) { build(:project) } - let(:namespace) { build(:group) } - let(:subgroup) { build(:group, parent: namespace) } + let(:namespace) { create(:group) } + let(:subgroup) { create(:group, parent: namespace) } def result(context) - context.to_lazy_hash.transform_values { |v| v.call } + context.to_lazy_hash.transform_values { |v| v.respond_to?(:call) ? v.call : v } end it 'does not call the attributes until needed' do @@ -78,27 +78,5 @@ describe Gitlab::ApplicationContext do expect(result(context)) .to include(project: project.full_path, root_namespace: project.full_path_components.first) end - - context 'only include values for which an option was specified' do - using RSpec::Parameterized::TableSyntax - - where(:provided_options, :expected_context_keys) do - [:user, :namespace, :project] | [:user, :project, :root_namespace] - [:user, :project] | [:user, :project, :root_namespace] - [:user, :namespace] | [:user, :root_namespace] - [:user] | [:user] - [] | [] - end - - with_them do - it do - # Build a hash that has all `provided_options` as keys, and `nil` as value - provided_values = provided_options.map { |key| [key, nil] }.to_h - context = described_class.new(provided_values) - - expect(context.to_lazy_hash.keys).to contain_exactly(*expected_context_keys) - end - end - end end end diff --git a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb index d16f34af325..b0d2e049777 100644 --- a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb +++ b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb @@ -35,7 +35,7 @@ describe Gitlab::Gfm::ReferenceRewriter do context 'description with ignored elements' do let(:text) do - "Hi. This references #1, but not `#2`\n" + + "Hi. This references #1, but not `#2`\n" \ '<pre>and not !1</pre>' end diff --git a/spec/lib/gitlab/gitlab_import/client_spec.rb b/spec/lib/gitlab/gitlab_import/client_spec.rb index 246ef6c02f2..6e4e88093bb 100644 --- a/spec/lib/gitlab/gitlab_import/client_spec.rb +++ b/spec/lib/gitlab/gitlab_import/client_spec.rb @@ -13,9 +13,7 @@ describe Gitlab::GitlabImport::Client do end it 'all OAuth2 client options are symbols' do - client.client.options.keys.each do |key| - expect(key).to be_kind_of(Symbol) - end + expect(client.client.options.keys).to all(be_kind_of(Symbol)) end it 'uses membership and simple flags' do diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/incoming_email_spec.rb index 598336d0b31..f5a6ea4d5b0 100644 --- a/spec/lib/gitlab/incoming_email_spec.rb +++ b/spec/lib/gitlab/incoming_email_spec.rb @@ -99,8 +99,8 @@ describe Gitlab::IncomingEmail do context 'self.scan_fallback_references' do let(:references) do - '<issue_1@localhost>' + - ' <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>' + + '<issue_1@localhost>' \ + ' <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>' \ ',<exchange@microsoft.com>' end diff --git a/spec/lib/gitlab/legacy_github_import/client_spec.rb b/spec/lib/gitlab/legacy_github_import/client_spec.rb index 194518a1f36..8d1786ae49a 100644 --- a/spec/lib/gitlab/legacy_github_import/client_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/client_spec.rb @@ -13,9 +13,7 @@ describe Gitlab::LegacyGithubImport::Client do end it 'convert OAuth2 client options to symbols' do - client.client.options.keys.each do |key| - expect(key).to be_kind_of(Symbol) - end + expect(client.client.options.keys).to all(be_kind_of(Symbol)) end it 'does not crash (e.g. Settingslogic::MissingSetting) when verify_ssl config is not present' do diff --git a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb index 6516016e67f..daee2c0bbd0 100644 --- a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb @@ -21,13 +21,19 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do describe '#call' do it 'yields block' do - expect { |b| subject.call(worker, job, :test, double, &b) }.to yield_control.once + expect { |b| subject.call(worker_class, job, :test, double, &b) }.to yield_control.once end - it 'increments enqueued jobs metric' do + it 'increments enqueued jobs metric with correct labels when worker is a string of the class' do expect(enqueued_jobs_metric).to receive(:increment).with(labels, 1) - subject.call(worker, job, :test, double) { nil } + subject.call(worker_class.to_s, job, :test, double) { nil } + end + + it 'increments enqueued jobs metric with correct labels' do + expect(enqueued_jobs_metric).to receive(:increment).with(labels, 1) + + subject.call(worker_class, job, :test, double) { nil } end end end @@ -46,7 +52,7 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do context "when workers are attributed" do def create_attributed_worker_class(latency_sensitive, external_dependencies, resource_boundary, category) - Class.new do + klass = Class.new do include Sidekiq::Worker include WorkerAttributes @@ -55,6 +61,7 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do worker_resource_boundary resource_boundary unless resource_boundary == :unknown feature_category category unless category.nil? end + stub_const("TestAttributedWorker", klass) end let(:latency_sensitive) { false } diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index cae88f39660..aa3a60b867a 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -15,11 +15,6 @@ describe Email do end describe '#update_invalid_gpg_signatures' do - let(:user) do - create(:user, email: 'tula.torphy@abshire.ca').tap do |user| - user.skip_reconfirmation! - end - end let(:user) { create(:user) } it 'synchronizes the gpg keys when the email is updated' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 2fd1a87e5f1..1e8598a457c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2150,20 +2150,10 @@ describe MergeRequest do subject.mark_as_mergeable! end - context 'and merge_requests_conditional_mergeability_check feature flag is enabled' do - it 'does not call MergeabilityCheckService' do - expect(MergeRequests::MergeabilityCheckService).not_to receive(:new) + it 'does not call MergeabilityCheckService' do + expect(MergeRequests::MergeabilityCheckService).not_to receive(:new) - subject.check_mergeability - end - end - - context 'and merge_requests_conditional_mergeability_check feature flag is disabled' do - before do - stub_feature_flags(merge_requests_conditional_mergeability_check: false) - end - - it_behaves_like 'method that executes MergeabilityCheckService' + subject.check_mergeability end end end diff --git a/spec/models/spam_log_spec.rb b/spec/models/spam_log_spec.rb index f4e073dc38f..8ebd97de9ff 100644 --- a/spec/models/spam_log_spec.rb +++ b/spec/models/spam_log_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe SpamLog do - let(:admin) { create(:admin) } + let_it_be(:admin) { create(:admin) } describe 'associations' do it { is_expected.to belong_to(:user) } @@ -31,4 +31,29 @@ describe SpamLog do expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) end end + + describe '.verify_recaptcha!' do + let_it_be(:spam_log) { create(:spam_log, user: admin, recaptcha_verified: false) } + + context 'the record cannot be found' do + it 'updates nothing' do + expect(instance_of(described_class)).not_to receive(:update!) + + described_class.verify_recaptcha!(id: spam_log.id, user_id: admin.id) + + expect(spam_log.recaptcha_verified).to be_falsey + end + + it 'does not error despite not finding a record' do + expect { described_class.verify_recaptcha!(id: -1, user_id: admin.id) }.not_to raise_error + end + end + + context 'the record exists' do + it 'updates recaptcha_verified' do + expect { described_class.verify_recaptcha!(id: spam_log.id, user_id: admin.id) } + .to change { spam_log.reload.recaptcha_verified }.from(false).to(true) + end + end + end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index dd93d5a6f5f..50754773fad 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -326,7 +326,7 @@ describe API::Internal::Base do expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path) expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage)) expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) - expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true') + expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true', 'gitaly-feature-commit-without-batch-check' => 'true') expect(user.reload.last_activity_on).to eql(Date.today) end end @@ -346,7 +346,7 @@ describe API::Internal::Base do expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path) expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage)) expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) - expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true') + expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true', 'gitaly-feature-commit-without-batch-check' => 'true') expect(user.reload.last_activity_on).to be_nil end end @@ -594,7 +594,7 @@ describe API::Internal::Base do expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path) expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage)) expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) - expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true') + expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true', 'gitaly-feature-commit-without-batch-check' => 'true') end end diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb index 67404cf10df..bbe0683c275 100644 --- a/spec/requests/api/issues/post_projects_issues_spec.rb +++ b/spec/requests/api/issues/post_projects_issues_spec.rb @@ -389,7 +389,7 @@ describe API::Issues do end before do - expect_next_instance_of(SpamService) do |spam_service| + expect_next_instance_of(SpamCheckService) do |spam_service| expect(spam_service).to receive_messages(check_for_spam?: true) end expect_next_instance_of(AkismetService) do |akismet_service| diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index 43f302ed194..39ac53899da 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -194,7 +194,7 @@ describe API::Issues do end before do - expect_next_instance_of(SpamService) do |spam_service| + expect_next_instance_of(SpamCheckService) do |spam_service| expect(spam_service).to receive_messages(check_for_spam?: true) end expect_next_instance_of(AkismetService) do |akismet_service| diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 5dc6b6176ee..bb68a8dcbbb 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -385,7 +385,7 @@ describe Issues::CreateService do context 'when recaptcha was not verified' do before do - expect_next_instance_of(SpamService) do |spam_service| + expect_next_instance_of(SpamCheckService) do |spam_service| expect(spam_service).to receive_messages(check_for_spam?: true) end end @@ -408,7 +408,7 @@ describe Issues::CreateService do it 'creates a new spam_log' do expect { issue } - .to log_spam(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue') + .to have_spam_log(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue') end it 'assigns a spam_log to an issue' do @@ -431,7 +431,7 @@ describe Issues::CreateService do it 'creates a new spam_log' do expect { issue } - .to log_spam(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue') + .to have_spam_log(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue') end it 'assigns a spam_log to an issue' do diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb index 203048984a1..4d87fa3e832 100644 --- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb +++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' describe MergeRequests::AddTodoWhenBuildFailsService do let(:user) { create(:user) } - let(:merge_request) { create(:merge_request) } let(:project) { create(:project, :repository) } let(:sha) { '1234567890abcdef1234567890abcdef12345678' } let(:ref) { merge_request.source_branch } diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb index 7cb0bd41f13..7eea2a7afc6 100644 --- a/spec/services/notes/quick_actions_service_spec.rb +++ b/spec/services/notes/quick_actions_service_spec.rb @@ -176,7 +176,6 @@ describe Notes::QuickActionsService do context 'CE restriction for issue assignees' do describe '/assign' do let(:project) { create(:project) } - let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } } let(:assignee) { create(:user) } let(:maintainer) { create(:user) } let(:service) { described_class.new(project, maintainer) } diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index b2576cae575..c56c23ffc70 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -1322,11 +1322,6 @@ describe QuickActions::InterpretService do let(:issuable) { issue } end - it_behaves_like 'empty command', _('Failed to mark this issue as a duplicate because referenced issue was not found.') do - let(:content) { '/duplicate #{issue.to_reference}' } - let(:issuable) { issue } - end - it_behaves_like 'empty command' do let(:content) { '/lock' } let(:issuable) { issue } diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb index 6f7ce7959ff..3ada50e80aa 100644 --- a/spec/services/snippets/create_service_spec.rb +++ b/spec/services/snippets/create_service_spec.rb @@ -86,7 +86,7 @@ describe Snippets::CreateService do it 'creates a new spam_log' do expect { snippet } - .to log_spam(title: snippet.title, noteable_type: snippet.class.name) + .to have_spam_log(title: snippet.title, noteable_type: snippet.class.name) end it 'assigns a spam_log to an issue' do diff --git a/spec/services/spam_check_service_spec.rb b/spec/services/spam_check_service_spec.rb new file mode 100644 index 00000000000..a58c8d9cfd8 --- /dev/null +++ b/spec/services/spam_check_service_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SpamCheckService do + let(:fake_ip) { '1.2.3.4' } + let(:fake_user_agent) { 'fake-user-agent' } + let(:fake_referrer) { 'fake-http-referrer' } + let(:env) do + { 'action_dispatch.remote_ip' => fake_ip, + 'HTTP_USER_AGENT' => fake_user_agent, + 'HTTP_REFERRER' => fake_referrer } + end + let(:request) { double(:request, env: env) } + + let_it_be(:project) { create(:project, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:issue) { create(:issue, project: project, author: user) } + + before do + issue.spam = false + end + + describe '#initialize' do + subject { described_class.new(spammable: issue, request: request) } + + context 'when the request is nil' do + let(:request) { nil } + + it 'assembles the options with information from the spammable' do + aggregate_failures do + expect(subject.options[:ip_address]).to eq(issue.ip_address) + expect(subject.options[:user_agent]).to eq(issue.user_agent) + expect(subject.options.key?(:referrer)).to be_falsey + end + end + end + + context 'when the request is present' do + let(:request) { double(:request, env: env) } + + it 'assembles the options with information from the spammable' do + aggregate_failures do + expect(subject.options[:ip_address]).to eq(fake_ip) + expect(subject.options[:user_agent]).to eq(fake_user_agent) + expect(subject.options[:referrer]).to eq(fake_referrer) + end + end + end + end + + describe '#execute' do + let(:request) { double(:request, env: env) } + + let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) } + + subject do + described_service = described_class.new(spammable: issue, request: request) + described_service.execute(user_id: user.id, api: nil, recaptcha_verified: recaptcha_verified, spam_log_id: existing_spam_log.id) + end + + context 'when recaptcha was already verified' do + let(:recaptcha_verified) { true } + + it "updates spam log and doesn't check Akismet" do + aggregate_failures do + expect(SpamLog).not_to receive(:create!) + expect(an_instance_of(described_class)).not_to receive(:check) + end + + subject + end + + it 'updates spam log' do + subject + + expect(existing_spam_log.reload.recaptcha_verified).to be_truthy + end + end + + context 'when recaptcha was not verified' do + let(:recaptcha_verified) { false } + + context 'when spammable attributes have not changed' do + before do + issue.closed_at = Time.zone.now + + allow(AkismetService).to receive(:new).and_return(double(spam?: true)) + end + + it 'returns false' do + expect(subject).to be_falsey + end + + it 'does not create a spam log' do + expect { subject } + .not_to change { SpamLog.count } + end + end + + context 'when spammable attributes have changed' do + before do + issue.description = 'SPAM!' + end + + context 'when indicated as spam by akismet' do + before do + allow(AkismetService).to receive(:new).and_return(double(spam?: true)) + end + + context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end + + it_behaves_like 'akismet spam' + + it 'checks as spam' do + subject + + expect(issue.reload.spam).to be_truthy + end + end + + context 'when allow_possible_spam feature flag is true' do + it_behaves_like 'akismet spam' + + it 'does not check as spam' do + subject + + expect(issue.spam).to be_falsey + end + end + end + + context 'when not indicated as spam by akismet' do + before do + allow(AkismetService).to receive(:new).and_return(double(spam?: false)) + end + + it 'returns false' do + expect(subject).to be_falsey + end + + it 'does not create a spam log' do + expect { subject } + .not_to change { SpamLog.count } + end + end + end + end + end +end diff --git a/spec/services/spam_service_spec.rb b/spec/services/spam_service_spec.rb deleted file mode 100644 index c8ebe87e4d2..00000000000 --- a/spec/services/spam_service_spec.rb +++ /dev/null @@ -1,111 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe SpamService do - describe '#when_recaptcha_verified' do - def check_spam(issue, request, recaptcha_verified) - described_class.new(spammable: issue, request: request).when_recaptcha_verified(recaptcha_verified) do - 'yielded' - end - end - - it 'yields block when recaptcha was already verified' do - issue = build_stubbed(:issue) - - expect(check_spam(issue, nil, true)).to eql('yielded') - end - - context 'when recaptcha was not verified' do - let(:project) { create(:project, :public) } - let(:issue) { create(:issue, project: project) } - let(:request) { double(:request, env: {}) } - - context 'when spammable attributes have not changed' do - before do - issue.closed_at = Time.zone.now - - allow(AkismetService).to receive(:new).and_return(double(spam?: true)) - end - - it 'returns false' do - expect(check_spam(issue, request, false)).to be_falsey - end - - it 'does not create a spam log' do - expect { check_spam(issue, request, false) } - .not_to change { SpamLog.count } - end - end - - context 'when spammable attributes have changed' do - before do - issue.description = 'SPAM!' - end - - context 'when indicated as spam by akismet' do - shared_examples 'akismet spam' do - it "doesn't check as spam when request is missing" do - check_spam(issue, nil, false) - - expect(issue).not_to be_spam - end - - it 'creates a spam log' do - expect { check_spam(issue, request, false) } - .to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue') - end - - it 'does not yield to the block' do - expect(check_spam(issue, request, false)) - .to eql(SpamLog.last) - end - end - - before do - allow(AkismetService).to receive(:new).and_return(double(spam?: true)) - end - - context 'when allow_possible_spam feature flag is false' do - before do - stub_feature_flags(allow_possible_spam: false) - end - - it_behaves_like 'akismet spam' - - it 'checks as spam' do - check_spam(issue, request, false) - - expect(issue.spam).to be_truthy - end - end - - context 'when allow_possible_spam feature flag is true' do - it_behaves_like 'akismet spam' - - it 'does not check as spam' do - check_spam(issue, request, false) - - expect(issue.spam).to be_nil - end - end - end - - context 'when not indicated as spam by akismet' do - before do - allow(AkismetService).to receive(:new).and_return(double(spam?: false)) - end - - it 'returns false' do - expect(check_spam(issue, request, false)).to be_falsey - end - - it 'does not create a spam log' do - expect { check_spam(issue, request, false) } - .not_to change { SpamLog.count } - end - end - end - end - end -end diff --git a/spec/support/helpers/user_login_helper.rb b/spec/support/helpers/user_login_helper.rb index 36c002f53af..66606832883 100644 --- a/spec/support/helpers/user_login_helper.rb +++ b/spec/support/helpers/user_login_helper.rb @@ -13,7 +13,7 @@ module UserLoginHelper def ensure_tab_pane_counts tabs_count = page.all('[role="tab"]').size - expect(page).to have_selector('[role="tabpanel"]', count: tabs_count) + expect(page).to have_selector('[role="tabpanel"]', visible: :all, count: tabs_count) end def ensure_one_active_tab diff --git a/spec/support/matchers/log_spam.rb b/spec/support/matchers/log_spam.rb index f6aa7dbd152..260d2930816 100644 --- a/spec/support/matchers/log_spam.rb +++ b/spec/support/matchers/log_spam.rb @@ -1,29 +1,31 @@ # frozen_string_literal: true # This matcher checks if one spam log with provided attributes was created +# during the block evocation. # # Example: # -# expect { create_issue }.to log_spam -RSpec::Matchers.define :log_spam do |expected| - def spam_logs - SpamLog.all - end +# expect { create_issue }.to log_spam(key1: value1, key2: value2) +RSpec::Matchers.define :log_spam do |expected| match do |block| + @existing_logs_count = SpamLog.count + block.call - expect(spam_logs).to contain_exactly( - have_attributes(expected) - ) + @new_logs_count = SpamLog.count + @last_spam_log = SpamLog.last + + expect(@new_logs_count - @existing_logs_count).to eq 1 + expect(@last_spam_log).to have_attributes(expected) end description do - count = spam_logs.count + count = @new_logs_count - @existing_logs_count if count == 1 keys = expected.keys.map(&:to_s) - actual = spam_logs.first.attributes.slice(*keys) + actual = @last_spam_log.attributes.slice(*keys) "create a spam log with #{expected} attributes. #{actual} created instead." else "create exactly 1 spam log with #{expected} attributes. #{count} spam logs created instead." @@ -32,3 +34,34 @@ RSpec::Matchers.define :log_spam do |expected| supports_block_expectations end + +# This matcher checks that the last spam log +# has the attributes provided. +# The spam log does not have to be created during the block evocation. +# The number of total spam logs just has to be more than one. +# +# Example: +# +# expect { create_issue }.to have_spam_log(key1: value1, key2: value2) + +RSpec::Matchers.define :have_spam_log do |expected| + match do |block| + block.call + + @total_logs_count = SpamLog.count + @latest_spam_log = SpamLog.last + expect(SpamLog.last).to have_attributes(expected) + end + + description do + if @total_logs_count > 0 + keys = expected.keys.map(&:to_s) + actual = @latest_spam_log.attributes.slice(*keys) + "the last spam log to have #{expected} attributes. Last spam log has #{actual} attributes instead." + else + "there to be a spam log, but there are no spam logs." + end + end + + supports_block_expectations +end diff --git a/spec/support/shared_examples/spam_check_shared_examples.rb b/spec/support/shared_examples/spam_check_shared_examples.rb new file mode 100644 index 00000000000..3ecae16db39 --- /dev/null +++ b/spec/support/shared_examples/spam_check_shared_examples.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +shared_examples 'akismet spam' do + context 'when request is missing' do + subject { described_class.new(spammable: issue, request: nil) } + + it "doesn't check as spam" do + subject + + expect(issue).not_to be_spam + end + end + + context 'when request exists' do + it 'creates a spam log' do + expect { subject } + .to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue') + end + end +end diff --git a/yarn.lock b/yarn.lock index f3872f1c41a..8aab584c343 100644 --- a/yarn.lock +++ b/yarn.lock @@ -732,15 +732,15 @@ dependencies: vue-eslint-parser "^6.0.4" -"@gitlab/svgs@^1.89.0": - version "1.89.0" - resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.89.0.tgz#5bdaff1b0af1cc07ed34e89c21c34c7c6a3e1caa" - integrity sha512-vI6VobZs6mq2Bbiej5bYMHyvtn8kD1O/uHSlyY9jgJoa2TXU+jFI9DqUpJmx8EIHt+o0qm/8G3XsFGEr5gLb7Q== - -"@gitlab/ui@^8.18.0": - version "8.18.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-8.18.0.tgz#11bd7d5fb2db10034fdf2544847dc9afd24cc02c" - integrity sha512-ihcXJDVUNvp8kv+ha+0d1rrRIG8IEWfDNICremTpl62V5kN9Eiwo0Csb8Gj20sBp9ERYCycjwpjvfU7dUwyAiw== +"@gitlab/svgs@^1.90.0": + version "1.90.0" + resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.90.0.tgz#e6fe0ca3d353fcdbd792c10d82444383c33f539d" + integrity sha512-6UikaIMGosmrDAd6Lf3QIJWDM4FwhoGIN+CJuFcWeHDMbZT69LnTabuGfvOQmp2+nlI68baRTSDufaux7m9Ajw== + +"@gitlab/ui@^8.20.0": + version "8.20.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-8.20.0.tgz#98c0db6ddaa6b3bf8e5dcd1ae869df1819bc4186" + integrity sha512-Gjq7030E1Swo4HSXUrc9pKxFsmlS3pX/uq/67hGghAtFI8svJWWmCBLnSDHUVgjEW+DdmOn3hqqTWRPNaXGtOw== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.3.0" |