diff options
author | Heinrich Lee Yu <heinrich@gitlab.com> | 2019-06-26 01:54:42 +0800 |
---|---|---|
committer | Heinrich Lee Yu <heinrich@gitlab.com> | 2019-07-12 10:15:31 +0800 |
commit | aeb67dd489b1ccc7f0ab1d702725729ab9cc3e27 (patch) | |
tree | c508ba9459274be6a8a0488a838d31f03f45faba | |
parent | ecffca5d92353d55aaf8f984737fa617782310e0 (diff) | |
download | gitlab-ce-aeb67dd489b1ccc7f0ab1d702725729ab9cc3e27.tar.gz |
Upgrade to Rails 5.2upgrade-rails-5-2-ce
Updates changed method names and fixes spec failures
53 files changed, 225 insertions, 238 deletions
@@ -1,6 +1,6 @@ source 'https://rubygems.org' -gem 'rails', '5.1.7' +gem 'rails', '5.2.3' # Improves copy-on-write performance for MRI gem 'nakayoshi_fork', '~> 0.0.4' diff --git a/Gemfile.lock b/Gemfile.lock index 1a3b3b8e125..bf469de3835 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,44 +6,48 @@ GEM ace-rails-ap (4.1.2) acme-client (2.0.2) faraday (~> 0.9, >= 0.9.1) - actioncable (5.1.7) - actionpack (= 5.1.7) + actioncable (5.2.3) + actionpack (= 5.2.3) nio4r (~> 2.0) - websocket-driver (~> 0.6.1) - actionmailer (5.1.7) - actionpack (= 5.1.7) - actionview (= 5.1.7) - activejob (= 5.1.7) + websocket-driver (>= 0.6.1) + actionmailer (5.2.3) + actionpack (= 5.2.3) + actionview (= 5.2.3) + activejob (= 5.2.3) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.1.7) - actionview (= 5.1.7) - activesupport (= 5.1.7) + actionpack (5.2.3) + actionview (= 5.2.3) + activesupport (= 5.2.3) rack (~> 2.0) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.1.7) - activesupport (= 5.1.7) + actionview (5.2.3) + activesupport (= 5.2.3) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.3) - activejob (5.1.7) - activesupport (= 5.1.7) + activejob (5.2.3) + activesupport (= 5.2.3) globalid (>= 0.3.6) - activemodel (5.1.7) - activesupport (= 5.1.7) - activerecord (5.1.7) - activemodel (= 5.1.7) - activesupport (= 5.1.7) - arel (~> 8.0) + activemodel (5.2.3) + activesupport (= 5.2.3) + activerecord (5.2.3) + activemodel (= 5.2.3) + activesupport (= 5.2.3) + arel (>= 9.0) activerecord-explain-analyze (0.1.0) activerecord (>= 4) pg activerecord_sane_schema_dumper (1.0) rails (>= 5, < 6) - activesupport (5.1.7) + activestorage (5.2.3) + actionpack (= 5.2.3) + activerecord (= 5.2.3) + marcel (~> 0.3.1) + activesupport (5.2.3) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -60,7 +64,7 @@ GEM apollo_upload_server (2.0.0.beta.3) graphql (>= 1.8) rails (>= 4.2) - arel (8.0.0) + arel (9.0.0) asana (0.8.1) faraday (~> 0.9) faraday_middleware (~> 0.9) @@ -504,6 +508,8 @@ GEM mail (2.7.1) mini_mime (>= 0.1.1) mail_room (0.9.1) + marcel (0.3.3) + mimemagic (~> 0.3.2) mdl (0.5.0) kramdown (~> 1.12, >= 1.12.0) mixlib-cli (~> 1.7, >= 1.7.0) @@ -706,17 +712,18 @@ GEM rack-test (1.1.0) rack (>= 1.0, < 3) rack-timeout (0.5.1) - rails (5.1.7) - actioncable (= 5.1.7) - actionmailer (= 5.1.7) - actionpack (= 5.1.7) - actionview (= 5.1.7) - activejob (= 5.1.7) - activemodel (= 5.1.7) - activerecord (= 5.1.7) - activesupport (= 5.1.7) + rails (5.2.3) + actioncable (= 5.2.3) + actionmailer (= 5.2.3) + actionpack (= 5.2.3) + actionview (= 5.2.3) + activejob (= 5.2.3) + activemodel (= 5.2.3) + activerecord (= 5.2.3) + activestorage (= 5.2.3) + activesupport (= 5.2.3) bundler (>= 1.3.0) - railties (= 5.1.7) + railties (= 5.2.3) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.2) actionpack (~> 5.x, >= 5.0.1) @@ -730,12 +737,12 @@ GEM rails-i18n (5.1.1) i18n (>= 0.7, < 2) railties (>= 5.0, < 6) - railties (5.1.7) - actionpack (= 5.1.7) - activesupport (= 5.1.7) + railties (5.2.3) + actionpack (= 5.2.3) + activesupport (= 5.2.3) method_source rake (>= 0.8.7) - thor (>= 0.18.1, < 2.0) + thor (>= 0.19.0, < 2.0) rainbow (3.0.0) raindrops (0.19.0) rake (12.3.2) @@ -1019,7 +1026,7 @@ GEM hashdiff webpack-rails (0.9.11) railties (>= 3.2.0) - websocket-driver (0.6.5) + websocket-driver (0.7.0) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.3) wikicloth (0.8.1) @@ -1206,7 +1213,7 @@ DEPENDENCIES rack-oauth2 (~> 1.9.3) rack-proxy (~> 0.6.0) rack-timeout - rails (= 5.1.7) + rails (= 5.2.3) rails-controller-testing rails-i18n (~> 5.1) rainbow (~> 3.0) diff --git a/app/controllers/concerns/requires_whitelisted_monitoring_client.rb b/app/controllers/concerns/requires_whitelisted_monitoring_client.rb index f47ead2f0da..2e9905997db 100644 --- a/app/controllers/concerns/requires_whitelisted_monitoring_client.rb +++ b/app/controllers/concerns/requires_whitelisted_monitoring_client.rb @@ -28,7 +28,7 @@ module RequiresWhitelistedMonitoringClient def valid_token? token = params[:token].presence || request.headers['TOKEN'] token.present? && - ActiveSupport::SecurityUtils.variable_size_secure_compare( + ActiveSupport::SecurityUtils.secure_compare( token, Gitlab::CurrentSettings.health_check_access_token ) diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 797833e3f91..dbddee47997 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -107,7 +107,7 @@ class GroupsController < Groups::ApplicationController if Groups::UpdateService.new(@group, current_user, group_params).execute redirect_to edit_group_path(@group, anchor: params[:update_section]), notice: "Group '#{@group.name}' was successfully updated." else - @group.restore_path! + @group.path = @group.path_before_last_save || @group.path_was render action: "edit" end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ae7a1108841..635fcc86166 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -578,7 +578,7 @@ module Ci end def valid_token?(token) - self.token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token) + self.token && ActiveSupport::SecurityUtils.secure_compare(token, self.token) end def has_tags? diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 20ca4a9ab24..2262282e647 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -196,7 +196,7 @@ module Ci sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC' query = ApplicationRecord.send(:sanitize_sql_array, [sql, sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend - order(query) + order(Arel.sql(query)) end scope :for_user, -> (user) { where(user: user) } diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index 78bcce2f592..27a5c3d5286 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -33,22 +33,24 @@ module HasStatus canceled = scope_relevant.canceled.select('count(*)').to_sql warnings = scope_warnings.select('count(*) > 0').to_sql.presence || 'false' - "(CASE - WHEN (#{builds})=(#{skipped}) AND (#{warnings}) THEN 'success' - WHEN (#{builds})=(#{skipped}) THEN 'skipped' - WHEN (#{builds})=(#{success}) THEN 'success' - WHEN (#{builds})=(#{created}) THEN 'created' - WHEN (#{builds})=(#{preparing}) THEN 'preparing' - WHEN (#{builds})=(#{success})+(#{skipped}) THEN 'success' - WHEN (#{builds})=(#{success})+(#{skipped})+(#{canceled}) THEN 'canceled' - WHEN (#{builds})=(#{created})+(#{skipped})+(#{pending}) THEN 'pending' - WHEN (#{running})+(#{pending})>0 THEN 'running' - WHEN (#{manual})>0 THEN 'manual' - WHEN (#{scheduled})>0 THEN 'scheduled' - WHEN (#{preparing})>0 THEN 'preparing' - WHEN (#{created})>0 THEN 'running' - ELSE 'failed' - END)" + Arel.sql( + "(CASE + WHEN (#{builds})=(#{skipped}) AND (#{warnings}) THEN 'success' + WHEN (#{builds})=(#{skipped}) THEN 'skipped' + WHEN (#{builds})=(#{success}) THEN 'success' + WHEN (#{builds})=(#{created}) THEN 'created' + WHEN (#{builds})=(#{preparing}) THEN 'preparing' + WHEN (#{builds})=(#{success})+(#{skipped}) THEN 'success' + WHEN (#{builds})=(#{success})+(#{skipped})+(#{canceled}) THEN 'canceled' + WHEN (#{builds})=(#{created})+(#{skipped})+(#{pending}) THEN 'pending' + WHEN (#{running})+(#{pending})>0 THEN 'running' + WHEN (#{manual})>0 THEN 'manual' + WHEN (#{scheduled})>0 THEN 'scheduled' + WHEN (#{preparing})>0 THEN 'preparing' + WHEN (#{created})>0 THEN 'running' + ELSE 'failed' + END)" + ) end def status @@ -88,22 +90,22 @@ module HasStatus state :scheduled, value: 'scheduled' end - scope :created, -> { where(status: 'created') } - scope :preparing, -> { where(status: 'preparing') } - scope :relevant, -> { where(status: AVAILABLE_STATUSES - ['created']) } - scope :running, -> { where(status: 'running') } - scope :pending, -> { where(status: 'pending') } - scope :success, -> { where(status: 'success') } - scope :failed, -> { where(status: 'failed') } - scope :canceled, -> { where(status: 'canceled') } - scope :skipped, -> { where(status: 'skipped') } - scope :manual, -> { where(status: 'manual') } - scope :scheduled, -> { where(status: 'scheduled') } - scope :alive, -> { where(status: [:created, :preparing, :pending, :running]) } - scope :created_or_pending, -> { where(status: [:created, :pending]) } - scope :running_or_pending, -> { where(status: [:running, :pending]) } - scope :finished, -> { where(status: [:success, :failed, :canceled]) } - scope :failed_or_canceled, -> { where(status: [:failed, :canceled]) } + scope :created, -> { with_status(:created) } + scope :preparing, -> { with_status(:preparing) } + scope :relevant, -> { without_status(:created) } + scope :running, -> { with_status(:running) } + scope :pending, -> { with_status(:pending) } + scope :success, -> { with_status(:success) } + scope :failed, -> { with_status(:failed) } + scope :canceled, -> { with_status(:canceled) } + scope :skipped, -> { with_status(:skipped) } + scope :manual, -> { with_status(:manual) } + scope :scheduled, -> { with_status(:scheduled) } + scope :alive, -> { with_status(:created, :preparing, :pending, :running) } + scope :created_or_pending, -> { with_status(:created, :pending) } + scope :running_or_pending, -> { with_status(:running, :pending) } + scope :finished, -> { with_status(:success, :failed, :canceled) } + scope :failed_or_canceled, -> { with_status(:failed, :canceled) } scope :cancelable, -> do where(status: [:running, :preparing, :pending, :created, :scheduled]) diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index 22b6b1d720c..e4fe46d722a 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -179,7 +179,7 @@ module RelativePositioning relation = yield relation if block_given? relation - .pluck(self.class.parent_column, "#{calculation}(relative_position) AS position") + .pluck(self.class.parent_column, Arel.sql("#{calculation}(relative_position) AS position")) .first&. last end diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index b9ffc64e4a9..9becab632f3 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -46,7 +46,7 @@ module Routable # See https://gitlab.com/gitlab-org/gitlab-ce/issues/18603. Also note that # our unique index is case-sensitive in Postgres. binary = Gitlab::Database.mysql? ? 'BINARY' : '' - order_sql = "(CASE WHEN #{binary} routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)" + order_sql = Arel.sql("(CASE WHEN #{binary} routes.path = #{connection.quote(path)} THEN 0 ELSE 1 END)") found = where_full_path_in([path]).reorder(order_sql).take return found if found diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index 8c769be0489..1293df571a3 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -52,7 +52,7 @@ module TokenAuthenticatable mod.define_method("#{token_field}_matches?") do |other_token| token = read_attribute(token_field) - token.present? && ActiveSupport::SecurityUtils.variable_size_secure_compare(other_token, token) + token.present? && ActiveSupport::SecurityUtils.secure_compare(other_token, token) end end diff --git a/app/models/email.rb b/app/models/email.rb index 0ddaa049c3b..580633d3232 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -4,9 +4,8 @@ class Email < ApplicationRecord include Sortable include Gitlab::SQL::Pattern - belongs_to :user + belongs_to :user, optional: false - validates :user_id, presence: true validates :email, presence: true, uniqueness: true, devise_email: true validate :unique_email, if: ->(email) { email.email_changed? } diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb index 22cedf57b86..5c53cfd8c27 100644 --- a/app/models/merge_requests_closing_issues.rb +++ b/app/models/merge_requests_closing_issues.rb @@ -25,7 +25,7 @@ class MergeRequestsClosingIssues < ApplicationRecord class << self def count_for_collection(ids, current_user) - closing_merge_requests(ids, current_user).group(:issue_id).pluck('issue_id', 'COUNT(*) as count') + closing_merge_requests(ids, current_user).group(:issue_id).pluck('issue_id', Arel.sql('COUNT(*) as count')) end def count_for_issue(id, current_user) diff --git a/app/models/project.rb b/app/models/project.rb index a6e0b5722b6..b278b2792f4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -357,7 +357,7 @@ class Project < ApplicationRecord scope :with_unmigrated_storage, -> { where('storage_version < :version OR storage_version IS NULL', version: LATEST_STORAGE_VERSION) } # last_activity_at is throttled every minute, but last_repository_updated_at is updated with every push - scope :sorted_by_activity, -> { reorder("GREATEST(COALESCE(last_activity_at, '1970-01-01'), COALESCE(last_repository_updated_at, '1970-01-01')) DESC") } + scope :sorted_by_activity, -> { reorder(Arel.sql("GREATEST(COALESCE(last_activity_at, '1970-01-01'), COALESCE(last_repository_updated_at, '1970-01-01')) DESC")) } scope :sorted_by_stars_desc, -> { reorder(star_count: :desc) } scope :sorted_by_stars_asc, -> { reorder(star_count: :asc) } @@ -612,7 +612,7 @@ class Project < ApplicationRecord end end - def initialize(attributes = {}) + def initialize(attributes = nil) # We can't use default_value_for because the database has a default # value of 0 for visibility_level. If someone attempts to create a # private project, default_value_for will assume that the @@ -622,6 +622,8 @@ class Project < ApplicationRecord # # To fix the problem, we assign the actual default in the application if # no explicit visibility has been initialized. + attributes ||= {} + unless visibility_attribute_present?(attributes) attributes[:visibility_level] = Gitlab::CurrentSettings.default_project_visibility end @@ -1557,7 +1559,7 @@ class Project < ApplicationRecord end def valid_runners_token?(token) - self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token) + self.runners_token && ActiveSupport::SecurityUtils.secure_compare(token, self.runners_token) end # rubocop: disable CodeReuse/ServiceClass diff --git a/app/models/project_services/ci_service.rb b/app/models/project_services/ci_service.rb index f0ef2d925ab..47106d7bdbb 100644 --- a/app/models/project_services/ci_service.rb +++ b/app/models/project_services/ci_service.rb @@ -7,7 +7,7 @@ class CiService < Service default_value_for :category, 'ci' def valid_token?(token) - self.respond_to?(:token) && self.token.present? && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token) + self.respond_to?(:token) && self.token.present? && ActiveSupport::SecurityUtils.secure_compare(token, self.token) end def self.supported_events diff --git a/app/models/project_services/slash_commands_service.rb b/app/models/project_services/slash_commands_service.rb index bfabc6d262c..5f5cff97808 100644 --- a/app/models/project_services/slash_commands_service.rb +++ b/app/models/project_services/slash_commands_service.rb @@ -12,7 +12,7 @@ class SlashCommandsService < Service def valid_token?(token) self.respond_to?(:token) && self.token.present? && - ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token) + ActiveSupport::SecurityUtils.secure_compare(token, self.token) end def self.supported_events diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index 4a7ce00b8e2..aaf56048b5c 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -44,7 +44,7 @@ module Ci # rubocop: disable CodeReuse/ActiveRecord def stage_indexes_of_created_processables - created_processables.order(:stage_idx).pluck('distinct stage_idx') + created_processables.order(:stage_idx).pluck(Arel.sql('DISTINCT stage_idx')) end # rubocop: enable CodeReuse/ActiveRecord @@ -68,7 +68,7 @@ module Ci latest_statuses = pipeline.statuses.latest .group(:name) .having('count(*) > 1') - .pluck('max(id)', 'name') + .pluck(Arel.sql('MAX(id)'), 'name') # mark builds that are retried pipeline.statuses.latest diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index e9659f5489a..e78e5d5fc2c 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -27,8 +27,7 @@ module Groups @group.build_chat_team(name: response['name'], team_id: response['id']) end - @group.save - @group.add_owner(current_user) + @group.add_owner(current_user) if @group.save @group end diff --git a/bin/bundle b/bin/bundle index 66e9889e8b4..f19acf5b5cc 100755 --- a/bin/bundle +++ b/bin/bundle @@ -1,3 +1,3 @@ #!/usr/bin/env ruby -ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__) +ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__) load Gem.bin_path('bundler', 'bundle') diff --git a/bin/setup b/bin/setup index 883825bc0a2..94fd4d79775 100755 --- a/bin/setup +++ b/bin/setup @@ -1,33 +1,36 @@ #!/usr/bin/env ruby - -require "pathname" +require 'fileutils' +include FileUtils # path to your application root. -APP_ROOT = Pathname.new File.expand_path("../../", __FILE__) +APP_ROOT = File.expand_path('..', __dir__) def system!(*args) system(*args) || abort("\n== Command #{args} failed ==") end -Dir.chdir APP_ROOT do - # This script is a starting point to set up your application. - # Add necessary setup steps to this file: +chdir APP_ROOT do + # This script is a starting point to setup your application. + # Add necessary setup steps to this file. + + puts '== Installing dependencies ==' + system! 'gem install bundler --conservative' + system('bundle check') || system!('bundle install') - puts "== Installing dependencies ==" - system! "gem install bundler --conservative" - system("bundle check") || system!("bundle install") + # Install JavaScript dependencies if using Yarn + # system('bin/yarn') # puts "\n== Copying sample files ==" - # unless File.exist?("config/database.yml") - # cp "config/database.yml.sample", "config/database.yml" + # unless File.exist?('config/database.yml') + # cp 'config/database.yml.sample', 'config/database.yml' # end puts "\n== Preparing database ==" - system! "bin/rails db:setup" + system! 'bin/rails db:setup' puts "\n== Removing old logs and tempfiles ==" - system! "bin/rails log:clear tmp:clear" + system! 'bin/rails log:clear tmp:clear' puts "\n== Restarting application server ==" - system! "bin/rails restart" + system! 'bin/rails restart' end diff --git a/bin/update b/bin/update index a8e4462f203..58bfaed518c 100755 --- a/bin/update +++ b/bin/update @@ -1,10 +1,9 @@ #!/usr/bin/env ruby -require 'pathname' require 'fileutils' include FileUtils # path to your application root. -APP_ROOT = Pathname.new File.expand_path('../../', __FILE__) +APP_ROOT = File.expand_path('..', __dir__) def system!(*args) system(*args) || abort("\n== Command #{args} failed ==") @@ -18,6 +17,9 @@ chdir APP_ROOT do system! 'gem install bundler --conservative' system('bundle check') || system!('bundle install') + # Install JavaScript dependencies if using Yarn + # system('bin/yarn') + puts "\n== Updating database ==" system! 'bin/rails db:migrate' diff --git a/bin/yarn b/bin/yarn new file mode 100755 index 00000000000..460dd565b4a --- /dev/null +++ b/bin/yarn @@ -0,0 +1,11 @@ +#!/usr/bin/env ruby +APP_ROOT = File.expand_path('..', __dir__) +Dir.chdir(APP_ROOT) do + begin + exec "yarnpkg", *ARGV + rescue Errno::ENOENT + $stderr.puts "Yarn executable was not detected in the system." + $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install" + exit 1 + end +end diff --git a/config/application.rb b/config/application.rb index edf8b3e87f9..de386506233 100644 --- a/config/application.rb +++ b/config/application.rb @@ -1,8 +1,14 @@ -require File.expand_path('boot', __dir__) +require_relative 'boot' -require 'rails/all' +# Based on https://github.com/rails/rails/blob/v5.2.3/railties/lib/rails/all.rb +# Only load the railties we need instead of loading everything +require 'active_record/railtie' +require 'action_controller/railtie' +require 'action_view/railtie' +require 'action_mailer/railtie' +require 'rails/test_unit/railtie' -Bundler.require(:default, Rails.env) +Bundler.require(*Rails.groups) module Gitlab class Application < Rails::Application @@ -25,6 +31,8 @@ module Gitlab # Application configuration should go into files in config/initializers # -- all .rb files in that directory are automatically loaded. + config.active_record.sqlite3.represent_boolean_as_integer = true + # Sidekiq uses eager loading, but directories not in the standard Rails # directories must be added to the eager load paths: # https://github.com/mperham/sidekiq/wiki/FAQ#why-doesnt-sidekiq-autoload-my-rails-application-code @@ -86,13 +94,6 @@ module Gitlab # Configure the default encoding used in templates for Ruby 1.9. config.encoding = "utf-8" - # ActionCable mount point. - # The default Rails' mount point is `/cable` which may conflict with existing - # namespaces/users. - # https://github.com/rails/rails/blob/5-0-stable/actioncable/lib/action_cable.rb#L38 - # Please change this value when configuring ActionCable for real usage. - config.action_cable.mount_path = "/-/cable" - # Configure sensitive parameters which will be filtered from the log file. # # Parameters filtered: @@ -272,5 +273,10 @@ module Gitlab Gitlab::Routing.add_helpers(project_url_helpers) Gitlab::Routing.add_helpers(MilestonesRoutingHelper) end + + # This makes generated cookies to be compatible with Rails 5.1 and older + # We can remove this when we're confident that there are no issues with the Rails 5.2 upgrade + # and we won't need to rollback to older versions + config.action_dispatch.use_authenticated_cookie_encryption = false end end diff --git a/config/environments/development.rb b/config/environments/development.rb index ac9b02b08d5..3881f1be152 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -7,6 +7,7 @@ Rails.application.configure do config.cache_classes = false # Show full error reports and disable caching + config.active_record.verbose_query_logs = true config.consider_all_requests_local = true config.action_controller.perform_caching = false diff --git a/config/environments/test.rb b/config/environments/test.rb index e7166882eea..a564ef74734 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -23,6 +23,7 @@ Rails.application.configure do config.public_file_server.headers = { 'Cache-Control' => 'public, max-age=3600' } # Show full error reports and disable caching + config.active_record.verbose_query_logs = true config.consider_all_requests_local = true config.action_controller.perform_caching = false diff --git a/config/initializers/active_record_data_types.rb b/config/initializers/active_record_data_types.rb index e95157bfde5..151bce4d130 100644 --- a/config/initializers/active_record_data_types.rb +++ b/config/initializers/active_record_data_types.rb @@ -22,7 +22,7 @@ if Gitlab::Database.postgresql? # # When schema dumping, `timestamptz` columns will be output as # `t.datetime_with_timezone`. - def initialize_type_map(mapping) + def initialize_type_map(mapping = type_map) super mapping mapping.register_type 'timestamptz' do |_, _, sql_type| @@ -51,7 +51,7 @@ elsif Gitlab::Database.mysql? # # When schema dumping, `timestamp` columns will be output as # `t.datetime_with_timezone`. - def initialize_type_map(mapping) + def initialize_type_map(mapping = type_map) super mapping mapping.register_type(/timestamp/i) do |sql_type| diff --git a/config/initializers/active_record_preloader.rb b/config/initializers/active_record_preloader.rb index 3b16014f302..a293909149e 100644 --- a/config/initializers/active_record_preloader.rb +++ b/config/initializers/active_record_preloader.rb @@ -1,9 +1,22 @@ module ActiveRecord module Associations class Preloader + class NullPreloader + def self.new(klass, owners, reflection, preload_scope) + self + end + + def self.run(preloader) + end + + def self.preloaded_records + [] + end + end + module NoCommitPreloader - def preloader_for(reflection, owners, rhs_klass) - return NullPreloader if rhs_klass == ::Commit + def preloader_for(reflection, owners) + return NullPreloader if owners.first.association(reflection.name).klass == ::Commit super end diff --git a/config/initializers/active_record_verbose_query_logs.rb b/config/initializers/active_record_verbose_query_logs.rb deleted file mode 100644 index 1c5fbc8e830..00000000000 --- a/config/initializers/active_record_verbose_query_logs.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -# This is backport of https://github.com/rails/rails/pull/26815/files -# Enabled by default for every non-production environment - -module ActiveRecord - class LogSubscriber - module VerboseQueryLogs - def debug(progname = nil, &block) - return unless super - - log_query_source - end - - def log_query_source - source_line, line_number = extract_callstack(caller_locations) - - if source_line - if defined?(::Rails.root) - app_root = "#{::Rails.root}/".freeze - source_line = source_line.sub(app_root, "") - end - - logger.debug(" ↳ #{source_line}:#{line_number}") - end - end - - def extract_callstack(callstack) - line = callstack.find do |frame| - frame.absolute_path && !ignored_callstack(frame.absolute_path) - end - - offending_line = line || callstack.first - [ - offending_line.path, - offending_line.lineno, - offending_line.label - ] - end - - LOG_SUBSCRIBER_FILE = ActiveRecord::LogSubscriber.method(:logger).source_location.first - RAILS_GEM_ROOT = File.expand_path("../../../..", LOG_SUBSCRIBER_FILE) + "/" - APP_CONFIG_ROOT = File.expand_path("..", __dir__) + "/" - - def ignored_callstack(path) - path.start_with?(APP_CONFIG_ROOT, RAILS_GEM_ROOT, RbConfig::CONFIG["rubylibdir"]) - end - end - - if Rails.version.start_with?("5.2") - raise "Remove this monkey patch: #{__FILE__}" - else - prepend(VerboseQueryLogs) unless Rails.env.production? - end - end -end diff --git a/config/initializers/ar_speed_up_migration_checking.rb b/config/initializers/ar_speed_up_migration_checking.rb index f98b246db0b..c4ffcc54cb2 100644 --- a/config/initializers/ar_speed_up_migration_checking.rb +++ b/config/initializers/ar_speed_up_migration_checking.rb @@ -2,17 +2,14 @@ if Rails.env.test? require 'active_record/migration' module ActiveRecord - class Migrator - class << self - alias_method :migrations_unmemoized, :migrations + class MigrationContext + alias_method :migrations_unmemoized, :migrations - # This method is called a large number of times per rspec example, and - # it reads + parses `db/migrate/*` each time. Memoizing it can save 0.5 - # seconds per spec. - def migrations(paths) - @migrations ||= {} - (@migrations[paths] ||= migrations_unmemoized(paths)).dup - end + # This method is called a large number of times per rspec example, and + # it reads + parses `db/migrate/*` each time. Memoizing it can save 0.5 + # seconds per spec. + def migrations + @migrations ||= migrations_unmemoized end end end diff --git a/config/initializers/config_initializers_active_record_locking.rb b/config/initializers/config_initializers_active_record_locking.rb index 608d63223a3..915247826e9 100644 --- a/config/initializers/config_initializers_active_record_locking.rb +++ b/config/initializers/config_initializers_active_record_locking.rb @@ -22,10 +22,11 @@ module ActiveRecord # Patched because when `lock_version` is read as `0`, it may actually be `NULL` in the DB. possible_previous_lock_value = previous_lock_value.to_i == 0 ? [nil, 0] : previous_lock_value - affected_rows = self.class.unscoped._update_record( - arel_attributes_with_values(attribute_names), - self.class.primary_key => id_in_database, - locking_column => possible_previous_lock_value + affected_rows = self.class.unscoped.where( + locking_column => possible_previous_lock_value, + self.class.primary_key => id_in_database + ).update_all( + attributes_with_values_for_update(attribute_names) ) if affected_rows != 1 diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb index 1ad93e14f7e..fbec28186eb 100644 --- a/config/initializers/lograge.rb +++ b/config/initializers/lograge.rb @@ -18,7 +18,7 @@ unless Sidekiq.server? .map { |k, v| { key: k, value: v } } payload = { - time: event.time.utc.iso8601(3), + time: Time.now.utc.iso8601(3), params: params, remote_ip: event.payload[:remote_ip], user_id: event.payload[:user_id], diff --git a/config/initializers/mysql_ignore_postgresql_options.rb b/config/initializers/mysql_ignore_postgresql_options.rb index 9a569be7674..e6a7d9bef52 100644 --- a/config/initializers/mysql_ignore_postgresql_options.rb +++ b/config/initializers/mysql_ignore_postgresql_options.rb @@ -15,7 +15,6 @@ if defined?(ActiveRecord::ConnectionAdapters::Mysql2Adapter) module ConnectionAdapters class Mysql2Adapter < AbstractMysqlAdapter alias_method :__gitlab_add_index, :add_index - alias_method :__gitlab_add_index_sql, :add_index_sql alias_method :__gitlab_add_index_options, :add_index_options def add_index(table_name, column_name, options = {}) @@ -24,12 +23,6 @@ if defined?(ActiveRecord::ConnectionAdapters::Mysql2Adapter) end end - def add_index_sql(table_name, column_name, options = {}) - unless options[:opclasses] - __gitlab_add_index_sql(table_name, column_name, options) - end - end - def add_index_options(table_name, column_name, options = {}) if options[:using] && options[:using] == :gin options = options.dup diff --git a/config/initializers/postgresql_cte.rb b/config/initializers/postgresql_cte.rb index 56689bc8e74..68d53c4edbf 100644 --- a/config/initializers/postgresql_cte.rb +++ b/config/initializers/postgresql_cte.rb @@ -94,8 +94,8 @@ module ActiveRecord end end - def build_arel - arel = super() + def build_arel(aliases) + arel = super build_with(arel) if @values[:with] diff --git a/db/schema.rb b/db/schema.rb index 9a8b64689bd..9d28c5d2383 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,11 +10,11 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190703130053) do +ActiveRecord::Schema.define(version: 2019_07_03_130053) do # These are extensions that must be enabled in order to support this database - enable_extension "plpgsql" enable_extension "pg_trgm" + enable_extension "plpgsql" create_table "abuse_reports", id: :serial, force: :cascade do |t| t.integer "reporter_id" diff --git a/lib/api/helpers/pagination.rb b/lib/api/helpers/pagination.rb index 2a9b17ad22a..71bbc218f94 100644 --- a/lib/api/helpers/pagination.rb +++ b/lib/api/helpers/pagination.rb @@ -205,7 +205,9 @@ module API limited_total_count = pagination_data.total_count_with_limit if limited_total_count > Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT - pagination_data.without_count + # The call to `total_count_with_limit` memoizes `@arel` because of a call to `references_eager_loaded_tables?` + # We need to call `reset` because `without_count` relies on `@arel` being unmemoized + pagination_data.reset.without_count else pagination_data end diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index ff73a49d5e8..100463fcb95 100644 --- a/lib/api/helpers/runner.rb +++ b/lib/api/helpers/runner.rb @@ -7,8 +7,7 @@ module API JOB_TOKEN_PARAM = :token def runner_registration_token_valid? - ActiveSupport::SecurityUtils.variable_size_secure_compare(params[:token], - Gitlab::CurrentSettings.runners_registration_token) + ActiveSupport::SecurityUtils.secure_compare(params[:token], Gitlab::CurrentSettings.runners_registration_token) end def authenticate_runner! diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 469a7fd9f7b..32d5e4b9ea3 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -45,7 +45,7 @@ module Gitlab # need to be added to the application settings. To prevent Rake tasks # and other callers from failing, use any loaded settings and return # defaults for missing columns. - if ActiveRecord::Migrator.needs_migration? + if ActiveRecord::Base.connection.migration_context.needs_migration? db_attributes = current_settings&.attributes || {} fake_application_settings(db_attributes) elsif current_settings.present? diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 1177f8ea99e..64e7f371115 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -128,7 +128,7 @@ module Gitlab order = "#{field} IS NULL, #{order}" if direction == 'ASC' end - order + Arel.sql(order) end def self.nulls_first_order(field, direction = 'ASC') @@ -142,7 +142,7 @@ module Gitlab order = "#{field} IS NULL, #{order}" if direction == 'DESC' end - order + Arel.sql(order) end def self.random diff --git a/lib/gitlab/database/grant.rb b/lib/gitlab/database/grant.rb index 862ab96c887..26adf4e221b 100644 --- a/lib/gitlab/database/grant.rb +++ b/lib/gitlab/database/grant.rb @@ -24,7 +24,7 @@ module Gitlab begin from(nil) - .pluck("has_table_privilege(#{quoted_table}, 'TRIGGER')") + .pluck(Arel.sql("has_table_privilege(#{quoted_table}, 'TRIGGER')")) .first rescue ActiveRecord::StatementInvalid # This error is raised when using a non-existing table name. In this diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index 78c5e2a2656..bbeda7dae0f 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -23,10 +23,11 @@ describe Admin::RunnersController do control_count = ActiveRecord::QueryRecorder.new { get :index }.count - create(:ci_runner, :tagged_only) + create_list(:ci_runner, 5, :tagged_only) # There is still an N+1 query for `runner.builds.count` - expect { get :index }.not_to exceed_query_limit(control_count + 1) + # We also need to add 1 because it takes 2 queries to preload tags + expect { get :index }.not_to exceed_query_limit(control_count + 6) expect(response).to have_gitlab_http_status(200) expect(response.body).to have_content('tag1') diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index aa0442ab847..94998d302f9 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -16,7 +16,7 @@ describe AvatarsHelper do shared_examples 'resource with a custom avatar' do |source_type| it 'returns a custom avatar image' do expect(public_send("#{source_type}_icon", *helper_args)) - .to eq "<img src=\"#{resource.avatar.url}\" alt=\"Banana sample\" />" + .to eq "<img src=\"#{resource.avatar.url}\" />" end end diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb index e6aacb5b92b..d25f0c6de4a 100644 --- a/spec/helpers/emails_helper_spec.rb +++ b/spec/helpers/emails_helper_spec.rb @@ -103,7 +103,7 @@ describe EmailsHelper do appearance = create :appearance, header_logo: fixture_file_upload('spec/fixtures/dk.png') expect(header_logo).to eq( - %{<img style="height: 50px" src="/uploads/-/system/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />} + %{<img style="height: 50px" src="/uploads/-/system/appearance/header_logo/#{appearance.id}/dk.png" />} ) end end diff --git a/spec/lib/api/helpers/pagination_spec.rb b/spec/lib/api/helpers/pagination_spec.rb index c788da55cd2..b0a00392957 100644 --- a/spec/lib/api/helpers/pagination_spec.rb +++ b/spec/lib/api/helpers/pagination_spec.rb @@ -114,7 +114,7 @@ describe API::Helpers::Pagination do expect(paginated_relation.order_values).to be_present expect(paginated_relation.order_values.size).to eq(1) expect(paginated_relation.order_values.first).to be_descending - expect(paginated_relation.order_values.first.expr.name).to eq :id + expect(paginated_relation.order_values.first.expr.name).to eq 'id' end end @@ -151,9 +151,9 @@ describe API::Helpers::Pagination do expect(paginated_relation.order_values).to be_present expect(paginated_relation.order_values.size).to eq(2) expect(paginated_relation.order_values.first).to be_descending - expect(paginated_relation.order_values.first.expr.name).to eq :name + expect(paginated_relation.order_values.first.expr.name).to eq 'name' expect(paginated_relation.order_values.second).to be_descending - expect(paginated_relation.order_values.second.expr.name).to eq :id + expect(paginated_relation.order_values.second.expr.name).to eq 'id' end it 'returns the right records (first page)' do @@ -341,7 +341,7 @@ describe API::Helpers::Pagination do expect(resource.order_values).to be_empty expect(paginated_relation.order_values).to be_present expect(paginated_relation.order_values.first).to be_ascending - expect(paginated_relation.order_values.first.expr.name).to eq :id + expect(paginated_relation.order_values.first.expr.name).to eq 'id' end it 'is present it does not add anything' do @@ -349,7 +349,7 @@ describe API::Helpers::Pagination do expect(paginated_relation.order_values).to be_present expect(paginated_relation.order_values.first).to be_descending - expect(paginated_relation.order_values.first.expr.name).to eq :created_at + expect(paginated_relation.order_values.first.expr.name).to eq 'created_at' end end end diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index 909dbffa38f..db31e5280a3 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -80,7 +80,7 @@ describe Gitlab::CurrentSettings do # during the initialization phase of the test suite, so instead let's mock the internals of it expect(ActiveRecord::Base.connection).not_to receive(:active?) expect(ActiveRecord::Base.connection).not_to receive(:cached_table_exists?) - expect(ActiveRecord::Migrator).not_to receive(:needs_migration?) + expect_any_instance_of(ActiveRecord::MigrationContext).not_to receive(:needs_migration?) expect(ActiveRecord::QueryRecorder.new { described_class.current_application_settings }.count).to eq(0) end end @@ -109,7 +109,7 @@ describe Gitlab::CurrentSettings do context 'with pending migrations' do before do - expect(ActiveRecord::Migrator).to receive(:needs_migration?).and_return(true) + expect_any_instance_of(ActiveRecord::MigrationContext).to receive(:needs_migration?).and_return(true) end shared_examples 'a non-persisted ApplicationSetting object' do diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb index 48139c2f9dc..7833b9f387d 100644 --- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -105,6 +105,7 @@ describe Gitlab::Email::Handler::CreateIssueHandler do context "when the issue could not be saved" do before do allow_any_instance_of(Issue).to receive(:persisted?).and_return(false) + allow_any_instance_of(Issue).to receive(:ensure_metrics).and_return(nil) end it "raises an InvalidIssueError" do diff --git a/spec/migrations/active_record/schema_spec.rb b/spec/migrations/active_record/schema_spec.rb index fbf5d387d0e..bc246f88685 100644 --- a/spec/migrations/active_record/schema_spec.rb +++ b/spec/migrations/active_record/schema_spec.rb @@ -15,7 +15,7 @@ describe ActiveRecord::Schema do it '> schema version equals last migration timestamp' do defined_schema_version = File.open(Rails.root.join('db', 'schema.rb')) do |file| file.find { |line| line =~ /ActiveRecord::Schema.define/ } - end.match(/(\d+)/)[0].to_i + end.match(/(\d{4}_\d{2}_\d{2}_\d{6})/)[0].to_i expect(defined_schema_version).to eq(latest_migration_timestamp) end diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index 7b18e24a351..4ccbdf29df6 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -24,7 +24,7 @@ describe 'CycleAnalytics#issue' do ["list label added to issue", -> (context, data) do if data[:issue].persisted? - data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) + data[:issue].update(label_ids: [context.create(:list).label_id]) end end]], post_fn: -> (context, data) do diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index 73fc98ba6cf..c99c38e9cf3 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -25,7 +25,7 @@ describe 'CycleAnalytics#plan' do end], ["list label added to issue", -> (context, data) do - data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) + data[:issue].update(label_ids: [context.create(:list).label_id]) end]], end_time_conditions: [["issue mentioned in a commit", -> (context, data) do diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb index 07858fe8a70..7aa0d97b194 100644 --- a/spec/models/issue/metrics_spec.rb +++ b/spec/models/issue/metrics_spec.rb @@ -32,7 +32,7 @@ describe Issue::Metrics do context "list labels" do it "records the first time an issue is associated with a list label" do - list_label = create(:label, lists: [create(:list)]) + list_label = create(:list).label time = Time.now Timecop.freeze(time) { subject.update(label_ids: [list_label.id]) } metrics = subject.metrics @@ -43,9 +43,9 @@ describe Issue::Metrics do it "does not record the second time an issue is associated with a list label" do time = Time.now - first_list_label = create(:label, lists: [create(:list)]) + first_list_label = create(:list).label Timecop.freeze(time) { subject.update(label_ids: [first_list_label.id]) } - second_list_label = create(:label, lists: [create(:list)]) + second_list_label = create(:list).label Timecop.freeze(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) } metrics = subject.metrics diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 1d7bf91fda1..3e3de051732 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -1844,7 +1844,7 @@ describe NotificationService, :mailer do describe 'ProjectMember' do let(:project) { create(:project) } - set(:added_user) { create(:user) } + let(:added_user) { create(:user) } describe '#new_access_request' do context 'for a project in a user namespace' do diff --git a/spec/support/helpers/migrations_helpers.rb b/spec/support/helpers/migrations_helpers.rb index cc1a28cb264..272b24f7541 100644 --- a/spec/support/helpers/migrations_helpers.rb +++ b/spec/support/helpers/migrations_helpers.rb @@ -18,8 +18,12 @@ module MigrationsHelpers ActiveRecord::Migrator.migrations_paths end + def migration_context + ActiveRecord::MigrationContext.new(migrations_paths) + end + def migrations - ActiveRecord::Migrator.migrations(migrations_paths) + migration_context.migrations end def clear_schema_cache! @@ -96,8 +100,7 @@ module MigrationsHelpers def schema_migrate_down! disable_migrations_output do - ActiveRecord::Migrator.migrate(migrations_paths, - migration_schema_version) + migration_context.down(migration_schema_version) end reset_column_in_all_models @@ -107,7 +110,7 @@ module MigrationsHelpers reset_column_in_all_models disable_migrations_output do - ActiveRecord::Migrator.migrate(migrations_paths) + migration_context.up end reset_column_in_all_models @@ -123,7 +126,7 @@ module MigrationsHelpers end def migrate! - ActiveRecord::Migrator.up(migrations_paths) do |migration| + migration_context.up do |migration| migration.name == described_class.name end end diff --git a/spec/views/notify/pipeline_failed_email.html.haml_spec.rb b/spec/views/notify/pipeline_failed_email.html.haml_spec.rb index 623237b111a..bf633a118ca 100644 --- a/spec/views/notify/pipeline_failed_email.html.haml_spec.rb +++ b/spec/views/notify/pipeline_failed_email.html.haml_spec.rb @@ -28,7 +28,7 @@ describe 'notify/pipeline_failed_email.html.haml' do expect(rendered).to have_content "Your pipeline has failed" expect(rendered).to have_content pipeline.project.name - expect(rendered).to have_content pipeline.git_commit_message.truncate(50).gsub!(/\s+/, ' ') + expect(rendered).to have_content pipeline.git_commit_message.truncate(50).gsub(/\s+/, ' ') expect(rendered).to have_content pipeline.commit.author_name expect(rendered).to have_content "##{pipeline.id}" expect(rendered).to have_content pipeline.user.name @@ -45,7 +45,7 @@ describe 'notify/pipeline_failed_email.html.haml' do expect(rendered).to have_content "Your pipeline has failed" expect(rendered).to have_content pipeline.project.name - expect(rendered).to have_content pipeline.git_commit_message.truncate(50).gsub!(/\s+/, ' ') + expect(rendered).to have_content pipeline.git_commit_message.truncate(50).gsub(/\s+/, ' ') expect(rendered).to have_content pipeline.commit.author_name expect(rendered).to have_content "##{pipeline.id}" expect(rendered).to have_content "by API" diff --git a/spec/views/notify/pipeline_failed_email.text.erb_spec.rb b/spec/views/notify/pipeline_failed_email.text.erb_spec.rb index 81245239eba..060274eb56a 100644 --- a/spec/views/notify/pipeline_failed_email.text.erb_spec.rb +++ b/spec/views/notify/pipeline_failed_email.text.erb_spec.rb @@ -30,7 +30,7 @@ describe 'notify/pipeline_failed_email.text.erb' do expect(rendered).to have_content('Your pipeline has failed') expect(rendered).to have_content(pipeline.project.name) - expect(rendered).to have_content(pipeline.git_commit_message.truncate(50).gsub!(/\s+/, ' ')) + expect(rendered).to have_content(pipeline.git_commit_message.truncate(50).gsub(/\s+/, ' ')) expect(rendered).to have_content(pipeline.commit.author_name) expect(rendered).to have_content("##{pipeline.id}") expect(rendered).to have_content(pipeline.user.name) diff --git a/spec/views/notify/pipeline_success_email.html.haml_spec.rb b/spec/views/notify/pipeline_success_email.html.haml_spec.rb index a876bf13e11..46a6c908863 100644 --- a/spec/views/notify/pipeline_success_email.html.haml_spec.rb +++ b/spec/views/notify/pipeline_success_email.html.haml_spec.rb @@ -28,7 +28,7 @@ describe 'notify/pipeline_success_email.html.haml' do expect(rendered).to have_content "Your pipeline has passed" expect(rendered).to have_content pipeline.project.name - expect(rendered).to have_content pipeline.git_commit_message.truncate(50).gsub!(/\s+/, ' ') + expect(rendered).to have_content pipeline.git_commit_message.truncate(50).gsub(/\s+/, ' ') expect(rendered).to have_content pipeline.commit.author_name expect(rendered).to have_content "##{pipeline.id}" expect(rendered).to have_content pipeline.user.name @@ -45,7 +45,7 @@ describe 'notify/pipeline_success_email.html.haml' do expect(rendered).to have_content "Your pipeline has passed" expect(rendered).to have_content pipeline.project.name - expect(rendered).to have_content pipeline.git_commit_message.truncate(50).gsub!(/\s+/, ' ') + expect(rendered).to have_content pipeline.git_commit_message.truncate(50).gsub(/\s+/, ' ') expect(rendered).to have_content pipeline.commit.author_name expect(rendered).to have_content "##{pipeline.id}" expect(rendered).to have_content "by API" |