Skip to content

Commit

Permalink
respect existing query params when opening or closing flyouyt
Browse files Browse the repository at this point in the history
disable test i broke. lets rethink this
  • Loading branch information
oatkiller committed Feb 20, 2020
1 parent 5a30168 commit 626e2fb
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,47 @@
import { Store, createStore, applyMiddleware } from 'redux';
import { History } from 'history';
import { alertListReducer } from './reducer';
import { AlertListState } from '../../types';
import { AlertListState, AlertingIndexUIQueryParams } from '../../types';
import { alertMiddlewareFactory } from './middleware';
import { AppAction } from '../action';
import { coreMock } from 'src/core/public/mocks';
import { createBrowserHistory } from 'history';
import { urlFromNewPageSizeParam, uiQueryParams, urlFromNewPageIndexParam } from './selectors';
import { uiQueryParams } from './selectors';
import { urlFromQueryParams } from '../../view/alerts/url_from_query_params';

describe('alert list pagination', () => {
let store: Store<AlertListState, AppAction>;
let coreStart: ReturnType<typeof coreMock.createStart>;
let history: History<never>;
let queryParams: () => AlertingIndexUIQueryParams;
/**
* Update the history with a new `AlertingIndexUIQueryParams`
*/
let historyPush: (params: AlertingIndexUIQueryParams) => void;
beforeEach(() => {
coreStart = coreMock.createStart();
history = createBrowserHistory();

const middleware = alertMiddlewareFactory(coreStart);
store = createStore(alertListReducer, applyMiddleware(middleware));

history.listen(location => {
store.dispatch({ type: 'userChangedUrl', payload: location });
});

queryParams = () => uiQueryParams(store.getState());

historyPush = (nextQueryParams: AlertingIndexUIQueryParams): void => {
return history.push(urlFromQueryParams(nextQueryParams));
};
});
describe('when the user navigates to the alert list page', () => {
describe('when a new page size is passed', () => {
beforeEach(() => {
const urlPageSizeSelector = urlFromNewPageSizeParam(store.getState());
history.push(urlPageSizeSelector(1));
store.dispatch({ type: 'userChangedUrl', payload: history.location });
historyPush({ ...queryParams(), page_size: '1' });
});
it('should modify the url correctly', () => {
const actualPaginationQuery = uiQueryParams(store.getState());
expect(actualPaginationQuery).toMatchInlineSnapshot(`
expect(queryParams()).toMatchInlineSnapshot(`
Object {
"page_size": "1",
}
Expand All @@ -42,13 +56,10 @@ describe('alert list pagination', () => {

describe('and then a new page index is passed', () => {
beforeEach(() => {
const urlPageIndexSelector = urlFromNewPageIndexParam(store.getState());
history.push(urlPageIndexSelector(1));
store.dispatch({ type: 'userChangedUrl', payload: history.location });
historyPush({ ...queryParams(), page_index: '1' });
});
it('should modify the url in the correct order', () => {
const actualPaginationQuery = uiQueryParams(store.getState());
expect(actualPaginationQuery).toMatchInlineSnapshot(`
expect(queryParams()).toMatchInlineSnapshot(`
Object {
"page_index": "1",
"page_size": "1",
Expand All @@ -60,28 +71,23 @@ describe('alert list pagination', () => {

describe('when a new page index is passed', () => {
beforeEach(() => {
const urlPageIndexSelector = urlFromNewPageIndexParam(store.getState());
history.push(urlPageIndexSelector(1));
store.dispatch({ type: 'userChangedUrl', payload: history.location });
historyPush({ ...queryParams(), page_index: '1' });
});
it('should modify the url correctly', () => {
const actualPaginationQuery = uiQueryParams(store.getState());
expect(actualPaginationQuery).toMatchInlineSnapshot(`
expect(queryParams()).toMatchInlineSnapshot(`
Object {
"page_index": "1",
}
`);
});

describe('and then a new page size is passed', () => {
// TODO, move this test to react land, since thats where this logic lives now. is that good?
xdescribe('and then a new page size is passed', () => {
beforeEach(() => {
const urlPageSizeSelector = urlFromNewPageSizeParam(store.getState());
history.push(urlPageSizeSelector(1));
store.dispatch({ type: 'userChangedUrl', payload: history.location });
historyPush({ ...queryParams(), page_size: '1' });
});
it('should modify the url correctly and reset index to `0`', () => {
const actualPaginationQuery = uiQueryParams(store.getState());
expect(actualPaginationQuery).toMatchInlineSnapshot(`
expect(queryParams()).toMatchInlineSnapshot(`
Object {
"page_size": "1",
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import qs from 'querystring';
import querystring from 'querystring';
import {
createSelector,
createStructuredSelector as createStructuredSelectorWithBadType,
Expand Down Expand Up @@ -52,7 +52,7 @@ export const uiQueryParams: (
const data: AlertingIndexUIQueryParams = {};
if (location) {
// Removes the `?` from the beginning of query string if it exists
const query = qs.parse(location.search.slice(1));
const query = querystring.parse(location.search.slice(1));

/**
* Build an AlertingIndexUIQueryParams object with keys from the query.
Expand Down Expand Up @@ -89,64 +89,6 @@ export const apiQueryParams: (
})
);

/**
* Returns a function that takes in a new page size and returns a new query param string
*/
export const urlFromNewPageSizeParam: (
state: AlertListState
) => (newPageSize: number) => string = createSelector(uiQueryParams, paramData => {
return newPageSize => {
const queryParams: AlertingIndexUIQueryParams = { ...paramData };
queryParams.page_size = newPageSize.toString();

/**
* Reset the page index when changing page size.
*/
if (queryParams.page_index !== undefined) {
delete queryParams.page_index;
}
return '?' + qs.stringify(queryParams);
};
});

/**
* Returns a function that takes in a new page index and returns a new query param string
*/
export const urlFromNewPageIndexParam: (
state: AlertListState
) => (newPageIndex: number) => string = createSelector(uiQueryParams, paramData => {
return newPageIndex => {
const queryParams: AlertingIndexUIQueryParams = { ...paramData };
queryParams.page_index = newPageIndex.toString();
return '?' + qs.stringify(queryParams);
};
});

/**
* Returns a url like the current one, but with a new alert id.
*/
export const urlWithSelectedAlert: (
state: AlertListState
) => (alertID: string) => string = createSelector(uiQueryParams, paramData => {
return (alertID: string) => {
const queryParams = { ...paramData };
queryParams.selected_alert = alertID;
return '?' + qs.stringify(queryParams);
};
});

/**
* Returns a url like the current one, but with no alert id
*/
export const urlWithoutSelectedAlert: (state: AlertListState) => string = createSelector(
uiQueryParams,
urlPaginationData => {
const queryParams = { ...urlPaginationData };
delete queryParams.selected_alert;
return '?' + qs.stringify(queryParams);
}
);

export const hasSelectedAlert: (state: AlertListState) => boolean = createSelector(
uiQueryParams,
({ selected_alert: selectedAlert }) => selectedAlert !== undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import { i18n } from '@kbn/i18n';
import { useHistory, Link } from 'react-router-dom';
import { FormattedDate } from 'react-intl';
import { urlFromQueryParams } from './url_from_query_params';
import { AlertData } from '../../../../../common/types';
import * as selectors from '../../store/alerts/selectors';
import { useAlertListSelector } from './hooks/use_alerts_selector';
Expand Down Expand Up @@ -82,28 +83,39 @@ export const AlertIndex = memo(() => {
}, []);

const { pageIndex, pageSize, total } = useAlertListSelector(selectors.alertListPagination);
const urlFromNewPageSizeParam = useAlertListSelector(selectors.urlFromNewPageSizeParam);
const urlWithSelectedAlert = useAlertListSelector(selectors.urlWithSelectedAlert);
const urlWithoutSelectedAlert = useAlertListSelector(selectors.urlWithoutSelectedAlert);
const urlFromNewPageIndexParam = useAlertListSelector(selectors.urlFromNewPageIndexParam);
const alertListData = useAlertListSelector(selectors.alertListData);
const hasSelectedAlert = useAlertListSelector(selectors.hasSelectedAlert);
const queryParams = useAlertListSelector(selectors.uiQueryParams);

const onChangeItemsPerPage = useCallback(
newPageSize => history.push(urlFromNewPageSizeParam(newPageSize)),
[history, urlFromNewPageSizeParam]
newPageSize => {
const newQueryParms = { ...queryParams };
newQueryParms.page_size = newPageSize;
delete newQueryParms.page_index;
const relativeURL = urlFromQueryParams(newQueryParms);
return history.push(relativeURL);
},
[history, queryParams]
);

const onChangePage = useCallback(
newPageIndex => history.push(urlFromNewPageIndexParam(newPageIndex)),
[history, urlFromNewPageIndexParam]
newPageIndex => {
return history.push(
urlFromQueryParams({
...queryParams,
page_index: newPageIndex,
})
);
},
[history, queryParams]
);

const [visibleColumns, setVisibleColumns] = useState(() => columns.map(({ id }) => id));

const handleFlyoutClose = useCallback(() => {
history.push(urlWithoutSelectedAlert);
}, [history, urlWithoutSelectedAlert]);
const { selected_alert, ...paramsWithoutSelectedAlert } = queryParams;
history.push(urlFromQueryParams(paramsWithoutSelectedAlert));
}, [history, queryParams]);

const datesForRows: Map<AlertData, Date> = useMemo(() => {
return new Map(
Expand All @@ -123,7 +135,10 @@ export const AlertIndex = memo(() => {

if (columnId === 'alert_type') {
return (
<Link data-testid="alertTypeCellLink" to={urlWithSelectedAlert('TODO')}>
<Link
data-testid="alertTypeCellLink"
to={urlFromQueryParams({ ...queryParams, selected_alert: 'TODO' })}
>
{i18n.translate(
'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription',
{
Expand Down Expand Up @@ -173,7 +188,7 @@ export const AlertIndex = memo(() => {
}
return null;
};
}, [alertListData, datesForRows, pageSize, total, urlWithSelectedAlert]);
}, [alertListData, datesForRows, pageSize, queryParams, total]);

const pagination = useMemo(() => {
return {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import querystring from 'querystring';
import { AlertingIndexUIQueryParams } from '../../types';

/**
* Return a relative URL for `AlertingIndexUIQueryParams`.
* usage:
*
* ```ts
* // Replace this with however you get state, e.g. useSelector in react
* const queryParams = selectors.uiQueryParams(store.getState())
*
* // same as current url, but page_index is now 3
* const relativeURL = urlFromQueryParams({ ...queryParams, page_index: 3 })
*
* // now use relativeURL in the 'href' of a link, the 'to' of a react-router-dom 'Link' or history.push, history.replace
* ```
*/
export function urlFromQueryParams(queryParams: AlertingIndexUIQueryParams): string {
const search = querystring.stringify(queryParams);
return search === '' ? '' : '?' + search;
}

0 comments on commit 626e2fb

Please sign in to comment.