Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #37825 - Upgrade Rails to 7.0 #10299

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ FOREMAN_GEMFILE = __FILE__ unless defined? FOREMAN_GEMFILE

source 'https://rubygems.org'

gem 'rails', '~> 6.1.6'
gem 'rails', '~> 7.0.3'
gem 'rest-client', '>= 2.0.0', '< 3', :require => 'rest_client'
gem 'audited', '~> 5.0', '!= 5.1.0'
gem 'will_paginate', '~> 3.3'
Expand Down
4 changes: 0 additions & 4 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,6 @@ def process_ajax_error(exception, action = nil)
render :partial => "common/ajax_error", :status => :internal_server_error, :locals => { :message => message }
end

def redirect_back_or_to(url)
redirect_back(fallback_location: url, allow_other_host: false)
end

def saved_redirect_url_or(default)
session["redirect_to_url_#{controller_name}"] || default
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/links_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class LinksController < ApplicationController

def show
url = external_url(type: params[:type], options: params)
redirect_to(url)
redirect_to(url, allow_other_host: true)
end

def external_url(type:, options: {})
Expand Down
2 changes: 1 addition & 1 deletion app/graphql/collection_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def validate
end

def preloader_for_association(records)
::ActiveRecord::Associations::Preloader.new.preload(records, association_name, base_scope).first
::ActiveRecord::Associations::Preloader.new(records: records, associations: association_name, scope: base_scope).call.first
end

def read_association(preloader, record)
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/layout_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def popover(title, msg, options = {})

def icon_text(i, text = "", opts = {})
opts[:kind] ||= "glyphicon"
(content_tag(:span, "", :class => "#{opts[:kind] + ' ' + opts[:kind]}-#{i} #{opts[:class]}", :title => opts[:title]) + " " + text).html_safe
(content_tag(:span, "", :class => "#{opts[:kind] + ' ' + opts[:kind]}-#{i} #{opts[:class]}", :title => opts[:title]) + " " + text.to_s).html_safe
end

def alert(opts = {})
Expand Down
4 changes: 4 additions & 0 deletions app/models/concerns/hidden_value.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ def safe_value
def hidden_value
HIDDEN_VALUE
end

def hidden_value?
attributes['hidden_value']
end
end
2 changes: 1 addition & 1 deletion app/models/concerns/hostext/token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Token
included do
has_one :token, :foreign_key => :host_id, :dependent => :destroy, :inverse_of => :host, :class_name => 'Token::Build'

scope :for_token, ->(token) { joins(:token).where(:tokens => { :value => token }).where("expires >= ?", Time.now.utc.to_s(:db)).select('hosts.*') }
scope :for_token, ->(token) { joins(:token).where(:tokens => { :value => token }).where("expires >= ?", Time.now.utc.to_fs(:db)).select('hosts.*') }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL: there's .to_fs in Rails. I wonder if Rails is smart enough to understand

Suggested change
scope :for_token, ->(token) { joins(:token).where(:tokens => { :value => token }).where("expires >= ?", Time.now.utc.to_fs(:db)).select('hosts.*') }
scope :for_token, ->(token) { joins(:token).where(:tokens => { :value => token }).where("expires >= ?", Time.now.utc).select('hosts.*') }

scope :for_token_when_built, ->(token) { joins(:token).where(:tokens => { :value => token }).select('hosts.*') }

before_validation :refresh_token_on_build
Expand Down
5 changes: 5 additions & 0 deletions app/models/host.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,9 @@ def self.respond_to_missing?(method, include_private = false)
method.to_s =~ /\Afind_by_(.*)\Z/ || method.to_s.include?('create') ||
[:reorder, :new].include?(method) || super
end

# This is a workaround for https://github.com/rails/rails/blob/v7.0.4/activerecord/lib/active_record/reflection.rb#L420-L443
def self.<(other)
other == ActiveRecord::Base || super
end
end
2 changes: 1 addition & 1 deletion bundler.d/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

