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

[Security Solutions][Endpoint] Fixes weird 'flash' when entries does not exists on event filters page #100203

Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -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(),
paul-tavares marked this conversation as resolved.
Show resolved Hide resolved
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably use the utility method we have to get the previous state out of an AsyncResource.

}

// 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 },
});
};