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 #38179 - Remove duplicates for multiCV hosts & activation keys #11308

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

pavanshekar
Copy link
Contributor

What are the changes introduced in this pull request?

Addresses the issue of duplicate relations for hosts and activation keys in multi-content-view (multiCV) scenarios. Specifically, it ensures that duplicate entries are removed for both hosts and activation keys when retrieving content views, thus improving data accuracy and efficiency.

Considerations taken when implementing this change?

Careful consideration was given to the fact that the content views have many-to-many relationships with hosts and activation keys through intermediate associations. This required applying uniqueness filtering without causing errors, such as the composite error related to ActiveRecord associations.

What are the testing steps for this pull request?

  1. Have a content view associated with multiCV hosts and/or multiCV activation keys
  2. The multiple CVEs on the hosts/AKs must be the same CV / different LCE

And then...

  1. Go to Content -> Lifecycle -> Content Views
  2. Expand the CV details
  3. The correct count should be displayed for Hosts and Activation Keys
  4. The content views API response 'hosts' and 'activation_keys' nodes does not list duplicate hosts/AKs

@jturel
Copy link
Member

jturel commented Feb 13, 2025

Would it be appropriate to use distinct in the default scope? https://guides.rubyonrails.org/association_basics.html#distinct

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Would be great to have unit tests for this too :)

@pavanshekar
Copy link
Contributor Author

Ack...I have added the unit tests

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Works as advertised, and thanks for adding the test!

ACK 👍

@jeremylenz jeremylenz merged commit 3bad7ac into Katello:master Feb 19, 2025
18 of 19 checks passed
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