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 #34320 - Hide Katello tabs & cards on new host detail page when host is unregistered #9942

Merged
merged 2 commits into from Mar 9, 2022

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Feb 11, 2022

What are the changes introduced in this pull request?

We need to allow Katello to hide its tabs and cards from the new host detail page.

  • Adds new helper functions hostIsNotRegistered and hostIsRegistered

  • Check if host is registered and if not, hide Errata and Content View overview cards

  • Also hide Content, Traces, and Repository sets tabs for non-registered hosts. The way it works is as follows:

    • Adds a new slot metadata attribute, hideTab, that is sent to Foreman. This allows us to pass a function to Foreman which Foreman will then call with the host details when setting up the page.
    • If Refs #34442 - Allow plugins to hide tabs on new host detail page theforeman/foreman#9104 is applied, and hideTab returns true for that tab, Foreman will hide the tab from the host details page.
    • I'm using hostIsNotRegistered as the hideTab function here, but other functions could be used for other slots in the future.
  • Also, recalculates errata counts on the host when unregistering to ensure they're properly cleared out.

Considerations taken when implementing this change?

Hiding cards I can do from Katello, but I found that changing Foreman code is the only clean way to hide an entire tab.
I found that passing the hideTab function to Foreman is a good way to keep Katello code in Katello and Foreman code in Foreman. This way Katello can be the one that cares about why we're hiding the tab, and Foreman doesn't have to touch such ugliness as subscription facets 😱.

What are the testing steps for this pull request?

  • Apply this Katello change
  • Register a host and get some installable errata on it
  • Find its content_facet in Rails console
  • Unregister the host
  • View the content facet's attributes and verify that the errata counts are cleared out:
installable_security_errata_count: 0,
installable_enhancement_errata_count: 0,
installable_bugfix_errata_count: 0,

@theforeman-bot
Copy link

Issues: #34320

@jeremylenz jeremylenz changed the title 34320 unregistered host Fixes #34320 - Hide Katello tabs & cards on new host detail page when host is unregistered Feb 11, 2022
@jeremylenz jeremylenz force-pushed the 34320-unregistered-host branch 4 times, most recently from a5c11b7 to 91527c8 Compare February 14, 2022 15:24
Copy link
Contributor

@amirfefer amirfefer left a comment

Choose a reason for hiding this comment

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

Thanks, @jeremylenz! 👍

Tabs are registered with slot&fill which uses a registry for registering new fills, how about leveraging this runtime mechanism for unregistering a tab (fill)?

We use this helper to register a core's tab in runtime -
https://github.com/theforeman/foreman/blob/develop/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/index.js#L7

We can leverage the slot's registry and create a helper for unregistering a specific tab directly from the registry.

@jeremylenz
Copy link
Member Author

@amirfefer I did try unregistering, but ran into some problems which I can't remember now. I think it was something to do with the tab unregistering itself. I guess I will look into it again.. 🤔

@jeremylenz
Copy link
Member Author

In TracesTab I added

  if (hostIsNotRegistered({ hostDetails })) {
    SlotsRegistry.remove('host-details-page-tabs', 'Traces');
    return null;
  }

But this didn't work because the tab itself was still there; it just didn't render any content inside it. Also, after a few seconds I got this console error

connectAdvanced.js:296 Uncaught TypeError: Cannot read properties of undefined (reading 'component')
    at selectFillsComponents (SlotSelectors.js:34:26)

and this screen:
slot_remove_errors

@jeremylenz
Copy link
Member Author

I also tried to use the Redux action

if (hostIsNotRegistered({ hostDetails })) {
    // SlotsRegistry.remove('host-details-page-tabs', 'Traces');
    dispatch(unregisterFillComponent('host-details-page-tabs', 'Traces'));
    return null;
  }

and I got

Warning: Cannot update a component (`ConnectFunction`) while rendering a different component (`TracesTab`). To locate the bad setState() call inside `TracesTab`, follow the stack trace as described in https://fb.me/setstate-in-render
    in TracesTab
    in Slot (created by ConnectFunction)
    in ConnectFunction (created by Context.Consumer)
    in Route (created by TabRouter)
    in Switch (created by TabRouter)
    in Router (created by HashRouter)
    in HashRouter (created by TabRouter)
    in TabRouter (created by HostDetails)
    in section (created by PageSection)
    in PageSection (created by HostDetails)
    in HostDetails (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 div (created by LayoutContainer)
    in LayoutContainer (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)))

