Onboarding fleet integration

[angorayc]

Summary

Please use this branch to see the complete version: elastic#193131

Fleet changes: elastic#194028

It shows 9 cards without the search box on the recommended tab; and shows all the cards and a search box on other tabs:

Screen.Recording.2024-09-24.at.18.36.20.mov

When clicking on back button, cancellation button, and save button, it should go back to the security onboarding hub:

Screen.Recording.2024-09-24.at.17.48.50.mov

Screen.Recording.2024-09-30.at.15.49.12.mov

[semd]

I think that what we want here is style={{ paddingTop: 0 }} instead of removing paddingSize="l" the rest of the paddings are fine.

[semd]

NIT: we could extract:
const fleetAgentLinkProps = { appId: FLEET_APP_ID, path: ADD_AGENT_PATH };
outside the component

[semd]

This requires a lot of custom logic to load a hook from Fleet. What if we create a HOC to do that, so we can clean this component from that hook lazy loading logic, this way we can also reuse it if we ever need to do this somewhere else.
We can create a new file at x-pack/plugins/security_solution/public/common/components/with_lazy_hook/index.tsx with:

export const withLazyHook = <D extends {}, P extends {}>(
  Component: React.ComponentType<PropsWithChildren<D & P>>,
  moduleImport: () => Promise<P>,
  fallback?: React.ReactNode
) => {
  return React.memo(function WithLazy(props: D) {
    const [lazyModuleProp, setLazyModuleProp] = useState<P>();

    useEffect(() => {
      moduleImport().then((module) => {
        setLazyModuleProp(() => module);
      });
    }, []);

    return lazyModuleProp ? <Component {...lazyModuleProp} {...props} /> : <>{fallback}</> ?? null;
  });
};

Then we could just do:

export const AvailablePackages = withLazyHook(
  PackageListGrid,
  () => import('@kbn/fleet-plugin/public').then((module) => module.AvailablePackagesHook()),
  <EuiSkeletonText
    data-test-subj="loadingPackages"
    isLoading={true}
    lines={LOADING_SKELETON_HEIGHT}
  />
);

But I don't think we need to create a file for that, we can just place it next to the component definition and export the wrapped component.
We don't actually need error catching for lazy loading, we never do that, we assume all our libraries exist if the bundle compiles.

[semd]

I like the idea of the INTEGRATION_TABS config, it could be in its own file if you want

[semd]

It would be good to create an enum for the tab IDs?

[semd]

Can we name this file constants.ts?

[semd]

Can we put this hook in its own file use_integration_card_list.ts along with all the related logic? Including the getFilteredCards function that currently is in the utils file, but is only used by this hook.

[semd]

Do we really need to extract this hook? We know the tab will always exist. Assuming we create the IntegrationTabId enum, we can map all tabs in the constants file with:

export const INTEGRATION_TABS_BY_ID = Object.fromEntries(
  INTEGRATION_TABS.map((tab) => [tab.id, tab])
) as Record<IntegrationTabId, Tab>;

And then we can do this in-place:

const selectedTab = useMemo(
    () => INTEGRATION_TABS_BY_ID[selectedTabId],
    [selectedTabId]
  );

So selectedTab can't be undefined.

[semd]

This can be undefined this casting to number is error-prone.

[semd]

Can we refactor this a bit? We should first check if checkCompleteMetadata is defined, while it's undefined it means the check is running. We can check if it's finished with a type guard, so it's also cast automatically.

It would also be good to move the callouts logic outside of this component in a IntegrationCardTopCallout (since we only show one at a time).

We can rename the AvailablePackages component to something easier to understand like IntegrationsCardGridTabs. I know a "package" is another name for "integration" in Fleet, but it will make it easier for future developers if we stick to only one of these names across the code of this card.

// put this type in a new "cards/integrations/types.ts"
interface IntegrationCardMetadata {
  integrationsInstalled: number;
  agentStillRequired: boolean;
}

const isCheckCompleteMetadata = (metadata?: unknown): metadata is IntegrationCardMetadata => {
  return metadata !== undefined;
};

export const IntegrationsCard: OnboardingCardComponent = ({
  checkCompleteMetadata, // this is undefined before the first checkComplete call finishes
}) => {
  if (!isCheckCompleteMetadata(checkCompleteMetadata)) {
    return <CenteredLoadingSpinner />;
  }
  const { integrationsInstalled, agentStillRequired } = checkCompleteMetadata;

  return (
    <OnboardingCardContentPanel>
      <IntegrationCardTopCallout
        agentRequired={agentStillRequired}
        integrationsInstalled={integrationsInstalled}
      />
      <IntegrationsCardGridTabs />
    </OnboardingCardContentPanel>
  );
};

[semd]

These names are a bit odd, can we use more conventional names?

Suggested change

	integrationsInstalled: 0,
	agentStillRequired: false,
	installedIntegrationsCount: 0,
	isAgentRequired: false,

[semd]

Why do we need 2 states for the selectedTabId?

[semd]

Suggested change

	export const PackageInstalledCallout = React.memo(
	export const InstalledIntegrationsCallout = React.memo(

[semd]

Can we extract the second callout in its own component like the AgentRequiredCallout?

[semd]

Since we have a lot of callouts, it would be good to put all of them in a /cards/integrations/callouts directory, to keep things a bit more organized

[semd]

What's the difference between APP_INTEGRATIONS_PATH and integrationRootUrl?

[semd]

This utils.ts is not necessary, nothing here is ever reused. Can we put everything in its correct place?

[semd]

Why do we assume that absolute URLs need to open in a new tab? Just curiosity

[semd]

A user may store the search term in a specific tab and when the page is reloaded be used in a different tab, which may no longer make sense. Is this a requirement?