Skip to content

Commit

Permalink
Fixes #37451 - Support Zeitwerk loader
Browse files Browse the repository at this point in the history
  • Loading branch information
ofedoren authored and adamruzicka committed Sep 11, 2024
1 parent bf79426 commit 87fa094
Show file tree
Hide file tree
Showing 14 changed files with 19 additions and 25 deletions.
2 changes: 1 addition & 1 deletion app/controllers/ansible_roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def default_order
end

def create_importer
@importer = ForemanAnsible::UiRolesImporter.new(@proxy)
@importer = ForemanAnsible::UIRolesImporter.new(@proxy)
@variables_importer = ForemanAnsible::VariablesImporter.new(@proxy)
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/ansible_variables_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def import_new_roles

def create_importer
@importer = ForemanAnsible::VariablesImporter.new(@proxy)
@importer_roles = ForemanAnsible::UiRolesImporter.new(@proxy)
@importer_roles = ForemanAnsible::UIRolesImporter.new(@proxy)
end

def find_required_proxy
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/ansible_roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def find_proxy
# rubocop:enable Layout/DotPosition

def create_importer
@roles_importer = ForemanAnsible::UiRolesImporter.new(@proxy)
@roles_importer = ForemanAnsible::UIRolesImporter.new(@proxy)
@variables_importer = ForemanAnsible::VariablesImporter.new(@proxy)
@importer = ForemanAnsible::ApiRolesImporter.new(@proxy)
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/ui_ansible_roles_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class UiAnsibleRolesController < ::Api::V2::BaseController
class UIAnsibleRolesController < ::Api::V2::BaseController
def resource_name(resource = 'AnsibleRole')
super resource
end
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion app/jobs/sync_roles_and_variables.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class SyncRolesAndVariables < ::ApplicationJob
queue_as :default

def perform(changed, proxy)
roles_importer = ForemanAnsible::UiRolesImporter.new(proxy)
roles_importer = ForemanAnsible::UIRolesImporter.new(proxy)
variables_importer = ForemanAnsible::VariablesImporter.new(proxy)
roles_importer.finish_import(changed)
variables_importer.import_variables_roles(changed) if changed['new'] || changed['old']
Expand Down
2 changes: 1 addition & 1 deletion app/services/foreman_ansible/api_roles_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class ApiRolesImporter < RolesImporter
include ::ForemanAnsible::AnsibleRolesDataPreparations

def import!(role_names)
@roles_importer = ForemanAnsible::UiRolesImporter.new(@ansible_proxy)
@roles_importer = ForemanAnsible::UIRolesImporter.new(@ansible_proxy)
@variables_importer = ForemanAnsible::VariablesImporter.new(@ansible_proxy)
params = { 'changed' => {} }
roles = prepare_ansible_import_rows(@roles_importer.import!, @variables_importer, false)
Expand Down
2 changes: 1 addition & 1 deletion app/services/foreman_ansible/ui_roles_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module ForemanAnsible
# imports ansible roles through UI
class UiRolesImporter < RolesImporter
class UIRolesImporter < RolesImporter
def import!
import_role_names
end
Expand Down
4 changes: 2 additions & 2 deletions foreman_ansible.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ Gem::Specification.new do |s|

s.add_dependency 'acts_as_list', '~> 1.0.3'
s.add_dependency 'deface', '< 2.0'
s.add_dependency 'foreman_remote_execution', '>= 9.0', '< 14'
s.add_dependency 'foreman-tasks', '>= 7.0', '< 10'
s.add_dependency 'foreman_remote_execution', '>= 14.0', '< 15'
s.add_dependency 'foreman-tasks', '>= 10.0', '< 11'

s.add_development_dependency 'theforeman-rubocop', '~> 0.1.0'
end
14 changes: 4 additions & 10 deletions lib/foreman_ansible/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,10 @@ module ForemanAnsible
class Engine < ::Rails::Engine
engine_name 'foreman_ansible'

config.autoload_paths += Dir["#{config.root}/app/controllers/concerns"]
config.autoload_paths += Dir["#{config.root}/app/models"]
config.autoload_paths += Dir["#{config.root}/app/helpers"]
config.autoload_paths += Dir["#{config.root}/app/overrides"]
config.autoload_paths += Dir["#{config.root}/app/services"]
config.autoload_paths += Dir["#{config.root}/app/views"]
config.autoload_paths += Dir["#{config.root}/app/lib"]

initializer 'foreman_ansible.register_plugin', :before => :finisher_hook do
require 'foreman_ansible/register'
initializer 'foreman_ansible.register_plugin', :before => :finisher_hook do |app|
app.reloader.to_prepare do
require 'foreman_ansible/register'

This comment has been minimized.

Copy link
@nadjaheitmann

nadjaheitmann Oct 7, 2024

Does it work like this? The guide recommends to remove require calls. https://guides.rubyonrails.org/classic_to_zeitwerk_howto.html#delete-any-require-calls

This comment has been minimized.

Copy link
@ofedoren

ofedoren Oct 7, 2024

Author Member

lib/foreman_ansible/register.rb is not managed by Zeitwerk by default, so anything in lib/ should be loaded explicitly, thus, without this require statement, the file is not being loaded at all.

This comment has been minimized.

Copy link
@nadjaheitmann

nadjaheitmann Oct 7, 2024

Ah, thanks for the explanation

end
end

initializer('foreman_ansible.require_dynflow',
Expand Down
2 changes: 1 addition & 1 deletion lib/foreman_ansible/register.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

Foreman::Plugin.register :foreman_ansible do
requires_foreman '>= 3.10.0'
requires_foreman '>= 3.13.0'
register_gettext

settings do
Expand Down
2 changes: 1 addition & 1 deletion test/functional/ansible_variables_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class AnsibleVariablesControllerTest < ActionController::TestCase
test 'there are no problems when the import hash is empty' do
ForemanAnsible::VariablesImporter.any_instance.
expects(:import_variable_names).returns({})
ForemanAnsible::UiRolesImporter.any_instance.
ForemanAnsible::UIRolesImporter.any_instance.
expects(:import_role_names).returns({})

get :import,
Expand Down
2 changes: 1 addition & 1 deletion test/functional/ui_ansible_roles_controller_test.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require 'test_plugin_helper'

class UiAnsibleRolesControllerTest < ActionController::TestCase
class UIAnsibleRolesControllerTest < ActionController::TestCase
setup do
@role = FactoryBot.create(:ansible_role)
end
Expand Down
6 changes: 3 additions & 3 deletions test/unit/services/ui_roles_importer_test.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# frozen_string_literal: true

require 'test_plugin_helper'
# unit tests for UiRolesImporter
class UiRolesImporterTest < ActiveSupport::TestCase
# unit tests for UIRolesImporter
class UIRolesImporterTest < ActiveSupport::TestCase
setup do
changed_roles
@importer = ForemanAnsible::UiRolesImporter.new
@importer = ForemanAnsible::UIRolesImporter.new
end

test 'should create new role' do
Expand Down

0 comments on commit 87fa094

Please sign in to comment.