@jeremylenz
Copy link
Member Author

I also tried to unregister the tab from ErrataOverviewCard:

Warning: Cannot update a component (`ConnectFunction`) while rendering a different component (`TracesTab`). To locate the bad setState() call inside `TracesTab`, follow the stack trace as described in https://fb.me/setstate-in-render
    in TracesTab
    in Slot (created by ConnectFunction)
    in ConnectFunction (created by Context.Consumer)
    in Route (created by TabRouter)
    in Switch (created by TabRouter)
    in Router (created by HashRouter)
    in HashRouter (created by TabRouter)
    in TabRouter (created by HostDetails)
    in section (created by PageSection)
    in PageSection (created by HostDetails)
    in HostDetails (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 div (created by LayoutContainer)
    in LayoutContainer (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)))

I think the problem is that there's simply no good place to put this code in Katello. That's why I think my solution of passing the function to Foreman is pretty clean and the best way to go.

@amirfefer thoughts?

@jeremylenz
Copy link
Member Author

rebased

@parthaa
Copy link
Contributor

parthaa commented Feb 28, 2022

Works well for me only oddity I saw was applicable_rpm_count: 1 in the content facet. Hiding stuff worked well.

#<Katello::Host::ContentFacet:0x00000000182b5850
 id: 1,
 host_id: 1,
 uuid: nil,
 content_view_id: 1,
 lifecycle_environment_id: 1,
 kickstart_repository_id: nil,
 content_source_id: nil,
 installable_security_errata_count: 0,
 installable_enhancement_errata_count: 0,
 installable_bugfix_errata_count: 0,
 applicable_rpm_count: 1,
 upgradable_rpm_count: 0,
 applicable_module_stream_count: 0,
 upgradable_module_stream_count: 0,
 applicable_deb_count: 0,
 upgradable_deb_count: 0>

@parthaa
Copy link
Contributor

parthaa commented Feb 28, 2022

I think the problem is that there's simply no good place to put this code in Katello. That's why I think my solution of passing the function to Foreman is pretty clean and the best way to go.

Unregistering slot/fills seems complicated, require deeper changes in foreman and has potential for breaking other things. You approach seems much less invasive, cleaner and easier to maintain. APJ :)

Copy link
Contributor

@parthaa parthaa left a comment

Choose a reason for hiding this comment

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

Works well for me. APJ

@jeremylenz
Copy link
Member Author

[test katello]

Refs #34320 - add & fix tests
@jeremylenz
Copy link
Member Author

rebased

@jeremylenz jeremylenz merged commit d5059d5 into Katello:master Mar 9, 2022
ianballou pushed a commit to ianballou/katello that referenced this pull request Mar 15, 2022
… host is unregistered (Katello#9942)

* Fixes #34320 - update errata applicability when unregistering
* Refs #34320 - Hide Katello tabs
Refs #34320 - add & fix tests
ianballou pushed a commit to ianballou/katello that referenced this pull request Mar 17, 2022
… host is unregistered (Katello#9942)

* Fixes #34320 - update errata applicability when unregistering
* Refs #34320 - Hide Katello tabs
Refs #34320 - add & fix tests
ianballou pushed a commit that referenced this pull request Mar 18, 2022
… host is unregistered (#9942)

* Fixes #34320 - update errata applicability when unregistering
* Refs #34320 - Hide Katello tabs
Refs #34320 - add & fix tests
sjha4 pushed a commit to sjha4/katello that referenced this pull request Mar 31, 2022
… host is unregistered (Katello#9942)

* Fixes #34320 - update errata applicability when unregistering
* Refs #34320 - Hide Katello tabs
Refs #34320 - add & fix tests

(cherry picked from commit f4f19c1)
parthaa pushed a commit to parthaa/katello that referenced this pull request Apr 11, 2022
…for unregistered host

Fixes #34320 - Hide Katello tabs & cards on new host detail page when host is unregistered (Katello#9942)

* Fixes #34320 - update errata applicability when unregistering
* Refs #34320 - Hide Katello tabs
Refs #34320 - add & fix tests

(cherry picked from commit d5059d5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants