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

Refs #34527 - Add Host Collections card #9990

Merged
merged 2 commits into from Mar 3, 2022

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Feb 28, 2022

What are the changes introduced in this pull request?

  • Adds a Host collections overview card to the host details page.
  • add tests

host_collections_card

Considerations taken when implementing this change?

I used flexbox rather than a table since it's such a simple layout. It worked out pretty well.
I had to add a few attributes to the rabl for host collections.
Dropdown menu items are disabled for now - they will be enabled in upcoming PRs.

What are the testing steps for this pull request?

  • Add your host to at least 3 host collections
  • Make sure at least one of the host collections has unlimited hosts
  • Make sure at least one of the host collections does not have unlimited hosts
  • Make sure some of your host collections have descriptions
  • Also view a host that is not part of any collections - the card will be blank but the dropdown will still be accessible

@theforeman-bot
Copy link

Issues: #34527

@jeremylenz
Copy link
Member Author

Tests added, ready for review

@chris1984
Copy link
Member

Code looks good starting to test

@chris1984
Copy link
Member

Out of scope for this pr but I noticed we are changing a lot of rabl lately and we have like 1% test coverage in that area:

https://github.com/Katello/katello/tree/master/test/views/api/v2

Do you think we should starting adding tests for when we change rabl?

@jeremylenz
Copy link
Member Author

Do you think we should starting adding tests for when we change rabl?

Rabl is a view and we don't really test views too much, so I don't think it's needed in general

@chris1984 chris1984 self-assigned this Mar 3, 2022
Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

ACK,

Works great screenshot of host collections with expanded description:

https://nimb.ws/WeXi80

Screenshot of host without any host collections:

https://nimb.ws/IG6Lmg

I didn't see any console errors while clicking around on the card.

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