Fixes #37347 - Add Packages wizard to All Hosts page #10962
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeremylenz
force-pushed
the
37347-packages-wizard
branch
from
8863469
to
4590f1e
Compare
jeremylenz
force-pushed
the
37347-packages-wizard
branch
3 times, most recently
from
7841521
to
9f40035
Compare
jeremylenz
changed the title
jeremylenz
force-pushed
the
37347-packages-wizard
branch
from
9f40035
to
9692fd3
Compare
ianballou
reviewed
May 1, 2024
There was a problem hiding this comment.
Seeing this following when hitting Next for all options except "Upgrade packages":
TypeError: isSelected is not a function
in RowSelectTd (created by Table)
in tr (created by TrBase)
in TrBase (created by Tr)
in Tr (created by Table)
in tbody (created by TbodyBase)
in TbodyBase (created by Tbody)
in Tbody (created by Table)
in table (created by TableComposableBase)
in TableComposableBase (created by TableComposable)
in TableComposable (created by Table)
in Table (created by TableIndexPage)
in section (created by PageSection)
in PageSection (created by TableIndexPage)
in div (created by TableIndexPage)
in TableIndexPage (created by HostReview)
in HostReview (created by BulkPackagesWizard)
in div (created by WizardBody)
in div (created by WizardBody)
in WizardBody (created by WizardToggle)
in div (created by WizardToggle)
in div (created by WizardToggle)
in WizardToggle (created by WizardInternal)
in WizardInternal (created by Wizard)
in div (created by Wizard)
in WizardContextProvider (created by Wizard)
in Wizard (created by BulkPackagesWizard)
in BulkPackagesWizard (created by BulkPackagesWizardModal)
in div (created by ModalBox)
in ModalBox (created by ModalContent)
in div (created by FocusTrap)
in FocusTrap (created by ForwardRef)
in ForwardRef (created by ModalContent)
in div (created by Backdrop)
in Backdrop (created by ModalContent)
in ModalContent (created by Modal)
in Modal (created by BulkPackagesWizardModal)
in BulkPackagesWizardModal
in Slot (created by ConnectFunction)
in ConnectFunction (created by HostsIndex)
in section (created by PageSection)
in PageSection (created by TableIndexPage)
in div (created by TableIndexPage)
in TableIndexPage (created by HostsIndex)
in HostsIndex (created by Context.Consumer)
in Route (created by AppSwitcher)
in Switch (created by ForemanSwitcher)
in ForemanSwitcher (created by AppSwitcher)
in AppSwitcher (created by ReactApp)
in ErrorBoundary (created by ReactApp)
in main (created by Page)
in div (created by Page)
in Page (created by Layout)
in div (created by FlexItem)
in FlexItem (created by Layout)
in div (created by Flex)
in Flex (created by Layout)
in Layout (created by ConnectedLayout)
in ConnectedLayout (created by ReactApp)
in Router (created by ConnectedRouter)
in ConnectedRouter (created by Context.Consumer)
in ConnectedRouterWithContext (created by ConnectFunction)
in ConnectFunction (created by ReactApp)
in ApolloProvider (created by ReactApp)
in div (created by ReactApp)
in ReactApp (created by I18nProviderWrapper(ReactApp))
in IntlProvider (created by I18nProviderWrapper(ReactApp))
in I18nProviderWrapper(ReactApp) (created by StoreProvider(I18nProviderWrapper(ReactApp)))
in Provider (created by StoreProvider(I18nProviderWrapper(ReactApp)))
in StoreProvider(I18nProviderWrapper(ReactApp)) (created by DataProvider(StoreProvider(I18nProviderWrapper(ReactApp))))
in DataProvider(StoreProvider(I18nProviderWrapper(ReactApp)))
in AwaitedMount
ianballou
reviewed
May 1, 2024
oh yeah I've been meaning to fix that 😄 I think it works on the review page at least |
Do you have the Foreman PR checked out? |
That must be it :) |
ianballou
reviewed
May 1, 2024
There was a problem hiding this comment.
Packages here seem to be referred to by name alone. Users would probably like to select and see the version that they're installing or upgrading to.
Could packages be referred to by their full NEVRA? Or, perhaps the package selector could mirror what's on the host details page:
|
Looks like users can currently filter package versions by using the search bar, but I'm guessing they'd like to be able to see what versions are available. |
This bulk wizard only allows upgrading to the latest version. I can't think of a sane way to allow any granularity there, when you're dealing with multiple hosts that may have different versions installed and different versions available. |
That would be nice! But the version they're installing or upgrading to could vary from host to host, depending on that host's LCE/CV. |
|
Hit an error trying to install 4 RPMs on two hosts: (Really odd that it's bookmark related?) |
ianballou
reviewed
May 2, 2024
|
@ianballou Can you choose 'via customized' and see what you end up with for the host search query? Does it resolve to 0 or 2 hosts? |
|
|
The bookmark issue does not appear for custom REX. |
jeremylenz
force-pushed
the
37347-packages-wizard
branch
from
9692fd3
to
ffdcd25
Compare
|
@ianballou Bookmark error issue should be fixed now. |
|
Bookmark error is fixed, up next: Confirmed vim-enhanced is available. Edit: vim-enhanced was installed on both hosts, which triggered the error. I was a little confused since it made it sound like the RPM wasn't among the synced content. I wonder if the error could be improved. |
jeremylenz
force-pushed
the
37347-packages-wizard
branch
from
ffdcd25
to
04a4fea
Compare
|
I just tested:
|
jeremylenz
force-pushed
the
37347-packages-wizard
branch
from
c0ca37e
to
1233f5f
Compare
ianballou
reviewed
May 3, 2024
| hostname, search, hostSearch, versions, descriptionFormat, | ||
| } = options; | ||
| const searchQuery = hostSearch ?? `name ^ (${hostname})`; | ||
| console.log(versions) |
|
Things all seem to be working now! |
|
I must admit it took me some time to find the wizard 🙈 Also, |
|
Looks like the bookmark clipping issue only happens when your screen is pretty narrow at least. But having it work with the side scroller would be cool if possible. |
I think we may need some help here on this part like with host details page :) |
Flipped the bookmark dropdown so it's entirely within the wizard window. This should fix the issue. (Will require updating both Foreman and Katello PR branches.) |
|
/packit build |
|
Account evgeni has no write access nor is author of PR! |
|
/packit build |
|
RPMs were built successfully. \o/ |
|
Still working just fine for me! The bookmark menu is now visible at all times. |
parthaa
reviewed
May 14, 2024
parthaa
reviewed
May 14, 2024
parthaa
reviewed
May 14, 2024
jeremylenz
force-pushed
the
37347-packages-wizard
branch
from
4f8ba03
to
672771d
Compare
jeremylenz
force-pushed
the
37347-packages-wizard
branch
from
672771d
to
76096b9
Compare
jeremylenz
force-pushed
the
37347-packages-wizard
branch
from
76096b9
to
9567301
Compare
jeremylenz
commented
May 16, 2024
| @@ -25,3 +25,4 @@ jobs: | |||
| plugin: katello | |||
| postgresql_container: ghcr.io/theforeman/postgresql-evr | |||
| test_existing_database: false | |||
| foreman_version: refs/pull/10128/head | |||
There was a problem hiding this comment.
Added this to run tests with the Foreman PR. Tests are green, except that our "React Tests" check seems to ignore the Foreman version specified. However, npm run test passes locally (with the Foreman PR checked out):
Test Suites: 137 passed, 137 total
Tests: 584 passed, 584 total
Snapshots: 165 passed, 165 total
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires theforeman/foreman#10128
What are the changes introduced in this pull request?
Add the Packages Wizard to the new All Hosts page, based on designs.
There are 4 steps in the wizard:
And there are 3 flows:
This also required adding features in Foreman React components, and adding new capabilities here:
:distinctand:selectoptions toapi_controller.rbso that you can get back a list distinct by package name. I had to write some SQL fragments myself since Active Record wasn't playing nice with.count.package_names_for_job_templateif the package search turns up empty. This is to prevent the template from updating all packages when you only wanted to update one package.Considerations taken when implementing this change?
For the wizard itself, I used the "React next" implementation in PF4. I thought this was a much cleaner API because of the handy wizardContext it provides, and also thought it might be easier to migrate once we move to PF5.
For most of the wizard, I use a React Context,
BulkPackagesWizardContext, to share values between all the wizard steps and components. But the<HostReview>component should be reusable in other wizards, and using context would have made it nonreusable. So for<HostReview>I made sure not to use any context values, and instead passed all values through props.Originally I had the
useBulkSelectcalls within each separate component (BulkPackagesTableandHostReview). And I was trying to share values by adding state to the top-level context. But I quickly realized that was a bad decision. Maybe surprisingly, things got a lot cleaner and more elegant once I moved bothuseBulkSelects to the top level.What are the testing steps for this pull request?