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

fix(RHINENG-14893): Correct empty state SystemDetails #2312

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
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
32 changes: 19 additions & 13 deletions src/SmartComponents/SystemDetails/Details.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import propTypes from 'prop-types';
import { Bullseye } from '@patternfly/react-core';
import ComplianceEmptyState from 'PresentationalComponents/ComplianceEmptyState';
import { useQuery } from '@apollo/client';
import { Spinner } from '@redhat-cloud-services/frontend-components/Spinner';
import './compliance.scss';
Expand All @@ -11,8 +10,15 @@ import SystemPoliciesAndRules from './SystemPoliciesAndRules';
import { SYSTEM_QUERY } from './constants';
import useTestResults from './useTestResults';
import SystemPoliciesAndRulesRest from './SystemPoliciesAndRulesRest';
import EmptyState from './EmptyState';

export const DetailsRest = ({ inventoryId, hidePassed, ...props }) => {
export const DetailsRest = ({
inventoryId,
hidePassed,
policiesCount,
insightsId,
...props
}) => {
const { testResults, testResultsLoading } = useTestResults(inventoryId);

return (
Expand All @@ -23,7 +29,7 @@ export const DetailsRest = ({ inventoryId, hidePassed, ...props }) => {
</Bullseye>
) : testResults.length === 0 ? (
// we render no policy cards nor rules table if there are no reporting policies
<ComplianceEmptyState title="No policies are reporting for this system" />
<EmptyState insightsId={insightsId} policiesCount={policiesCount} />
) : (
<SystemPoliciesAndRulesRest
{...props}
Expand All @@ -39,6 +45,8 @@ export const DetailsRest = ({ inventoryId, hidePassed, ...props }) => {
DetailsRest.propTypes = {
inventoryId: propTypes.string,
hidePassed: propTypes.bool,
policiesCount: propTypes.number,
insightsId: propTypes.string,
};

export const DetailsGraphQL = ({ inventoryId, hidePassed, ...props }) => {
Expand All @@ -59,16 +67,12 @@ export const DetailsGraphQL = ({ inventoryId, hidePassed, ...props }) => {

return (
<div className="ins-c-compliance__scope">
{!data?.system || is404 ? (
<ComplianceEmptyState title="No policies are reporting for this system" />
) : (
<SystemPoliciesAndRules
{...props}
hidePassed={hidePassed}
data={data}
loading={loading}
/>
)}
<SystemPoliciesAndRules
{...props}
hidePassed={hidePassed}
data={data}
loading={loading}
/>
</div>
);
};
Expand All @@ -95,6 +99,8 @@ export const Details = (props) => {
Details.propTypes = {
inventoryId: propTypes.string,
hidePassed: propTypes.bool,
policiesCount: propTypes.number,
insightsId: propTypes.string,
};

export default Details;
41 changes: 33 additions & 8 deletions src/SmartComponents/SystemDetails/EmptyState.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,46 @@ import propTypes from 'prop-types';
import { NotConnected } from '@redhat-cloud-services/frontend-components/NotConnected';
import NoPoliciesState from './NoPoliciesState';
import NoReportsState from './NoReportsState';
import { useLocation } from 'react-router-dom';
import { useParams } from 'react-router-dom';
import useSystem from 'Utilities/hooks/api/useSystem';
import { Bullseye, Spinner } from '@patternfly/react-core';

const EmptyState = ({ system }) => {
if (!system?.insightsId) {
const EmptyState = ({ insightsId, policiesCount }) => {
const location = useLocation();
const isInventoryInPath = location.pathname.includes('inventory');
const { inventoryId } = useParams();
console.log(`DEBUG insightsId ${!isInventoryInPath || !!insightsId}`)
const { data: { data } = {} } = useSystem({
params: [inventoryId],
skip: !isInventoryInPath || !!insightsId,
});

const effectiveInsightsId = data?.insights_id || insightsId;
const effectivePoliciesCount = data?.policies.length || policiesCount;

if (isInventoryInPath && !data) {
return (
<Bullseye>
<Spinner />
</Bullseye>
);
}

if (!effectiveInsightsId) {
return <NotConnected />;
}

if (policiesCount === 0) {
return <NoPoliciesState />;
} else {
if (!system?.hasPolicy) {
return <NoPoliciesState system={system} />;
} else if (system?.hasPolicy && system?.testResultProfiles?.length === 0) {
return <NoReportsState system={system} />;
}
return <NoReportsState policiesCount={effectivePoliciesCount} />;
}
};

EmptyState.propTypes = {
system: propTypes.object,
insightsId: propTypes.string,
policiesCount: propTypes.number,
};

export default EmptyState;
12 changes: 5 additions & 7 deletions src/SmartComponents/SystemDetails/NoReportsState.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
EmptyStateHeader,
} from '@patternfly/react-core';

const NoReportsState = ({ system }) => (
const NoReportsState = ({ policiesCount }) => (
<Bullseye>
<EmptyState
style={{
Expand All @@ -31,9 +31,9 @@ const NoReportsState = ({ system }) => (
headingLevel="h1"
/>
<EmptyStateBody>
This system is part of {system?.policies?.length}
{system?.policies?.length > 1 ? ' policies' : ' policy'}, but has not
returned any results.
This system is part of {policiesCount}
{policiesCount > 1 ? ' policies' : ' policy'}, but has not returned any
results.
</EmptyStateBody>
<EmptyStateBody>
Reports are returned when the system checks into Insights. By default,
Expand All @@ -44,9 +44,7 @@ const NoReportsState = ({ system }) => (
);

NoReportsState.propTypes = {
system: propTypes.shape({
policies: propTypes.array,
}),
policiesCount: propTypes.number,
};

export default NoReportsState;
4 changes: 2 additions & 2 deletions src/SmartComponents/SystemDetails/NoReportsState.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import NoReportsState from './NoReportsState';

describe('NoReportsState', () => {
it('with a system having policies', () => {
render(<NoReportsState system={{ policies: [{}] }} />);
render(<NoReportsState policiesCount={1} />);

expect(
screen.getByText(
Expand All @@ -15,7 +15,7 @@ describe('NoReportsState', () => {
});

it('with a system having multiple policies', () => {
render(<NoReportsState system={{ policies: [{}, {}] }} />);
render(<NoReportsState policiesCount={2} />);

expect(
screen.getByText(
Expand Down
25 changes: 23 additions & 2 deletions src/SmartComponents/SystemDetails/SystemDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ const SystemDetailBase = ({
loading,
systemName,
inventoryId,
policiesCount,
insightsId,
}) => (
<StateViewWithError stateValues={{ error, data, loading }}>
<StateViewPart stateKey="data">
Expand All @@ -60,7 +62,12 @@ const SystemDetailBase = ({
<InventoryDetails inventoryId={inventoryId} />
</PageHeader>
<PageSection>
<Details hidePassed inventoryId={inventoryId} />
<Details
hidePassed
inventoryId={inventoryId}
policiesCount={policiesCount}
insightsId={insightsId}
/>
</PageSection>
</StateViewPart>
<StateViewPart stateKey="loading">
Expand All @@ -77,6 +84,8 @@ SystemDetailBase.propTypes = {
loading: propTypes.bool,
systemName: propTypes.string,
inventoryId: propTypes.string,
policiesCount: propTypes.number,
insightsId: propTypes.string,
};

const SystemDetailsGraphQL = ({ route }) => {
Expand Down Expand Up @@ -105,11 +114,23 @@ const SystemDetailsRest = ({ route }) => {
loading,
} = useSystem({ params: [inventoryId] });
const systemName = data?.display_name || inventoryId;
const policiesCount = data?.policies.length;
const insightsId = data?.insights_id;

useTitleEntity(route, systemName);

return (
<SystemDetailBase {...{ data, error, loading, systemName, inventoryId }} />
<SystemDetailBase
{...{
data,
error,
loading,
systemName,
inventoryId,
policiesCount,
insightsId,
}}
/>
);
};

Expand Down
39 changes: 38 additions & 1 deletion src/SmartComponents/SystemDetails/SystemDetails.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useQuery } from '@apollo/client';
import { useLocation } from 'react-router-dom';
import SystemDetails from './SystemDetails.js';
import useAPIV2FeatureFlag from '../../Utilities/hooks/useAPIV2FeatureFlag.js';
import useSystem from 'Utilities/hooks/api/useSystem';

jest.mock('@apollo/client');

Expand All @@ -24,6 +25,8 @@ jest.mock('Utilities/hooks/useDocumentTitle', () => ({

jest.mock('../../Utilities/hooks/useAPIV2FeatureFlag.js');

jest.mock('Utilities/hooks/api/useSystem', () => jest.fn());

describe('SystemDetails', () => {
const defaultLocation = {
query: {
Expand All @@ -34,6 +37,7 @@ describe('SystemDetails', () => {
system: {
insightsId: 'ID_YEAH',
name: 'test.host.local',
policies: [],
testResultProfiles: [],
},
};
Expand All @@ -49,6 +53,17 @@ describe('SystemDetails', () => {
});

it('expect to render the inventory details', () => {
require('react-router-dom').useLocation.mockReturnValue({
pathname: '/insights/inventory',
});
useSystem.mockImplementation(() => ({
data: {},
error: undefined,
loading: undefined,
}));

expect(useSystem).not.toHaveBeenCalled();

render(
<TestWrapper>
<SystemDetails />
Expand All @@ -70,6 +85,28 @@ describe('SystemDetails', () => {

expect(screen.getByText('Loading...')).toBeInTheDocument();
});
});

describe('SystemDetails - REST', () => {
beforeEach(() => {
useAPIV2FeatureFlag.mockImplementation(() => true);
});

it('expect to render Inventory Details Wrapper', () => {
useSystem.mockImplementation(() => ({
data: { data: { display_name: "foo", policies: [{}], insights_id: "123" }},
error: undefined,
loading: undefined,
}));
render(
<TestWrapper>
<SystemDetails route={{}}/>
</TestWrapper>
);

// TODO: add tests for the REST API component
expect(
screen.getByLabelText('Inventory Details Wrapper')
).toBeInTheDocument();
expect(screen.getByText('Loading...')).toBeInTheDocument();
})
});
21 changes: 13 additions & 8 deletions src/SmartComponents/SystemDetails/SystemPoliciesAndRules.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ const SystemPoliciesAndRules = ({ data: { system }, loading, hidePassed }) => {
const [selectedPolicy, setSelectedPolicy] = useState(
system.testResultProfiles[0]?.id
);
const policies = system?.testResultProfiles;
const testResultProfiles = system?.testResultProfiles;
const policiesCount = system?.policies.length;
const insightsId = system?.insightsId;

const sorter = natsort({ desc: false, insensitive: true });
const sortedTestResultProfiles = system?.testResultProfiles.sort(
Expand All @@ -36,12 +38,12 @@ const SystemPoliciesAndRules = ({ data: { system }, loading, hidePassed }) => {
) : apiV2Enabled === true ? (
<SystemPolicyCardsRest />
) : (
<SystemPolicyCards policies={policies} loading={loading} />
<SystemPolicyCards policies={testResultProfiles} loading={loading} />
)}
<br />
{system?.testResultProfiles?.length ? (
{testResultProfiles?.length ? (
<>
{system.testResultProfiles.length > 1 && (
{testResultProfiles.length > 1 && (
<Tabs
activeKey={selectedPolicy}
style={{
Expand Down Expand Up @@ -91,7 +93,7 @@ const SystemPoliciesAndRules = ({ data: { system }, loading, hidePassed }) => {
/>
</>
) : (
<EmptyState system={system} />
<EmptyState insightsId={insightsId} policiesCount={policiesCount} />
)}
</>
);
Expand All @@ -101,11 +103,14 @@ SystemPoliciesAndRules.propTypes = {
data: propTypes.shape({
system: propTypes.shape({
hasPolicy: propTypes.bool,
policies: propTypes.shape({
id: propTypes.string,
}),
policies: propTypes.arrayOf(
propTypes.shape({
id: propTypes.string,
})
),
profiles: propTypes.array,
testResultProfiles: propTypes.array,
insightsId: propTypes.string,
}),
}),
loading: propTypes.bool,
Expand Down