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

Add 'Clean up' button to Host Details page #1623

Merged
merged 16 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 120 additions & 84 deletions assets/js/components/HostDetails/HostDetails.jsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
import React, { useState, useEffect } from 'react';
import { useParams } from 'react-router-dom';
import { useSelector } from 'react-redux';
import { useSelector, useDispatch } from 'react-redux';

import { networkClient } from '@lib/network';
import { agentVersionWarning } from '@lib/agent';

import ListView from '@components/ListView';
import Table from '@components/Table';

import PageHeader from '@components/PageHeader';
import BackButton from '@components/BackButton';
import ClusterLink from '@components/ClusterLink';
import WarningBanner from '@components/Banners/WarningBanner';
import CleanUpButton from '@components/CleanUpButton';
import DeregistrationModal from '@components/DeregistrationModal';
import SuseLogo from '@static/suse_logo.svg';
import {
getInstancesOnHost,
getClusterByHost,
getHost,
} from '@state/selectors';
import { deregisterHost } from '@state/hosts';
import StatusPill from './StatusPill';
import ProviderDetails from './ProviderDetails';

Expand All @@ -36,6 +38,18 @@ function HostDetails() {
const { grafanaPublicUrl } = config;

const [exportersStatus, setExportersStatus] = useState([]);
const [cleanUpModalOpen, setCleanUpModalOpen] = useState(false);

const dispatch = useDispatch();

const openDeregistrationModal = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can use directly the setCleanUpModalOpen(true) , right?
I know that you did to be consistent with the other.
Anyway, up to you, not a big deal hehe

setCleanUpModalOpen(true);
};

const cleanUpHost = () => {
setCleanUpModalOpen(false);
dispatch(deregisterHost(host));
Copy link
Contributor

Choose a reason for hiding this comment

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

We should get the host as argument in the function.
If we use the global variable, we have the risk of having strange things.
For example, if the function was used when host is not loaded, things wouldn't work.

};

