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 #36610 - Add system purpose to activation key details #10697

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Aug 15, 2023

What are the changes introduced in this pull request?

image

Adds the System Purpose card to the new activation key details page, as per the mockup. This means the System Purpose card now works on both this page and the new host details page.

Also added the page section which adds the grey background.

Considerations taken when implementing this change?

The mockup has a slightly different layout on the AK details page vs. the host details page. On AK details, it's a 2-column layout, while it's 1 column on host details. The same component handles both situations, using a few ternaries and such.

Patternfly has some super-weird CSS that misaligns the card expansion carat. I had to fix it.

What are the testing steps for this pull request?

Turn on the 'Show Experimental Labs' setting and go look at your activation keys.

Make sure viewing and editing of System Purpose attributes works as expected on both pages.

@theforeman-bot
Copy link

Issues: #36610

@lfu
Copy link
Member

lfu commented Sep 27, 2023

Tests worked well.

@jeremylenz
Copy link
Member Author

I realized that available release versions are not getting properly retrieved. will update soon

@jeremylenz
Copy link
Member Author

@lfu Addressed comments and fixed release versions, also added tests for it.

@lfu
Copy link
Member

lfu commented Sep 28, 2023

Seems this API call always returns the same result no mattter the host's release version:
/api/v2/hosts/<ID>/subscriptions/available_release_versions
which may be out of the scope of this PR.

Other than that, it worked well.

@jeremylenz
Copy link
Member Author

Seems this API call always returns the same result no mattter the host's release version:

# host_subscriptions_controller.rb:

def available_release_versions
      releases = @host.content_facet.try(:available_releases) || []

# content_facet.rb:

def available_releases
        self.content_view_environments.flat_map do |cve|
          cve.content_view.version(cve.lifecycle_environment).available_releases
        end
      end

# content_view_version.rb

  def available_releases
      Katello::RootRepository.where(:id => self.repositories.select(:root_id)).pluck(:minor).compact.uniq.sort
    end

I think because it's based on what RootRepositories you have, this is expected.

@jeremylenz
Copy link
Member Author

[test katello]

Copy link
Member

@lfu lfu left a comment

Choose a reason for hiding this comment

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

APJ

@jeremylenz jeremylenz merged commit 4867c8a into Katello:master Sep 29, 2023
5 checks passed
@jeremylenz
Copy link
Member Author

Thanks @lfu and @trev-allison03! Merged

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