Skip to content

Commit

Permalink
Fixes #37109 - allow to use eager loading on latest_version_object
Browse files Browse the repository at this point in the history
  • Loading branch information
sbernhard committed Feb 7, 2024
1 parent 3dcae86 commit 8599f0e
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 4 deletions.
13 changes: 9 additions & 4 deletions app/models/katello/content_view.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions app/models/katello/content_view_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions test/models/content_view_component_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -229,25 +229,29 @@ 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)
assert ContentViewComponent.create(:composite_content_view => @composite,
: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)
assert ContentViewComponent.create(:composite_content_view => @composite,
: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
Expand Down
6 changes: 6 additions & 0 deletions test/models/content_view_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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')
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit 8599f0e

Please sign in to comment.