gem 'bullet', '>= 6.1.0'
gem "parallel_tests"
gem 'spring', '>= 1.0', '< 3'
gem 'spring', '~> 4.0'
gem 'benchmark-ips', '>= 2.8.2'
gem 'foreman'
gem('bootsnap', :require => false)
Expand Down
12 changes: 8 additions & 4 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class Foreman::Consoletie < Rails::Railtie

module Foreman
class Application < Rails::Application
config.load_defaults '6.1'
config.load_defaults '7.0'

# Rails 5.0 changed this to true, but a lot of code depends on this
config.active_record.belongs_to_required_by_default = false
Expand All @@ -99,11 +99,13 @@ class Application < Rails::Application
config.active_support.use_authenticated_message_encryption = false
config.action_dispatch.use_authenticated_cookie_encryption = false

# Rails 6.0 changed this to :zeitwerk
config.autoloader = :zeitwerk

# Rails 6.1 changed this to true, but apparently our codebase is not ready for bidirectional associations
config.active_record.has_many_inversing = false

# Rails 7.0 changed this to true
config.active_record.verify_foreign_keys_for_fixtures = false
config.active_record.automatic_scope_inversing = false

# Setup additional routes by loading all routes file from routes directory
Dir["#{Rails.root}/config/routes/**/*.rb"].each do |route_file|
config.paths['config/routes.rb'] << route_file
Expand All @@ -113,6 +115,8 @@ class Application < Rails::Application
# Application configuration should go into files in config/initializers
# -- all .rb files in that directory are automatically loaded.

# Recommended by Rails: https://guides.rubyonrails.org/v7.0/configuring.html#config-add-autoload-paths-to-load-path
config.add_autoload_paths_to_load_path = false
# Autoloading
config.autoload_paths += %W(#{config.root}/app/models/auth_sources)
config.autoload_paths += %W(#{config.root}/app/models/compute_resources)
Expand Down
2 changes: 1 addition & 1 deletion config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# test suite. You never need to work with it otherwise. Remember that
# your test database is "scratch space" for the test suite and is wiped
# and recreated between test runs. Don't rely on the data there!
config.cache_classes = true
config.cache_classes = false

config.eager_load = true

Expand Down
2 changes: 1 addition & 1 deletion config/initializers/core_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def self.per_page
# Foreman.in_rake? prevents the failure of db:migrate for postgresql
# don't query settings table if in rake
return 20 unless Foreman.settings.ready?
Setting[:entries_per_page] rescue 20
Foreman.settings[:entries_per_page] rescue 20
end

def self.audited(*args)
Expand Down
2 changes: 1 addition & 1 deletion test/factories/audit.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
FactoryBot.define do
factory :audit do
sequence(:version) { |n| n.to_s }
auditable_type { "test" }
auditable_type { ApplicationRecord }
action { "update" }

trait :with_diff do
Expand Down
1 change: 0 additions & 1 deletion test/unit/foreman/access_control_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'test_helper'
require 'foreman/access_control'

class AccessControlTest < ActiveSupport::TestCase
test '#path_hash_to_string reads controller and action' do
Expand Down
1 change: 0 additions & 1 deletion test/unit/foreman/cast_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'test_helper'
require 'foreman/cast'

class CastTest < ActiveSupport::TestCase
include Foreman::Cast
Expand Down
2 changes: 1 addition & 1 deletion test/unit/has_many_common_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class HasManyCommonTest < ActiveSupport::TestCase

## Test name methods resolve for Plugin AR objects
class ::FakePlugin; end
class ::FakePlugin::FakeModel; end
class ::FakePlugin::FakeModel < ApplicationRecord; end
Comment on lines 94 to +95
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this even be

module FakePlugin
  class FakeModel < ApplicationRecord
  end
end


test "should return plugin associations" do
Host::Managed.class_eval do
Expand Down
2 changes: 0 additions & 2 deletions test/unit/shared/access_permissions_test_base.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
require 'foreman/access_control'

module AccessPermissionsTestBase
extend ActiveSupport::Concern

Expand Down
Loading