From 8599f0eb02f04ede8a9a5fcb3ebe489e33a1f0b2 Mon Sep 17 00:00:00 2001 From: Bernhard Suttner Date: Fri, 26 Jan 2024 15:43:04 +0100 Subject: [PATCH] Fixes #37109 - allow to use eager loading on latest_version_object --- app/models/katello/content_view.rb | 13 +++++++++---- app/models/katello/content_view_version.rb | 2 ++ test/models/content_view_component_test.rb | 4 ++++ test/models/content_view_test.rb | 6 ++++++ 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/app/models/katello/content_view.rb b/app/models/katello/content_view.rb index fa2356bd917..a5d586c3f78 100644 --- a/app/models/katello/content_view.rb +++ b/app/models/katello/content_view.rb @@ -18,6 +18,7 @@ class ContentView < Katello::Model has_many :content_view_versions, :class_name => "Katello::ContentViewVersion", :dependent => :destroy alias_method :versions, :content_view_versions + has_one :latest_version_object, -> { latest }, :class_name => "Katello::ContentViewVersion", :dependent => :destroy # Note the difference between content_view_components and component_composites both refer to # ContentViewComponent but mean different things. # content_view_components -> Topdown, given I am a composite CV get the associated components belonging to me @@ -341,10 +342,6 @@ def latest_version_env latest_version_object.try(:environments) || [] end - def latest_version_object - self.versions.order('major DESC').order('minor DESC').first - end - def last_task last_task_id = history.order(:created_at)&.last&.task_id last_task_id ? ForemanTasks::Task.find_by(id: last_task_id) : nil @@ -611,6 +608,14 @@ def create_new_version(major = next_version, minor = 0, components = self.compon :components => components ) + # TODO: If a controller creates a new version and then uses latest_version_object, the old data is displayed. + # To prevent this, a 'reload' would currently be necessary, but this is not very performant. + # However, this is currently not a problem because after your create_new_version there is no immediate + # access to latest_version_object, but the ContentView object is first completely reloaded. + # + # In Rails 7.1, individual connections can be reloaded: + # https://www.shakacode.com/blog/rails-7-1-allows-resetting-singular-associations/ + update(:next_version => major.to_i + 1) unless major.to_i < next_version version end diff --git a/app/models/katello/content_view_version.rb b/app/models/katello/content_view_version.rb index 4929a27baa4..6f93ff50453 100644 --- a/app/models/katello/content_view_version.rb +++ b/app/models/katello/content_view_version.rb @@ -66,6 +66,8 @@ class ContentViewVersion < Katello::Model joins(:repositories).includes(:content_view).merge(repositories).distinct end + scope :latest, -> { order('major DESC', 'minor DESC').limit(1) } + scoped_search :on => :content_view_id, :only_explicit => true, :validator => ScopedSearch::Validators::INTEGER scoped_search :on => :major, :rename => :version, :complete_value => true, :ext_method => :find_by_version serialize :content_counts diff --git a/test/models/content_view_component_test.rb b/test/models/content_view_component_test.rb index 9f6d31f6ed5..c170d4a8682 100644 --- a/test/models/content_view_component_test.rb +++ b/test/models/content_view_component_test.rb @@ -229,6 +229,7 @@ def test_needs_publish :action => "publish") assert_equal @composite.needs_publish?, true @composite.create_new_version + @composite.reload cv_history.update!(katello_content_view_version_id: @composite.latest_version_object.id) assert_equal @composite.reload.needs_publish?, false version1 = create(:katello_content_view_version, :content_view => view1) @@ -236,6 +237,7 @@ def test_needs_publish :content_view_version => version1, :latest => false) assert_equal @composite.reload.needs_publish?, true @composite.create_new_version + @composite.reload cv_history.update!(katello_content_view_version_id: @composite.latest_version_object.id) assert_equal @composite.reload.needs_publish?, false create(:katello_content_view_version, :content_view => view2) @@ -243,11 +245,13 @@ def test_needs_publish :content_view => view2, :latest => true) assert_equal @composite.reload.needs_publish?, true @composite.create_new_version + @composite.reload cv_history.update!(katello_content_view_version_id: @composite.latest_version_object.id) assert_equal @composite.reload.needs_publish?, false create(:katello_content_view_version, :content_view => view2, :major => "1000") assert_equal @composite.reload.needs_publish?, true @composite.create_new_version + @composite.reload cv_history.update!(katello_content_view_version_id: @composite.latest_version_object.id) assert_equal @composite.reload.needs_publish?, false end diff --git a/test/models/content_view_test.rb b/test/models/content_view_test.rb index 8570d50f83d..80af112f852 100644 --- a/test/models/content_view_test.rb +++ b/test/models/content_view_test.rb @@ -664,6 +664,7 @@ def test_audits_cleaned_cv_needs_publish content_view = FactoryBot.build(:katello_content_view, :name => "New CV cleaned audits") content_view.save! content_view.create_new_version + content_view.reload task = ForemanTasks::Task.create!(:label => 'Actions::Katello::ContentView::Publish', :state => 'stopped', :type => 'ForemanTasks::Task::DynflowTask', :result => 'success') ::Katello::ContentViewHistory.create!(:katello_content_view_version_id => content_view.latest_version_object.id, @@ -678,6 +679,7 @@ def test_audits_never_created_cv_needs_publish content_view = FactoryBot.build(:katello_content_view, :name => "New CV applied filters nil") content_view.save! content_view.create_new_version + content_view.reload task = ForemanTasks::Task.create!(:label => 'Actions::Katello::ContentView::Publish', :state => 'stopped', :type => 'ForemanTasks::Task::DynflowTask', :result => 'success') ::Katello::ContentViewHistory.create!(:katello_content_view_version_id => content_view.latest_version_object.id, @@ -695,6 +697,7 @@ def test_published_cv_needs_publish_on_dependency_solving_update content_view.save! assert content_view.needs_publish? #New CV needs publish content_view.create_new_version + content_view.reload content_view.latest_version_object.update!(applied_filters: {"dependency_solving": false}) task = ForemanTasks::Task.create!(:label => 'Actions::Katello::ContentView::Publish', :state => 'stopped', :type => 'ForemanTasks::Task::DynflowTask', :result => 'success') @@ -721,6 +724,7 @@ def test_published_cv_needs_publish_on_repositories_update content_view.save! assert content_view.needs_publish? #New CV needs publish content_view.create_new_version + content_view.reload task = ForemanTasks::Task.create!(:label => 'Actions::Katello::ContentView::Publish', :state => 'stopped', :type => 'ForemanTasks::Task::DynflowTask', :result => 'success') ::Katello::ContentViewHistory.create!(:katello_content_view_version_id => content_view.latest_version_object.id, @@ -745,6 +749,7 @@ def test_published_cv_needs_publish_on_repository_publication_update content_view.save! assert content_view.needs_publish? #New CV needs publish content_view.create_new_version + content_view.reload task = ForemanTasks::Task.create!(:label => 'Actions::Katello::ContentView::Publish', :state => 'stopped', :type => 'ForemanTasks::Task::DynflowTask', :result => 'success') ::Katello::ContentViewHistory.create!(:katello_content_view_version_id => content_view.latest_version_object.id, @@ -761,6 +766,7 @@ def test_nil_on_failed_cv_publish_needs_publish content_view.save! assert content_view.needs_publish? #no version needs publish content_view.create_new_version + content_view.reload task = ForemanTasks::Task.create!(:label => 'Actions::Katello::ContentView::Publish', :state => 'stopped', :type => 'ForemanTasks::Task::DynflowTask', :result => 'failed') ::Katello::ContentViewHistory.create!(:katello_content_view_version_id => content_view.latest_version_object.id,