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 #38112 - Order LCEs by LCE path in Content View GUI and Hammer #11267

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pavanshekar
Copy link
Contributor

What are the changes introduced in this pull request?

Updated the ContentView and ContentViewVersion models to order LCEs based on their promotion paths in both the GUI Content View page and the Hammer CLI. The API response was updated to reflect the new ordering, providing consistency across interfaces.

Considerations taken when implementing this change?

Optimized the changes to minimize performance impact while maintaining backward compatibility to ensure existing functionality remains unaffected. Ensured consistency across the GUI and Hammer CLI with thorough testing to validate correct behavior. Designed the implementation to be modular and maintainable for future updates.

What are the testing steps for this pull request?

  1. Publish and promote a content view, after selecting multiple LCE
  2. Go to Content -> Lifecycle -> Content Views and verify that the content view GUI displays the LCEs in the correct order as per the LCE path
  3. Use the Hammer CLI command (hammer content-view version list --content-view-id <content_view_id>) to verify that the LCEs are listed in the correct order as per the LCE path

Comment on lines 112 to 114
content_view_environments = organization.readable_promotion_paths.flatten
content_view_environments.insert(0, organization.library)
content_view_environments.intersection(environments)
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these KTEnvironments and not Katello::ContentViewEnvironments?

Copy link
Member

Choose a reason for hiding this comment

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

Renaming the variable will help here..The content_view_environments should be something like organization_readable_environments

@@ -49,7 +49,7 @@ end
extends 'katello/api/v2/common/timestamps'

version = @object || @resource
child :environments => :environments do
child :sorted_content_view_environments => :environments do
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these are content view environments either..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have renamed at both the places with the name suggested by @sjha4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants