Skip to content

Commit

Permalink
Fixes weird 'flash' when entries does not exists on event filters pag…
Browse files Browse the repository at this point in the history
…e. Also fixes a multilang and query when empty string
  • Loading branch information
dasansol92 committed May 17, 2021
1 parent c4529fa commit ea600ed
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,7 @@ export const PaginatedContent = memo(
return <Item {...itemComponentProps(item)} key={key} />;
});
}

return noItemsMessage || <DefaultNoItemsFound />;
if (!loading) return noItemsMessage || <DefaultNoItemsFound />;
}, [
ItemComponent,
error,
Expand All @@ -214,6 +213,7 @@ export const PaginatedContent = memo(
itemKeys,
items,
noItemsMessage,
loading,
]);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { MANAGEMENT_DEFAULT_PAGE, MANAGEMENT_DEFAULT_PAGE_SIZE } from '../../../common/constants';
import { EventFiltersListPageState } from '../types';
import { createLoadedResourceState, createUninitialisedResourceState } from '../../../state';
import { createUninitialisedResourceState } from '../../../state';

export const initialEventFiltersPageState = (): EventFiltersListPageState => ({
entries: [],
Expand All @@ -29,7 +29,7 @@ export const initialEventFiltersPageState = (): EventFiltersListPageState => ({
forceRefresh: false,
data: createUninitialisedResourceState(),
/** We started off assuming data exists, until we can confirm otherwise */
dataExist: createLoadedResourceState(true),
dataExist: createUninitialisedResourceState(),
deletion: {
item: undefined,
status: createUninitialisedResourceState(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,24 +254,24 @@ const refreshListDataIfNeeded: MiddlewareActionHandler = async (store, eventFilt
dispatch({
type: 'eventFiltersListPageDataChanged',
payload: createLoadedResourceState({
query,
query: { ...query, filter },
content: results,
}),
});

dispatch({
type: 'eventFiltersListPageDataExistsChanged',
payload: {
type: 'LoadedResourceState',
data: Boolean(results.total),
},
});

// If no results were returned, then just check to make sure data actually exists for
// event filters. This is used to drive the UI between showing "empty state" and "no items found"
// messages to the user
if (results.total === 0) {
await checkIfEventFilterDataExist(store, eventFiltersService);
} else {
dispatch({
type: 'eventFiltersListPageDataExistsChanged',
payload: {
type: 'LoadedResourceState',
data: Boolean(results.total),
},
});
}
} catch (error) {
dispatch({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,24 +88,31 @@ export const getListFetchError: EventFiltersSelector<
return (isFailedResourceState(listPageDataState) && listPageDataState.error) || undefined;
});

export const getListIsLoading: EventFiltersSelector<boolean> = createSelector(
getCurrentListPageDataState,
(listDataState) => isLoadingResourceState(listDataState)
);

export const getListPageDataExistsState: EventFiltersSelector<
StoreState['listPage']['dataExist']
> = ({ listPage: { dataExist } }) => dataExist;

export const getListIsLoading: EventFiltersSelector<boolean> = createSelector(
getCurrentListPageDataState,
getListPageDataExistsState,
(listDataState, dataExists) =>
isLoadingResourceState(listDataState) || isLoadingResourceState(dataExists)
);

export const getListPageDoesDataExist: EventFiltersSelector<boolean> = createSelector(
getListPageDataExistsState,
(dataExistsState) => {
if (isLoadedResourceState(dataExistsState)) {
return dataExistsState.data;
}
if (isLoadingResourceState(dataExistsState)) {
// Ignore will be fixed with when AsyncResourceState is refactored (#830)
// @ts-ignore
return dataExistsState.previousState.data;
}

// Until we know for sure that data exists (LoadedState), we assume `true`
return true;
// Until we know for sure that data exists (LoadedState), we assume `false`
return false;
}
);

Expand Down Expand Up @@ -179,7 +186,7 @@ export const listDataNeedsRefresh: EventFiltersSelector<boolean> = createSelecto
forceRefresh ||
location.page_index + 1 !== currentQuery.page ||
location.page_size !== currentQuery.perPage ||
(!!location.filter && location.filter !== currentQuery.filter)
location.filter !== currentQuery.filter
);
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,22 +186,22 @@ describe('event filters selectors', () => {
});

describe('getListPageDoesDataExist()', () => {
it('should return true (default) until we get a Loaded Resource state', () => {
expect(getListPageDoesDataExist(initialState)).toBe(true);
it('should return false (default) until we get a Loaded Resource state', () => {
expect(getListPageDoesDataExist(initialState)).toBe(false);

// Set DataExists to Loading
// ts-ignore will be fixed when AsyncResourceState is refactored (#830)
// @ts-ignore
initialState.listPage.dataExist = createLoadingResourceState(initialState.listPage.dataExist);
expect(getListPageDoesDataExist(initialState)).toBe(true);
expect(getListPageDoesDataExist(initialState)).toBe(false);

// Set DataExists to Failure
initialState.listPage.dataExist = createFailedResourceState({
statusCode: 500,
error: 'Internal Server Error',
message: 'Something is not right',
});
expect(getListPageDoesDataExist(initialState)).toBe(true);
expect(getListPageDoesDataExist(initialState)).toBe(false);
});

it('should return false if no data exists', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const getCreationSuccessMessage = (
entry: CreateExceptionListItemSchema | UpdateExceptionListItemSchema | undefined
) => {
return i18n.translate('xpack.securitySolution.eventFilter.form.creationSuccessToastTitle', {
defaultMessage: '"{name}" has been added to the event exceptions list.',
defaultMessage: '"{name}" has been added to the event filters list.',
values: { name: entry?.name },
});
};
Expand All @@ -33,21 +33,21 @@ export const getUpdateSuccessMessage = (

export const getCreationErrorMessage = (creationError: ServerApiError) => {
return i18n.translate('xpack.securitySolution.eventFilter.form.failedToastTitle.create', {
defaultMessage: 'There was an error creating the new exception: "{error}"',
defaultMessage: 'There was an error creating the new event filter: "{error}"',
values: { error: creationError.message },
});
};

export const getUpdateErrorMessage = (updateError: ServerApiError) => {
return i18n.translate('xpack.securitySolution.eventFilter.form.failedToastTitle.update', {
defaultMessage: 'There was an error updating the exception: "{error}"',
defaultMessage: 'There was an error updating the event filter: "{error}"',
values: { error: updateError.message },
});
};

export const getGetErrorMessage = (getError: ServerApiError) => {
return i18n.translate('xpack.securitySolution.eventFilter.form.failedToastTitle.get', {
defaultMessage: 'Unable to edit trusted application: "{error}"',
defaultMessage: 'Unable to edit event filter: "{error}"',
values: { error: getError.message },
});
};

0 comments on commit ea600ed

Please sign in to comment.