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(ESSNTL-4194): Minor improvements #1879

Merged
merged 16 commits into from
Jun 21, 2023
Merged
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
3 changes: 2 additions & 1 deletion src/components/GroupsTable/GroupsTable.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import GroupsTable from './GroupsTable';

const DEFAULT_ROW_COUNT = 50;
const TABLE_HEADERS = ['Name', 'Total systems', 'Last modified'];
const SORTABLE_HEADERS = ['Name', 'Total systems'];
const ROOT = 'div[id="groups-table"]';

const mountTable = (initialEntry = '/') =>
Expand Down Expand Up @@ -172,7 +173,7 @@ describe('sorting', () => {
cy.wait('@getGroups'); // first initial request
});

_.zip(['name', 'host_ids', 'updated_at'], TABLE_HEADERS).forEach(
_.zip(['name', 'host_ids'], SORTABLE_HEADERS).forEach(
([category, label]) => {
SORTING_ORDERS.forEach((order) => {
it(`${order} by ${label}`, () => {
Expand Down
13 changes: 9 additions & 4 deletions src/components/GroupsTable/GroupsTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const GROUPS_TABLE_COLUMNS = [
},
{
title: 'Last modified',
transforms: [sortable, cellWidth(20)]
transforms: [cellWidth(20)]
}
];

Expand Down Expand Up @@ -155,11 +155,13 @@ const GroupsTable = () => {
}));
setRows(newRows);

if (selectedIds.length <= 1) {
setSelectedGroup(selectedIds.length === 0 ? undefined : {
if (selectedIds.length === 1) {
setSelectedGroup({
id: selectedIds[0],
name: groups.find(({ id }) => id === selectedIds[0])?.name
});
} else {
setSelectedGroup(undefined);
}
}, [groups, selectedIds]);

Expand Down Expand Up @@ -322,7 +324,10 @@ const GroupsTable = () => {

setDeleteModalOpen(value);
}}
reloadData={() => fetchData(filters)}
reloadData={() => {
fetchData(filters);
setSelectedIds([]);
}}
groupIds={selectedGroup !== undefined ? [selectedGroup.id] : selectedIds}
/>
}
Expand Down
3 changes: 3 additions & 0 deletions src/components/InventoryGroups/InventoryGroups.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe('groups table page', () => {
it('renders table if there is at least one group', () => {
interceptors['successful with some items']();
mountPage();
cy.wait('@getGroups');

cy.get('h1').contains('Groups');
cy.get('#groups-table');
Expand All @@ -30,6 +31,7 @@ describe('groups table page', () => {
it('renders only empty state when there are no groups', () => {
interceptors['successful empty']();
mountPage();
cy.wait('@getGroups');

cy.get('h1').contains('Groups');
cy.get('#groups-table').should('not.exist');
Expand All @@ -39,6 +41,7 @@ describe('groups table page', () => {
it('renders error message when request fails', () => {
interceptors['failed with server error']();
mountPage();
cy.wait('@getGroups');

cy.get('h1').contains('Groups');
cy.get('#groups-table').should('not.exist');
Expand Down
16 changes: 11 additions & 5 deletions src/components/InventoryGroups/InventoryGroups.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
PageHeader,
PageHeaderTitle
} from '@redhat-cloud-services/frontend-components';
import React, { useEffect, useState } from 'react';
import React, { useEffect, useRef, useState } from 'react';

import { Bullseye, Spinner } from '@patternfly/react-core';
import GroupsTable from '../GroupsTable/GroupsTable';
Expand All @@ -15,23 +15,29 @@ const InventoryGroups = () => {
const [hasGroups, setHasGroups] = useState(false);
const [hasError, setHasError] = useState(false);

const ignore = useRef(false); // https://react.dev/learn/synchronizing-with-effects#fetching-data

const handleLoading = async () => {
// make initial request to check if there is at least one group available
try {
const { total } = await getGroups();

if (total > 0) {
setHasGroups(true);
!ignore.current && setHasGroups(true);
}
} catch (error) {
setHasError(true);
!ignore.current && setHasError(true);
}

setIsLoading(false);
!ignore.current && setIsLoading(false);
};

useEffect(() => {
handleLoading();

return () => {
ignore.current = true;
};
}, []);

return (
Expand All @@ -49,7 +55,7 @@ const InventoryGroups = () => {
) : hasGroups ? (
<GroupsTable />
) : (
<NoGroupsEmptyState />
<NoGroupsEmptyState reloadData={handleLoading}/>
)}
</section>
</React.Fragment>
Expand Down
5 changes: 3 additions & 2 deletions src/components/InventoryGroups/Modals/DeleteGroupModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ const DeleteGroupModal = ({
const groupsAreEmpty = (fetchedGroups || []).every(({ host_count: hostCount }) => hostCount === 0);
const [isLoading, setIsLoading] = useState(true);

console.log('###', groupIds);
useEffect(() => {
// check that all groups are empty before deletion
let ignore = false;
Expand Down Expand Up @@ -128,14 +129,14 @@ const DeleteGroupModal = ({
description:
groupIds.length > 1
? `${groupIds.length} groups deleted`
: `${name} has been removed successfully`
: `${fetchedGroups?.[0]?.name} has been removed successfully`
},
onError: {
title: 'Error',
description:
groupIds.length > 1
? `Failed to delete ${groupIds.length} groups`
: `Failed to delete group ${name}`
: `Failed to delete group ${fetchedGroups?.[0]?.name}`
}
};
apiWithToast(dispatch, () => deleteGroupsById(groupIds), statusMessages);
Expand Down
6 changes: 5 additions & 1 deletion src/components/InventoryGroups/Modals/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ const RepoModal = ({
//reload comes from the table and fetches fresh data
onSubmit={async (values) => {
await onSubmit(values);
setTimeout(async () => await reloadData(), 500);

if (reloadData !== undefined) {
setTimeout(async () => await reloadData(), 500);
}

closeModal();
}}
onCancel={() => closeModal()}
Expand Down
9 changes: 7 additions & 2 deletions src/components/InventoryGroups/NoGroupsEmptyState.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import {
Title
} from '@patternfly/react-core';
import { ExternalLinkAltIcon, PlusCircleIcon } from '@patternfly/react-icons';
import PropTypes from 'prop-types';

import { global_palette_black_600 as globalPaletteBlack600 } from '@patternfly/react-tokens/dist/js/global_palette_black_600';
import CreateGroupModal from './Modals/CreateGroupModal';

const NoGroupsEmptyState = () => {
const NoGroupsEmptyState = ({ reloadData }) => {
const [createGroupModalOpen, setCreateGroupModalOpen] = useState(false);

return (
Expand All @@ -24,7 +25,7 @@ const NoGroupsEmptyState = () => {
<CreateGroupModal
isModalOpen={createGroupModalOpen}
setIsModalOpen={setCreateGroupModalOpen}
//Todo: reloadData={reloadData} add refetch data so the list of groups will update
reloadData={reloadData}
/>
<EmptyStateIcon icon={PlusCircleIcon} color={globalPaletteBlack600.value} />
<Title headingLevel="h4" size="lg">
Expand All @@ -47,4 +48,8 @@ const NoGroupsEmptyState = () => {
</EmptyState>
);};

NoGroupsEmptyState.propTypes = {
reloadData: PropTypes.func
};

export default NoGroupsEmptyState;