const getExportersStatus = async () => {
const { data } = await networkClient.get(
Expand All @@ -55,97 +69,119 @@ function HostDetails() {
const versionWarningMessage = agentVersionWarning(host.agent_version);

return (
<div>
<BackButton url="/hosts">Back to Hosts</BackButton>
<div className="flex">
<PageHeader>
Host Details: <span className="font-bold">{host.hostname}</span>
</PageHeader>
<StatusPill
className="self-center ml-4 shadow"
heartbeat={host.heartbeat}
>
Agent
</StatusPill>

{Object.entries(exportersStatus).map(
([exporterName, exporterStatus]) => (
<StatusPill
key={exporterName}
className="self-center ml-4 shadow"
heartbeat={exporterStatus}
>
{exporterName}
</StatusPill>
)
<>
<DeregistrationModal
hostname={host.hostname}
isOpen={!!cleanUpModalOpen}
onCleanUp={() => {
cleanUpHost();
}}
onCancel={() => {
setCleanUpModalOpen(false);
}}
/>
<div>
<BackButton url="/hosts">Back to Hosts</BackButton>
<div className="flex">
<PageHeader>
Host Details: <span className="font-bold">{host.hostname}</span>
</PageHeader>
<StatusPill
className="self-center ml-4 shadow"
heartbeat={host.heartbeat}
>
Agent
</StatusPill>

{Object.entries(exportersStatus).map(
([exporterName, exporterStatus]) => (
<StatusPill
key={exporterName}
className="self-center ml-4 shadow"
heartbeat={exporterStatus}
>
{exporterName}
</StatusPill>
)
)}
{host.deregisterable && (
<div className="ml-auto my-auto">
<CleanUpButton
cleaning={host.deregistering}
onClick={() => {
openDeregistrationModal();
}}
/>
</div>
)}
</div>
{versionWarningMessage && (
<WarningBanner>{versionWarningMessage}</WarningBanner>
)}
</div>
{versionWarningMessage && (
<WarningBanner>{versionWarningMessage}</WarningBanner>
)}
<div className="mt-4 bg-white shadow rounded-lg py-4 px-8">
<ListView
orientation="vertical"
data={[
{ title: 'Name', content: host.hostname },
{
title: 'Cluster',
content: <ClusterLink cluster={cluster} />,
},
{ title: 'Agent version', content: host.agent_version },
]}
/>
</div>
<div className="mt-4 bg-white shadow rounded-lg py-4 px-8">
<ListView
orientation="vertical"
data={[
{ title: 'Name', content: host.hostname },
{
title: 'Cluster',
content: <ClusterLink cluster={cluster} />,
},
{ title: 'Agent version', content: host.agent_version },
]}
/>
</div>

<div className="mt-8 bg-white shadow rounded-lg py-4 px-8">
<iframe
title="node-exporter chart"
src={`${grafanaPublicUrl}/d-solo/rYdddlPWj/node-exporter-full?orgId=1&refresh=1m&theme=light&panelId=77&var-agentID=${host.id}`}
width="100%"
height="200"
frameBorder="0"
/>
</div>
<div className="mt-4 bg-white shadow rounded-lg py-4 px-8">
<iframe
title="node-exporter chart trento"
src={`${grafanaPublicUrl}/d-solo/rYdddlPWj/node-exporter-full?orgId=1&refresh=1m&theme=light&panelId=78&var-agentID=${host.id}`}
width="100%"
height="200"
frameBorder="0"
/>
</div>
<div className="mt-8 bg-white shadow rounded-lg py-4 px-8">
<iframe
title="node-exporter chart"
src={`${grafanaPublicUrl}/d-solo/rYdddlPWj/node-exporter-full?orgId=1&refresh=1m&theme=light&panelId=77&var-agentID=${host.id}`}
width="100%"
height="200"
frameBorder="0"
/>
</div>
<div className="mt-4 bg-white shadow rounded-lg py-4 px-8">
<iframe
title="node-exporter chart trento"
src={`${grafanaPublicUrl}/d-solo/rYdddlPWj/node-exporter-full?orgId=1&refresh=1m&theme=light&panelId=78&var-agentID=${host.id}`}
width="100%"
height="200"
frameBorder="0"
/>
</div>

<div className="mt-16">
<div className="mb-4">
<h2 className="text-2xl font-bold">Provider details</h2>
<div className="mt-16">
<div className="mb-4">
<h2 className="text-2xl font-bold">Provider details</h2>
</div>
<ProviderDetails
provider={host.provider}
provider_data={host.provider_data}
/>
</div>
<ProviderDetails
provider={host.provider}
provider_data={host.provider_data}
/>
</div>

<div className="mt-8">
<div>
<h2 className="text-2xl font-bold">SAP instances</h2>
<div className="mt-8">
<div>
<h2 className="text-2xl font-bold">SAP instances</h2>
</div>
<Table config={sapInstancesTableConfiguration} data={sapSystems} />
</div>
<Table config={sapInstancesTableConfiguration} data={sapSystems} />
</div>

<div className="mt-16">
<div className="flex flex-direction-row">
<img src={SuseLogo} className="h-12" alt="suse company logo" />
<h2 className="ml-2 text-2xl font-bold self-center">
SLES subscription details
</h2>
<div className="mt-16">
<div className="flex flex-direction-row">
<img src={SuseLogo} className="h-12" alt="suse company logo" />
<h2 className="ml-2 text-2xl font-bold self-center">
SLES subscription details
</h2>
</div>
<Table
config={subscriptionsTableConfiguration}
data={host.sles_subscriptions}
/>
</div>
<Table
config={subscriptionsTableConfiguration}
data={host.sles_subscriptions}
/>
</div>
</div>
</>
);
}

Expand Down
114 changes: 113 additions & 1 deletion assets/js/components/HostDetails/HostDetails.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { screen } from '@testing-library/react';
import { screen, fireEvent } from '@testing-library/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Use userEvent instead of fireEvent. It is the recommended library.
You will need to setup the user, etc
Check other test files with this approach

import 'intersection-observer';
import '@testing-library/jest-dom';
import MockAdapter from 'axios-mock-adapter';

Expand Down Expand Up @@ -66,4 +67,115 @@ describe('HostDetails component', () => {
)
).toBeInTheDocument();
});

it("should not display 'Clean up' button when host is not deregisterable", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

double quotes? I guess the ci will complain about this hehe
I would remove the single quotes in any case from Clean up and put them in the test string

Copy link
Contributor

Choose a reason for hiding this comment

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

We could maybe add a deregistration describe to group the tests about this topic.
Again, not strong opinions

Copy link
Member

Choose a reason for hiding this comment

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

+1

const hosts = hostFactory.buildList(1, { deregisterable: false });
const state = {
...defaultInitialState,
hostsList: {
hosts,
},
};
const [StatefulHostDetails] = withState(<HostDetails />, state);

const { id } = hosts[0];
renderWithRouterMatch(StatefulHostDetails, {
path: '/hosts/:hostID',
route: `/hosts/${id}`,
});

const cleanUpButton = screen.queryByRole('button', { name: 'Clean up' });
expect(cleanUpButton).not.toBeInTheDocument();
});

it("should display 'Clean up' button when host is deregisterable", () => {
const hosts = hostFactory.buildList(1, { deregisterable: true });

const state = {
...defaultInitialState,
hostsList: {
hosts,
},
};
const [StatefulHostDetails] = withState(<HostDetails />, state);

const { id } = hosts[0];

renderWithRouterMatch(StatefulHostDetails, {
path: '/hosts/:hostID',
route: `/hosts/${id}`,
});

expect(
screen.getByRole('button', { name: 'Clean up' })
).toBeInTheDocument();
});

it('should show the host in deregistering state', () => {
const host = hostFactory.build({
deregisterable: true,
deregistering: true,
});
const state = {
...defaultInitialState,
hostsList: {
hosts: [host],
},
};

const [StatefulHostDetails] = withState(<HostDetails />, state);

const { id } = host;

renderWithRouterMatch(StatefulHostDetails, {
path: '/hosts/:hostID',
route: `/hosts/${id}`,
});

const cleanUpButton = screen.queryByRole('button', { name: 'Clean up' });
expect(cleanUpButton).not.toBeInTheDocument();

const loadingSpinner = screen.getByRole('alert', { name: 'Loading' });
expect(loadingSpinner).toBeInTheDocument();
});

it('should request a deregistration when the clean up button in the modal is clicked', async () => {
const host = hostFactory.build({ deregisterable: true });
const state = {
...defaultInitialState,
hostsList: {
hosts: [host],
},
};

const [StatefulHostDetails, store] = withState(<HostDetails />, state);

const { id, hostname } = host;

renderWithRouterMatch(StatefulHostDetails, {
path: '/hosts/:hostID',
route: `/hosts/${id}`,
});

const cleanUpButton = screen.getByRole('button', { name: 'Clean up' });
fireEvent.click(cleanUpButton);

expect(
screen.getByText(`Clean up data discovered by agent on host ${hostname}`)
).toBeInTheDocument();

const cleanUpModalButton = screen.getAllByRole('button', {
name: 'Clean up',
})[1];
fireEvent.click(cleanUpModalButton);

const actions = store.getActions();
const expectedActions = [
{
type: 'DEREGISTER_HOST',
payload: expect.objectContaining({ id, hostname }),
},
];
expect(actions).toEqual(expect.arrayContaining(expectedActions));
});
});
Loading