Skip to content

Commit

Permalink
[#218] fix: Broker page refreshes automatically upon broker creation
Browse files Browse the repository at this point in the history
fixed the bug where the broker page refreshes automatically upon broker creation.
and updated the unit tests.
  • Loading branch information
Msarawan authored and lavocatt committed Sep 12, 2024
1 parent c9d7a49 commit 3985c9e
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 38 deletions.
24 changes: 15 additions & 9 deletions src/brokers/view-brokers/BrokersList.container.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import { render, screen, waitFor } from '@app/test-utils';
import { AMQBrokerModel } from '@app/k8s/models';
import { BrokersContainer } from './BrokersList.container';
import { useParams, useNavigate } from 'react-router-dom-v5-compat';
import { k8sListItems } from '@openshift-console/dynamic-plugin-sdk';
import { useK8sWatchResource } from '@openshift-console/dynamic-plugin-sdk';

jest.mock('@openshift-console/dynamic-plugin-sdk', () => ({
k8sListItems: jest.fn(),
useK8sWatchResource: jest.fn(),
k8sDelete: jest.fn(),
}));

Expand Down Expand Up @@ -45,12 +45,12 @@ describe('BrokersContainer', () => {
const mockUseNavigate = useNavigate as jest.Mock;
const mockUseParams = useParams as jest.Mock;
const mockNamespace = 'test-namespace';
const mockK8sListItems = k8sListItems as jest.Mock;
const mockK8sWatchResource = useK8sWatchResource as jest.Mock;

beforeEach(() => {
mockUseNavigate.mockReturnValue(jest.fn());
mockUseParams.mockReturnValue({ ns: mockNamespace });
mockK8sListItems.mockResolvedValue([]);
mockK8sWatchResource.mockReturnValue([[], true, null]);
});

afterEach(() => {
Expand All @@ -61,9 +61,14 @@ describe('BrokersContainer', () => {
render(<BrokersContainer />);

await waitFor(() => {
expect(k8sListItems).toHaveBeenCalledWith({
model: AMQBrokerModel,
queryParams: { ns: mockNamespace },
expect(useK8sWatchResource).toHaveBeenCalledWith({
namespace: mockNamespace,
groupVersionKind: {
kind: AMQBrokerModel.kind,
version: AMQBrokerModel.apiVersion,
group: AMQBrokerModel.apiGroup,
},
isList: true,
});
});
expect(screen.getByText('BrokersList Component')).toBeInTheDocument();
Expand All @@ -72,9 +77,10 @@ describe('BrokersContainer', () => {
).toBeInTheDocument();
});

it('should handle API error on fetchK8sListItems', async () => {
it('should handle API error on fetching the ListItems', async () => {
const errorMessage = 'Failed to load brokers';
mockK8sListItems.mockRejectedValue(new Error(errorMessage));

mockK8sWatchResource.mockReturnValue([[], false, errorMessage]);

render(<BrokersContainer />);

Expand Down
51 changes: 23 additions & 28 deletions src/brokers/view-brokers/BrokersList.container.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { useEffect, useState, FC } from 'react';
import { k8sListItems, k8sDelete } from '@openshift-console/dynamic-plugin-sdk';
import { useState, FC } from 'react';
import {
k8sDelete,
useK8sWatchResource,
} from '@openshift-console/dynamic-plugin-sdk';
import { AMQBrokerModel } from '@app/k8s/models';
import { K8sResourceCommonWithData, BrokerCR } from '@app/k8s/types';
import { BrokersList } from './components/BrokersList/BrokersList';
Expand All @@ -11,33 +14,23 @@ export const BrokersContainer: FC = () => {
const { ns: namespace } = useParams<{ ns?: string }>();

//states
const [brokers, setBrokers] = useState<K8sResourceCommonWithData[]>();
const [loading, setLoading] = useState<boolean>(true);
const [loadError, setLoadError] = useState<any>();
const [isModalOpen, setIsModalOpen] = useState(false);
const [selectedBroker, setSelectedBroker] =
useState<K8sResourceCommonWithData>();
const [_deleteError, setDeleteError] = useState<string | null>(null);
const [_deleteSuccess, setDeleteSuccess] = useState<boolean>(false);

const fetchK8sListItems = () => {
setLoading(false);
k8sListItems<K8sResourceCommonWithData>({
model: AMQBrokerModel,
queryParams: { ns: namespace },
})
.then((brokers) => {
setBrokers(brokers);
})
.catch((e) => {
setLoadError(e.message);
})
.finally(() => {
setLoading(true);
});
};

useEffect(() => {
fetchK8sListItems();
}, [namespace]);
const [brokers, loaded, loadError] = useK8sWatchResource<
K8sResourceCommonWithData[]
>({
namespace,
groupVersionKind: {
kind: AMQBrokerModel.kind,
version: AMQBrokerModel.apiVersion,
group: AMQBrokerModel.apiGroup,
},
isList: true,
});

const onEditBroker = (broker: BrokerCR) => {
const namespace = broker.metadata.namespace;
Expand All @@ -51,10 +44,12 @@ export const BrokersContainer: FC = () => {
resource: { ...selectedBroker },
})
.then(() => {
fetchK8sListItems();
setDeleteSuccess(true);
setDeleteError(null);
})
.catch((e) => {
setLoadError(e.message);
setDeleteError(`Failed to delete broker: ${e.message}`);
setDeleteSuccess(false);
})
.finally(() => {
setIsModalOpen(false);
Expand All @@ -77,7 +72,7 @@ export const BrokersContainer: FC = () => {
<BrokersList
brokers={brokers}
loadError={loadError}
loaded={loading}
loaded={loaded}
namespace={namespace}
onOpenModal={onOpenModal}
onEditBroker={onEditBroker}
Expand Down
2 changes: 1 addition & 1 deletion src/k8s/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
export const AMQBrokerModel: K8sModel = {
apiGroup: AMQ_BROKER_APIGROUP,
apiVersion: API_VERSION,
kind: 'ActiveMQArtemisList',
kind: 'ActiveMQArtemis',
label: 'Broker',
labelKey: 'Brokers',
labelPlural: 'Brokers',
Expand Down

0 comments on commit 3985c9e

Please sign in to comment.