Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Commit 1da8fda

Browse files
committed
fix: Refactor to Topic panel
- Addresses initial review comments. Contributes to: #124 Signed-off-by: Andrew Borley <[email protected]>
1 parent b7111e6 commit 1da8fda

24 files changed

+299
-145
lines changed

client/Bootstrap/index.tsx

+4-2
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,17 @@ import {
1313
FeatureFlag,
1414
LoggingProvider,
1515
} from 'Contexts';
16-
import { Home } from 'Panels/Home';
16+
import { Home, Topics } from 'Panels';
1717

1818
init(); //Bootstrap i18next support
1919
ReactDOM.render(
2020
<ApolloProvider client={apolloClient}>
2121
<ConfigFeatureFlagProvider>
2222
<LoggingProvider>
2323
<FeatureFlag flag={'client.Pages.PlaceholderHome'}>
24-
<Home />
24+
<Home>
25+
<Topics />
26+
</Home>
2527
</FeatureFlag>
2628
</LoggingProvider>
2729
</ConfigFeatureFlagProvider>

client/Contexts/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ It is expected that when used in production Context Provider(s) will be used by
66

77
## Test approach
88

9-
Elements should be tested in a functional manor. See [Test Driven Development](../../docs/Test.md#style-of-test).
9+
Elements should be tested in a functional manner. See [Test Driven Development](../../docs/Test.md#style-of-test).
1010

1111
## Expected files
1212

client/Elements/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
## Test approach
66

7-
Bootstrap should be tested in a behavioural manor. See [Behavioural Driven Development](../../docs/Test.md#style-of-test).
7+
Bootstrap should be tested in a behavioural manner. See [Behavioural Driven Development](../../docs/Test.md#style-of-test).
88

99
## Expected files
1010

client/Groups/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
## Test approach
66

7-
Bootstrap should be tested in a behavioural manor. See [Behavioural Driven Development](../../docs/Test.md#style-of-test).
7+
Bootstrap should be tested in a behavioural manner. See [Behavioural Driven Development](../../docs/Test.md#style-of-test).
88

99
## Expected files
1010

client/Hooks/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ This directory contains all re-usable custom [React Hooks](https://reactjs.org/d
44

55
## Test approach
66

7-
Elements should be tested in a functional manor. See [Test Driven Development](../../docs/Test.md#style-of-test).
7+
Elements should be tested in a functional manner. See [Test Driven Development](../../docs/Test.md#style-of-test).
88

99
## Expected files
1010

client/Models/README.md

-37
This file was deleted.

client/Models/useTopics/Hook.ts

-14
This file was deleted.

client/Models/useTopics/README.md

-5
This file was deleted.

client/Models/useTopics/useTopics.spec.tsx

-40
This file was deleted.

client/Panels/Home/Home.steps.tsx

+3-21
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,7 @@
55
import { Given, When, Then, Fusion } from 'jest-cucumber-fusion';
66
import { RenderResult } from '@testing-library/react';
77
import merge from 'lodash.merge';
8-
import {
9-
renderWithCustomConfigFeatureFlagContext,
10-
withApolloProviderReturning,
11-
apolloMockResponse,
12-
} from 'utils/test';
13-
import { mockGetTopicsRequests } from 'Models';
8+
import { renderWithCustomConfigFeatureFlagContext } from 'utils/test';
149
import { Home } from '.';
1510
import React, { ReactElement } from 'react';
1611

@@ -36,10 +31,7 @@ Given('a Home component', () => {
3631
When('it is rendered', () => {
3732
renderResult = renderWithCustomConfigFeatureFlagContext(
3833
coreConfigFromContext,
39-
withApolloProviderReturning(
40-
[mockGetTopicsRequests.successRequest],
41-
component
42-
)
34+
component
4335
);
4436
showVersionSet = true;
4537
});
@@ -55,10 +47,7 @@ When('it is rendered with no version', () => {
5547
},
5648
},
5749
}),
58-
withApolloProviderReturning(
59-
[mockGetTopicsRequests.successRequest],
60-
component
61-
)
50+
component
6251
);
6352
showVersionSet = false;
6453
});
@@ -70,13 +59,6 @@ Then('it should display the expected text', async () => {
7059
showVersionSet
7160
? expect(getByText(versionString)).toBeInTheDocument()
7261
: expect(queryByText(versionString)).not.toBeInTheDocument();
73-
74-
const loadingTopicsString = 'Loading...';
75-
expect(getByText(loadingTopicsString)).toBeInTheDocument();
76-
77-
await apolloMockResponse();
78-
expect(queryByText(loadingTopicsString)).not.toBeInTheDocument();
79-
expect(getByText('Number of topics: 3')).toBeInTheDocument();
8062
});
8163

8264
Fusion('Home.feature');

client/Panels/Home/Home.tsx

-9
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import get from 'lodash.get';
77
import image from 'Images/logo.png';
88
import './style.scss';
99
import { useConfigFeatureFlag, useLogger } from 'Hooks';
10-
import { useTopics } from 'Models';
1110

1211
const Home: FunctionComponent = ({ children }) => {
1312
const { client, featureFlags, isComplete } = useConfigFeatureFlag();
@@ -18,19 +17,11 @@ const Home: FunctionComponent = ({ children }) => {
1817
const { debug } = useLogger('Home');
1918
debug(`Client version to display: ${version}`);
2019

21-
const { useGetTopics } = useTopics();
22-
const { data, loading } = useGetTopics();
23-
2420
return (
2521
<div className='home'>
2622
<img src={image} alt='Strimzi logo' />
2723
<p>Welcome to the Strimzi UI</p>
2824
{showVersion && isComplete && <p>{`Version: ${version}`}</p>}
29-
{loading || !data ? (
30-
<p>Loading...</p>
31-
) : (
32-
<p>{`Number of topics: ${data.topics.length}`}</p>
33-
)}
3425
{children}
3526
</div>
3627
);

client/Panels/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ A `Panel` component represents a top level element of a UI - the primary/signifi
44

55
## Test approach
66

7-
Bootstrap should be tested in a behavioural manor. See [Behavioural Driven Development](../../docs/Test.md#style-of-test).
7+
Bootstrap should be tested in a behavioural manner. See [Behavioural Driven Development](../../docs/Test.md#style-of-test).
88

99
## Expected files
1010

client/Panels/Topics/Model.ts

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright Strimzi authors.
3+
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html).
4+
*/
5+
import { useState, useEffect } from 'react';
6+
import { GET_TOPICS } from 'Queries/Topics';
7+
import { useQuery } from '@apollo/client';
8+
import {
9+
useTopicsModelType,
10+
useGetTopicsResultType,
11+
} from './useTopicsModel.types';
12+
13+
export const useTopicsModel = (): useTopicsModelType => {
14+
const useGetTopics = (filter?: string): useGetTopicsResultType => {
15+
// Use debounce to ensure char-by-char changes to the filter string do not cause floods of useQuery network calls
16+
const debouncedFilter = useDebounce(filter, 500);
17+
// Caching in useQuery means that new network calls are only made when the debouncedFilter value changes
18+
return useQuery(GET_TOPICS, { variables: { filter: debouncedFilter } });
19+
};
20+
21+
return {
22+
useGetTopics,
23+
};
24+
};
25+
26+
// Adapted from https://usehooks.com/useDebounce/
27+
const useDebounce = <T>(value: T, delay: number): T => {
28+
// State and setters for debounced value
29+
const [debouncedValue, setDebouncedValue] = useState(value);
30+
useEffect(
31+
() => {
32+
// Update debounced value after delay
33+
const handler = setTimeout(() => setDebouncedValue(value), delay);
34+
35+
// Cancel the timeout if value changes (also on delay change or unmount)
36+
return () => clearTimeout(handler);
37+
},
38+
[value, delay] // Only re-call effect if value or delay changes
39+
);
40+
return debouncedValue;
41+
};

client/Panels/Topics/README.md

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Topics
2+
3+
The `Topics` panel retrieves and displays the list of Kafka topics for a cluster.

client/Panels/Topics/Topics.feature

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
2+
# Copyright Strimzi authors.
3+
# License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html).
4+
Feature: Topics panel
5+
6+
Scenario: Seeing all available topics:
7+
Given a Topics panel component
8+
When it is rendered
9+
Then I see all topics
10+
11+
Scenario: Viewing a specific topic (when it exists in the cluster):
12+
Given a Topics panel component
13+
When it is rendered
14+
And I filter for topic 'testtopic1'
15+
Then I see topic 'testtopic1' in the topic list

client/Panels/Topics/Topics.steps.tsx

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright Strimzi authors.
3+
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html).
4+
*/
5+
import {
6+
And,
7+
Given,
8+
When,
9+
Then,
10+
Fusion,
11+
Before,
12+
After,
13+
} from 'jest-cucumber-fusion';
14+
import {
15+
fireEvent,
16+
render,
17+
RenderResult,
18+
waitFor,
19+
} from '@testing-library/react';
20+
import { Topics } from '.';
21+
import React, { ReactElement } from 'react';
22+
import { withApolloProviderReturning } from 'utils/test';
23+
import {
24+
mockGetTopicsRequests,
25+
mockGetTopicsResponses,
26+
} from './useTopicsModel.assets';
27+
28+
let renderResult: RenderResult;
29+
let component: ReactElement;
30+
31+
Before(() => {
32+
jest.useFakeTimers();
33+
});
34+
35+
After(() => {
36+
jest.useRealTimers();
37+
});
38+
39+
Given('a Topics panel component', () => {
40+
component = <Topics />;
41+
});
42+
43+
When('it is rendered', () => {
44+
renderResult = render(
45+
withApolloProviderReturning(
46+
[
47+
mockGetTopicsRequests.successRequest,
48+
mockGetTopicsRequests.successFilteredRequest,
49+
],
50+
component
51+
)
52+
);
53+
jest.runAllTimers();
54+
});
55+
56+
And(/^I filter for topic '(.+)'$/, async (filter) => {
57+
fireEvent.change(renderResult.getByPlaceholderText('filter'), {
58+
target: { value: filter },
59+
});
60+
await waitFor(() => {
61+
jest.runAllTimers();
62+
});
63+
});
64+
65+
Then('I see all topics', async () => {
66+
const { findByText } = renderResult;
67+
expect(
68+
await findByText(JSON.stringify(mockGetTopicsResponses.successResponse), {
69+
exact: false,
70+
})
71+
).toBeInTheDocument();
72+
});
73+
74+
Then(/^I see topic '(.+)' in the topic list$/, async (topicName) => {
75+
const { findByText } = renderResult;
76+
expect(
77+
await findByText(
78+
JSON.stringify(mockGetTopicsResponses.successFilteredResponse),
79+
{ exact: false }
80+
)
81+
).toBeInTheDocument();
82+
expect(
83+
await findByText(topicName as string, { exact: false })
84+
).toBeInTheDocument();
85+
});
86+
87+
Fusion('Topics.feature');

0 commit comments

Comments
 (0)