Skip to content

Commit 024b1f2

Browse files
byronhulcherbyronhulcher
committed
PR Feedback
1 parent b9aba42 commit 024b1f2

9 files changed

Lines changed: 54 additions & 111 deletions

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/meta_engines_table.test.tsx‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -198,27 +198,25 @@ describe('MetaEnginesTable', () => {
198198
});
199199
});
200200

201-
describe('source engines', () => {
202-
describe('source engine table', () => {
203-
beforeEach(() => {
204-
resetMocks();
205-
});
201+
describe('expanded source engines', () => {
202+
beforeEach(() => {
203+
resetMocks();
204+
});
206205

207-
it('is hidden by default', () => {
208-
const wrapper = shallow(<MetaEnginesTable {...props} />);
209-
const table = wrapper.find(EuiBasicTable);
210-
expect(table.dive().find(MetaEnginesTableExpandedRow)).toHaveLength(0);
211-
});
206+
it('is hidden by default', () => {
207+
const wrapper = shallow(<MetaEnginesTable {...props} />);
208+
const table = wrapper.find(EuiBasicTable);
209+
expect(table.dive().find(MetaEnginesTableExpandedRow)).toHaveLength(0);
210+
});
212211

213-
it('is visible when the row has been expanded', () => {
214-
setMockValues({
215-
...DEFAULT_VALUES,
216-
expandedSourceEngines: { 'test-engine': true },
217-
});
218-
const wrapper = shallow(<MetaEnginesTable {...props} />);
219-
const table = wrapper.find(EuiBasicTable);
220-
expect(table.dive().find(MetaEnginesTableExpandedRow)).toHaveLength(1);
212+
it('is visible when the row has been expanded', () => {
213+
setMockValues({
214+
...DEFAULT_VALUES,
215+
expandedSourceEngines: { 'test-engine': true },
221216
});
217+
const wrapper = shallow(<MetaEnginesTable {...props} />);
218+
const table = wrapper.find(EuiBasicTable);
219+
expect(table.dive().find(MetaEnginesTableExpandedRow)).toHaveLength(1);
222220
});
223221
});
224222
});

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/meta_engines_table.tsx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,10 @@ export const MetaEnginesTable: React.FC<EnginesTableProps> = ({
9292
const columns: Array<EuiBasicTableColumn<EngineDetails>> = [
9393
{
9494
...NAME_COLUMN,
95-
render: (name: string, item: EngineDetails) => (
95+
render: (_, item: EngineDetails) => (
9696
<MetaEnginesTableNameColumnContent
97-
name={name}
9897
item={item}
99-
isExpanded={!!itemIdToExpandedRowMap[name]}
98+
isExpanded={!!itemIdToExpandedRowMap[item.name]}
10099
hideRow={hideRow}
101100
showRow={fetchOrDisplayRow}
102101
sendEngineTableLinkClickTelemetry={sendEngineTableLinkClickTelemetry}

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/meta_engines_table_expanded_row.scss‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
.meta-engines__source-engines-table {
2-
margin: -$euiSizeS -$euiSizeS $euiSizeS -$euiSizeS;
1+
.metaEnginesSourceEnginesTable {
2+
margin: (-$euiSizeS) (-$euiSizeS) $euiSizeS (-$euiSizeS);
33

44
thead {
55
display: none;

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/meta_engines_table_expanded_row.tsx‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import React from 'react';
99

10-
import { EuiBasicTable, EuiHealth, EuiTextAlign, EuiTitle } from '@elastic/eui';
10+
import { EuiBasicTable, EuiHealth, EuiTitle } from '@elastic/eui';
1111

1212
import { EngineDetails } from '../../../engine/types';
1313
import { SOURCE_ENGINES_TITLE } from '../../constants';
@@ -31,12 +31,10 @@ export const MetaEnginesTableExpandedRow: React.FC<MetaEnginesTableExpandedRowPr
3131
sourceEngines,
3232
conflictingEngines,
3333
}) => (
34-
<div className="meta-engines__source-engines-table">
35-
<EuiTextAlign textAlign="center">
36-
<EuiTitle size="xs">
37-
<h3>{SOURCE_ENGINES_TITLE}</h3>
38-
</EuiTitle>
39-
</EuiTextAlign>
34+
<div className="metaEnginesSourceEnginesTable">
35+
<EuiTitle size="xs" className="eui-textCenter">
36+
<h3>{SOURCE_ENGINES_TITLE}</h3>
37+
</EuiTitle>
4038
<EuiBasicTable
4139
items={sourceEngines}
4240
columns={[

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/meta_engines_table_logic.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ describe('MetaEnginesTableLogic', () => {
7777
});
7878
});
7979

80-
describe('sourceEngines is updated by addSourceEngines', () => {
80+
it('sourceEngines is updated by addSourceEngines', () => {
8181
mount({
8282
...DEFAULT_VALUES,
8383
sourceEngines: {
@@ -122,6 +122,16 @@ describe('MetaEnginesTableLogic', () => {
122122
});
123123

124124
it('calls fetchSourceEngines when it needs to fetch data for the itemId', () => {
125+
http.get.mockReturnValueOnce(
126+
Promise.resolve({
127+
meta: {
128+
page: {
129+
total_pages: 1,
130+
},
131+
},
132+
results: [{ name: 'source-engine-1' }, { name: 'source-engine-2' }],
133+
})
134+
);
125135
mount();
126136
jest.spyOn(MetaEnginesTableLogic.actions, 'fetchSourceEngines');
127137

@@ -205,7 +215,6 @@ describe('MetaEnginesTableLogic', () => {
205215

206216
MetaEnginesTableLogic.actions.fetchSourceEngines('test-engine-1');
207217
await nextTick();
208-
await nextTick(); // I think I need this for multiple http.get calls
209218

210219
expect(MetaEnginesTableLogic.actions.addSourceEngines).toHaveBeenCalledWith({
211220
'test-engine-1': [

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/meta_engines_table_name_column_content.test.tsx‎

Lines changed: 6 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
* 2.0.
66
*/
77

8-
import { mountWithIntl } from '../../../../../__mocks__';
9-
108
import React from 'react';
119

1210
import { shallow } from 'enzyme';
@@ -20,11 +18,10 @@ import { MetaEnginesTableNameColumnContent } from './meta_engines_table_name_col
2018

2119
describe('MetaEnginesTableNameColumnContent', () => {
2220
it('includes the name of the engine', () => {
23-
const wrapper = mountWithIntl(
21+
const wrapper = shallow(
2422
<MetaEnginesTableNameColumnContent
2523
showRow={jest.fn()}
2624
hideRow={jest.fn()}
27-
name={'test-engine'}
2825
item={
2926
{
3027
name: 'test-engine',
@@ -41,7 +38,7 @@ describe('MetaEnginesTableNameColumnContent', () => {
4138
/>
4239
);
4340

44-
expect(wrapper.text()).toContain('test-engine');
41+
expect(wrapper.find('[data-test-subj="EngineName"]')).toHaveLength(1);
4542
});
4643

4744
describe('toggle button', () => {
@@ -52,7 +49,6 @@ describe('MetaEnginesTableNameColumnContent', () => {
5249
<MetaEnginesTableNameColumnContent
5350
showRow={showRow}
5451
hideRow={jest.fn()}
55-
name={'test-engine'}
5652
item={
5753
{
5854
name: 'test-engine',
@@ -68,7 +64,7 @@ describe('MetaEnginesTableNameColumnContent', () => {
6864
sendEngineTableLinkClickTelemetry={jest.fn()}
6965
/>
7066
);
71-
wrapper.find('.meta-engines__expand-row').at(0).simulate('click');
67+
wrapper.find('[data-test-subj="ExpandRowButton"]').at(0).simulate('click');
7268

7369
expect(showRow).toHaveBeenCalled();
7470
});
@@ -80,7 +76,6 @@ describe('MetaEnginesTableNameColumnContent', () => {
8076
<MetaEnginesTableNameColumnContent
8177
showRow={jest.fn()}
8278
hideRow={hideRow}
83-
name={'test-engine'}
8479
item={
8580
{
8681
name: 'test-engine',
@@ -96,19 +91,18 @@ describe('MetaEnginesTableNameColumnContent', () => {
9691
sendEngineTableLinkClickTelemetry={jest.fn()}
9792
/>
9893
);
99-
wrapper.find('.meta-engines__expand-row').at(0).simulate('click');
94+
wrapper.find('[data-test-subj="ExpandRowButton"]').at(0).simulate('click');
10095

10196
expect(hideRow).toHaveBeenCalled();
10297
});
10398
});
10499

105100
describe('engine count', () => {
106101
it('is included and labelled', () => {
107-
const wrapper = mountWithIntl(
102+
const wrapper = shallow(
108103
<MetaEnginesTableNameColumnContent
109104
showRow={jest.fn()}
110105
hideRow={jest.fn()}
111-
name={'test-engine'}
112106
item={
113107
{
114108
name: 'test-engine',
@@ -129,60 +123,7 @@ describe('MetaEnginesTableNameColumnContent', () => {
129123
/>
130124
);
131125

132-
const wrapperContent = wrapper.text();
133-
expect(wrapperContent).toContain('2 Engines');
134-
});
135-
it('defaults to 0', () => {
136-
const wrapper = mountWithIntl(
137-
<MetaEnginesTableNameColumnContent
138-
showRow={jest.fn()}
139-
hideRow={jest.fn()}
140-
name={'test-engine'}
141-
item={
142-
{
143-
name: 'test-engine',
144-
created_at: 'Fri, 1 Jan 1970 12:00:00 +0000',
145-
language: 'English',
146-
isMeta: true,
147-
document_count: 99999,
148-
field_count: 10,
149-
includedEngines: [] as EngineDetails[],
150-
} as EngineDetails
151-
}
152-
isExpanded
153-
sendEngineTableLinkClickTelemetry={jest.fn()}
154-
/>
155-
);
156-
157-
const wrapperContent = wrapper.text();
158-
expect(wrapperContent).toContain('0 Engines');
159-
});
160-
161-
it('label loses pluralization with only 1 source engine', () => {
162-
const wrapper = mountWithIntl(
163-
<MetaEnginesTableNameColumnContent
164-
showRow={jest.fn()}
165-
hideRow={jest.fn()}
166-
name={'test-engine'}
167-
item={
168-
{
169-
name: 'test-engine',
170-
created_at: 'Fri, 1 Jan 1970 12:00:00 +0000',
171-
language: 'English',
172-
isMeta: true,
173-
document_count: 99999,
174-
field_count: 10,
175-
engine_count: 1,
176-
includedEngines: [{ name: 'source-engine-1' }] as EngineDetails[],
177-
} as EngineDetails
178-
}
179-
isExpanded
180-
sendEngineTableLinkClickTelemetry={jest.fn()}
181-
/>
182-
);
183-
184-
const wrapperContent = wrapper.text();
185-
expect(wrapperContent).toContain('1 Engine');
126+
expect(wrapper.find('[data-test-subj="SourceEnginesCount"]')).toHaveLength(1);
186127
});
187128
});
188129

@@ -191,7 +132,6 @@ describe('MetaEnginesTableNameColumnContent', () => {
191132
<MetaEnginesTableNameColumnContent
192133
showRow={jest.fn()}
193134
hideRow={jest.fn()}
194-
name={'test-engine'}
195135
item={
196136
{
197137
name: 'test-engine',

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/meta_engines_table_name_column_content.tsx‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import { generateEncodedPath } from '../../../../utils/encode_path_params';
1717
import { EngineDetails } from '../../../engine/types';
1818

1919
interface MetaEnginesTableNameContentProps {
20-
name: string;
2120
isExpanded: boolean;
2221
item: EngineDetails;
2322
hideRow: (name: string) => void;
@@ -26,8 +25,7 @@ interface MetaEnginesTableNameContentProps {
2625
}
2726

2827
export const MetaEnginesTableNameColumnContent: React.FC<MetaEnginesTableNameContentProps> = ({
29-
name,
30-
item,
28+
item: { name, schemaConflicts, engine_count: engineCount },
3129
isExpanded,
3230
sendEngineTableLinkClickTelemetry,
3331
hideRow,
@@ -37,29 +35,30 @@ export const MetaEnginesTableNameColumnContent: React.FC<MetaEnginesTableNameCon
3735
<EuiLinkTo
3836
to={generateEncodedPath(ENGINE_PATH, { engineName: name })}
3937
onClick={sendEngineTableLinkClickTelemetry}
38+
data-test-subj="EngineName"
4039
>
4140
<strong>{name}</strong>
4241
</EuiLinkTo>
4342
<button
4443
type="button"
4544
onClick={() => (isExpanded ? hideRow(name) : showRow(name))}
4645
aria-expanded={isExpanded}
47-
className="meta-engines__expand-row"
46+
data-test-subj="ExpandRowButton"
4847
>
4948
<EuiFlexGroup alignItems="center" gutterSize="s" responsive={false}>
5049
<EuiFlexItem grow={false}>
5150
<EuiIcon type={isExpanded ? 'arrowDown' : 'arrowRight'} />
5251
</EuiFlexItem>
53-
<EuiFlexItem grow={false}>
52+
<EuiFlexItem grow={false} data-test-subj="SourceEnginesCount">
5453
{i18n.translate(
5554
'xpack.enterpriseSearch.appSearch.engines.metaEnginesTable.sourceEnginesCount',
5655
{
57-
defaultMessage: '{sourceEnginesCount, plural, one {# Engine} other {# Engines}}',
58-
values: { sourceEnginesCount: item.engine_count || 0 },
56+
defaultMessage: '{sourceEnginesCount, plural, one {# engine} other {# engines}}',
57+
values: { sourceEnginesCount: engineCount || 0 },
5958
}
6059
)}
6160
</EuiFlexItem>
62-
{item.schemaConflicts && Object.keys(item.schemaConflicts).length > 0 && (
61+
{schemaConflicts && Object.keys(schemaConflicts).length > 0 && (
6362
<EuiFlexItem grow={false}>
6463
<EuiHealth color="warning">
6564
{i18n.translate(

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/shared_columns.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export const NAME_COLUMN: EuiTableFieldDataColumnType<EngineDetails> = {
3939
export const CREATED_AT_COLUMN: EuiTableFieldDataColumnType<EngineDetails> = {
4040
field: 'created_at',
4141
name: i18n.translate('xpack.enterpriseSearch.appSearch.enginesOverview.table.column.createdAt', {
42-
defaultMessage: 'Created At',
42+
defaultMessage: 'Created at',
4343
}),
4444
dataType: 'string',
4545
render: (dateString: string) => <FormattedDateTime date={new Date(dateString)} hideTime />,
@@ -50,7 +50,7 @@ export const DOCUMENT_COUNT_COLUMN: EuiTableFieldDataColumnType<EngineDetails> =
5050
name: i18n.translate(
5151
'xpack.enterpriseSearch.appSearch.enginesOverview.table.column.documentCount',
5252
{
53-
defaultMessage: 'Document Count',
53+
defaultMessage: 'Document count',
5454
}
5555
),
5656
dataType: 'number',
@@ -61,7 +61,7 @@ export const DOCUMENT_COUNT_COLUMN: EuiTableFieldDataColumnType<EngineDetails> =
6161
export const FIELD_COUNT_COLUMN: EuiTableFieldDataColumnType<EngineDetails> = {
6262
field: 'field_count',
6363
name: i18n.translate('xpack.enterpriseSearch.appSearch.enginesOverview.table.column.fieldCount', {
64-
defaultMessage: 'Field Count',
64+
defaultMessage: 'Field count',
6565
}),
6666
dataType: 'number',
6767
render: (number: number) => <FormattedNumber value={number} />,

‎x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/tables/utils.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('getConflictingEnginesFromConflictingField', () => {
2525
it('returns a flat array of all engines with conflicts across different schema types, including duplicates', () => {
2626
const result = getConflictingEnginesFromConflictingField(CONFLICTING_FIELD);
2727

28-
// we can't guaruntee ordering
28+
// we can't guarantee ordering
2929
expect(result).toHaveLength(6);
3030
expect(result).toContain('source-engine-1');
3131
expect(result).toContain('source-engine-2');
@@ -55,7 +55,7 @@ describe('getConflictingEnginesFromSchemaConflicts', () => {
5555

5656
const result = getConflictingEnginesFromSchemaConflicts(SCHEMA_CONFLICTS);
5757

58-
// we can't guaruntee ordering
58+
// we can't guarantee ordering
5959
expect(result).toHaveLength(4);
6060
expect(result).toContain('source-engine-1');
6161
expect(result).toContain('source-engine-2');

0 commit comments

Comments
 (0)