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 #36526 - Add banners warning about katello-agent removal #10626

Merged

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Jun 21, 2023

What are the changes introduced in this pull request?

Add several banners in the web UI warning users about impending katello-agent removal.
Also, I removed the banners on the content host details / Packages and Errata tabs in favor of a single banner at the top of the page.

image
image
image
image

Considerations taken when implementing this change?

I didn't add a single banner at the top of the new host details page because that would require a Foreman code change.

There were already banners on the Content Host Bulk Actions modals for errata and packages.

Can you think of any other places where I should add a banner?

Downstream, "Katello 4.10" will be translated to "Satellite 6.15".

The PF4 banners have a close button which is nice! but the Angular ones don't, sorry!

What are the testing steps for this pull request?

In order to see these banners, the UI has to think that you have katello-agent enabled. So, you either have to actually set up katello-agent, or fake it somehow.

@theforeman-bot
Copy link

Issues: #36526

@@ -1,7 +1,7 @@
<section>
<p bst-alert="warning" >
<span translate>
Katello-agent is deprecated and will be removed in a future release.
Katello-agent is deprecated and will be removed in Katello 4.10.
Copy link
Member Author

Choose a reason for hiding this comment

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

This hard-coding was missed in the last PR, which updated the removal version in the API deprecation warnings only.

@chris1984 chris1984 self-assigned this Jun 23, 2023
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, looked around at host collections and bulk actions and I can see the warning there as well. Overall looks great :) Left 2 questions but nothing that blocks merging it.

Content hosts page - ✔️
Errata tab - ✔️
Packages tab - ✔️
Legacy ui details - ✔️
Packages installed/applicable - ✔️
Package actions - should we add it here along with the group package deprecation warning message or do you think the notice at the top is enough? - ✔️
Bulk actions packages - ✔️
Bulk actions errata - ✔️
Host collections errata - ✔️
Host collections packages - ✔️

Right now it reads Katello-agent is deprecated and will be removed in Katello 4.10. do you think we should drop the period at the end?

@jeremylenz
Copy link
Member Author

do you think we should drop the period at the end?

Nah, it's the period at the end of the sentence!

You said you left 2 questions, but I don't see them?

@chris1984
Copy link
Member

Package actions - should we add it here along with the group package deprecation warning message or do you think the notice at the top is enough?

Ah sorry it got mixed up in my review comment:

Package actions - should we add it here along with the group package deprecation warning message or do you think the notice at the top is enough?

@jeremylenz
Copy link
Member Author

Package actions - should we add it here along with the group package deprecation warning message or do you think the notice at the top is enough?

Ohh, this was the second question!

I actually removed the banner from package actions because there were two of the same banner on the screen and it looked silly.

@chris1984
Copy link
Member

Package actions - should we add it here along with the group package deprecation warning message or do you think the notice at the top is enough?

Ohh, this was the second question!

I actually removed the banner from package actions because there were two of the same banner on the screen and it looked silly.

Sounds good, then yeah looks good to me on all the areas I checked, 🚢 it

@jeremylenz jeremylenz merged commit e59f07d into Katello:master Jun 23, 2023
5 checks passed
lfu pushed a commit to lfu/katello that referenced this pull request Jul 12, 2023
lfu pushed a commit that referenced this pull request Jul 12, 2023
wbclark pushed a commit to wbclark/katello that referenced this pull request Sep 7, 2023
wbclark pushed a commit that referenced this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants