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 pagination bugs in CCR and Remote Clusters #65931

Merged
merged 6 commits into from
May 14, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,50 @@ describe('<AutoFollowPatternList />', () => {
});
});

describe('when there are multiple pages of auto-follow patterns', () => {
let find;
let component;
let table;
let actions;
let form;

const autoFollowPatterns = [
getAutoFollowPatternMock({ name: 'unique', followPattern: '{{leader_index}}' }),
];

for (let i = 0; i < 29; i++) {
autoFollowPatterns.push(
getAutoFollowPatternMock({ name: `${i}`, followPattern: '{{leader_index}}' })
);
}

beforeEach(async () => {
httpRequestsMockHelpers.setLoadAutoFollowPatternsResponse({ patterns: autoFollowPatterns });

// Mount the component
({ find, component, table, actions, form } = setup());

await nextTick(); // Make sure that the http request is fulfilled
component.update();
});

test('pagination works', () => {
actions.clickPaginationNextButton();
const { tableCellsValues } = table.getMetaData('autoFollowPatternListTable');

// Pagination defaults to 20 auto-follow patterns per page. We loaded 30 auto-follow patterns,
// so the second page should have 10.
expect(tableCellsValues.length).toBe(10);
});

// Skipped until we can figure out how to get this test to work.
test.skip('search works', () => {
form.setInputValue(find('autoFollowPatternSearch'), 'unique');
const { tableCellsValues } = table.getMetaData('autoFollowPatternListTable');
expect(tableCellsValues.length).toBe(1);
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sebelga There are two other search tests like this one in the other tests files, and they are all failing based on the tableCellsValues.length remaining unchanged at 20. It looks like the input is being found and the change event is simulated, but it's not bubbling back up to the list component (which is responsible for handling it and updating the array of filtered items). Any ideas on what could be causing this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have your tried adding component.update() after form.setInputValue()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you decided to use component.find('input[type="search"]') instead of using a data-test-subj?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have your tried adding component.update() after form.setInputValue()?

Yup, no change.

Is there a reason why you decided to use component.find('input[type="search"]') instead of using a data-test-subj?

Fixed! I didn't realize this was possible until you suggested it and I looked at the EUI search bar config.

});

describe('when there are auto-follow patterns', () => {
let find;
let exists;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
* you may not use this file except in compliance with the Elastic License.
*/

/**
* The below import is required to avoid a console error warn from brace package
* console.warn ../node_modules/brace/index.js:3999
Could not load worker ReferenceError: Worker is not defined
at createWorker (/<path-to-repo>/node_modules/brace/index.js:17992:5)
*/
import * as stubWebWorker from '../../../../../test_utils/stub_web_worker'; // eslint-disable-line no-unused-vars

import { getFollowerIndexMock } from './fixtures/follower_index';
import './mocks';
import { setupEnvironment, pageHelpers, nextTick, getRandomString } from './helpers';
Expand Down Expand Up @@ -59,6 +67,54 @@ describe('<FollowerIndicesList />', () => {
});
});

describe('when there are multiple pages of follower indices', () => {
let find;
let component;
let table;
let actions;
let form;

const followerIndices = [
{
name: 'unique',
seeds: [],
},
];

for (let i = 0; i < 29; i++) {
followerIndices.push({
name: `name${i}`,
seeds: [],
});
}

beforeEach(async () => {
httpRequestsMockHelpers.setLoadFollowerIndicesResponse({ indices: followerIndices });

// Mount the component
({ find, component, table, actions, form } = setup());

await nextTick(); // Make sure that the http request is fulfilled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my notes and finding, we should not add nextTick() in our tests anymore but use jest.useFakeTimers()

component.update();
});

test('pagination works', () => {
actions.clickPaginationNextButton();
const { tableCellsValues } = table.getMetaData('followerIndexListTable');

// Pagination defaults to 20 follower indices per page. We loaded 30 follower indices,
// so the second page should have 10.
expect(tableCellsValues.length).toBe(10);
});

// Skipped until we can figure out how to get this test to work.
test.skip('search works', () => {
form.setInputValue(find('followerIndexSearch'), 'unique');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably do this

act(() => {
  form.setInputValue(find('followerIndexSearch'), 'unique');
});
component.update();
...

const { tableCellsValues } = table.getMetaData('followerIndexListTable');
expect(tableCellsValues.length).toBe(1);
});
});

describe('when there are follower indices', () => {
let find;
let exists;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ export const setup = props => {
autoFollowPatternLink.simulate('click');
};

const clickPaginationNextButton = () => {
testBed.find('autoFollowPatternListTable.pagination-button-next').simulate('click');
};

return {
...testBed,
actions: {
Expand All @@ -94,6 +98,7 @@ export const setup = props => {
clickAutoFollowPatternAt,
getPatternsActionMenuItemText,
clickPatternsActionMenuItem,
clickPaginationNextButton,
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ export const setup = props => {
followerIndexLink.simulate('click');
};

const clickPaginationNextButton = () => {
testBed.find('followerIndexListTable.pagination-button-next').simulate('click');
};

return {
...testBed,
actions: {
Expand All @@ -72,6 +76,7 @@ export const setup = props => {
clickContextMenuButtonAt,
openTableRowContextMenuAt,
clickFollowerIndexAt,
clickPaginationNextButton,
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,30 @@ import {
import { routing } from '../../../../../services/routing';
import { trackUiMetric } from '../../../../../services/track_ui_metric';

const getFilteredPatterns = (autoFollowPatterns, queryText) => {
if (queryText) {
const normalizedSearchText = queryText.toLowerCase();

return autoFollowPatterns.filter(autoFollowPattern => {
const {
name,
remoteCluster,
followIndexPatternPrefix,
followIndexPatternSuffix,
} = autoFollowPattern;

const inName = name.toLowerCase().includes(normalizedSearchText);
const inRemoteCluster = remoteCluster.toLowerCase().includes(normalizedSearchText);
const inPrefix = followIndexPatternPrefix.toLowerCase().includes(normalizedSearchText);
const inSuffix = followIndexPatternSuffix.toLowerCase().includes(normalizedSearchText);

return inName || inRemoteCluster || inPrefix || inSuffix;
});
}

return autoFollowPatterns;
};

export class AutoFollowPatternTable extends PureComponent {
static propTypes = {
autoFollowPatterns: PropTypes.array,
Expand All @@ -31,41 +55,42 @@ export class AutoFollowPatternTable extends PureComponent {
resumeAutoFollowPattern: PropTypes.func.isRequired,
};

state = {
selectedItems: [],
};
static getDerivedStateFromProps(props, state) {
const { autoFollowPatterns } = props;
const { prevAutoFollowPatterns, queryText } = state;

onSearch = ({ query }) => {
const { text } = query;
const normalizedSearchText = text.toLowerCase();
this.setState({
queryText: normalizedSearchText,
});
};
// If an auto-follow pattern gets deleted, we need to recreate the cached filtered auto-follow patterns.
if (prevAutoFollowPatterns !== autoFollowPatterns) {
return {
prevAutoFollowPatterns: autoFollowPatterns,
filteredAutoFollowPatterns: getFilteredPatterns(autoFollowPatterns, queryText),
};
}

getFilteredPatterns = () => {
const { autoFollowPatterns } = this.props;
const { queryText } = this.state;
return null;
}

if (queryText) {
return autoFollowPatterns.filter(autoFollowPattern => {
const {
name,
remoteCluster,
followIndexPatternPrefix,
followIndexPatternSuffix,
} = autoFollowPattern;
constructor(props) {
super(props);

const inName = name.toLowerCase().includes(queryText);
const inRemoteCluster = remoteCluster.toLowerCase().includes(queryText);
const inPrefix = followIndexPatternPrefix.toLowerCase().includes(queryText);
const inSuffix = followIndexPatternSuffix.toLowerCase().includes(queryText);
this.state = {
prevAutoFollowPatterns: props.autoFollowPatterns,
selectedItems: [],
filteredAutoFollowPatterns: props.autoFollowPatterns,
queryText: '',
};
}

return inName || inRemoteCluster || inPrefix || inSuffix;
});
}
onSearch = ({ query }) => {
const { autoFollowPatterns } = this.props;
const { text } = query;

return autoFollowPatterns.slice(0);
// We cache the filtered indices instead of calculating them inside render() because
// of https://github.com/elastic/eui/issues/3445.
this.setState({
queryText: text,
filteredAutoFollowPatterns: getFilteredPatterns(autoFollowPatterns, text),
});
};

getTableColumns() {
Expand Down Expand Up @@ -144,7 +169,7 @@ export class AutoFollowPatternTable extends PureComponent {
defaultMessage: 'Leader patterns',
}
),
render: leaderPatterns => leaderPatterns.join(', '),
render: leaderIndexPatterns => leaderIndexPatterns.join(', '),
},
{
field: 'followIndexPatternPrefix',
Expand Down Expand Up @@ -278,7 +303,7 @@ export class AutoFollowPatternTable extends PureComponent {
};

render() {
const { selectedItems } = this.state;
const { selectedItems, filteredAutoFollowPatterns } = this.state;

const sorting = {
sort: {
Expand All @@ -297,27 +322,28 @@ export class AutoFollowPatternTable extends PureComponent {
this.setState({ selectedItems: selectedItems.map(({ name }) => name) }),
};

const items = this.getFilteredPatterns();

const search = {
toolsLeft: selectedItems.length ? (
<AutoFollowPatternActionMenu
arrowDirection="down"
patterns={this.state.selectedItems.map(name => items.find(item => item.name === name))}
patterns={this.state.selectedItems.map(name =>
filteredAutoFollowPatterns.find(item => item.name === name)
)}
/>
) : (
undefined
),
onChange: this.onSearch,
box: {
incremental: true,
'data-test-subj': 'autoFollowPatternSearch',
},
};

return (
<Fragment>
<EuiInMemoryTable
items={items}
items={filteredAutoFollowPatterns}
itemId="name"
columns={this.getTableColumns()}
search={search}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const scope = SECTIONS.FOLLOWER_INDEX;
const mapStateToProps = state => ({
apiStatusDelete: getApiStatus(`${scope}-delete`)(state),
});
//

const mapDispatchToProps = dispatch => ({
selectFollowerIndex: name => dispatch(selectDetailFollowerIndex(name)),
});
Expand Down
Loading