Logs navigation: fix multiple incorrect calls to addResultsFromCache (#80307)

* Logs container: prevent unnecessary rerenders from arrow functions

* Logs navigation: refactor effects and calls to addResultsToCache

* Logs navigation pages: disable buttons while loading

* Logs navigation: add regression test

* Formatting
This commit is contained in:
Matias Chomicki
2024-01-11 00:52:43 +02:00
committed by GitHub
parent 752d788bd6
commit be12d3919f
4 changed files with 42 additions and 14 deletions
@@ -244,6 +244,14 @@ class LogsContainer extends PureComponent<LogsContainerProps, LogsContainerState
);
};
addResultsToCache = () => {
this.props.addResultsToCache(this.props.exploreId);
};
clearCache = () => {
this.props.clearCache(this.props.exploreId);
};
render() {
const {
loading,
@@ -267,8 +275,6 @@ class LogsContainer extends PureComponent<LogsContainerProps, LogsContainerState
splitOpenFn,
isLive,
exploreId,
addResultsToCache,
clearCache,
logsVolume,
scrollElement,
} = this.props;
@@ -330,8 +336,8 @@ class LogsContainer extends PureComponent<LogsContainerProps, LogsContainerState
getRowContextQuery={this.getLogRowContextQuery}
getLogRowContextUi={this.getLogRowContextUi}
getFieldLinks={this.getFieldLinks}
addResultsToCache={() => addResultsToCache(exploreId)}
clearCache={() => clearCache(exploreId)}
addResultsToCache={this.addResultsToCache}
clearCache={this.clearCache}
eventBus={this.props.eventBus}
panelState={this.props.panelState}
logsFrames={this.props.logsFrames}
@@ -138,4 +138,31 @@ describe('LogsNavigation', () => {
await userEvent.click(screen.getByTestId('olderLogsButton'));
expect(scrollToTopLogsMock).toHaveBeenCalled();
});
it('should not trigger actions while loading', async () => {
const scrollToTopLogs = jest.fn();
const changeTimeMock = jest.fn();
setup({ scrollToTopLogs, onChangeTime: changeTimeMock, loading: true });
expect(scrollToTopLogs).not.toHaveBeenCalled();
expect(changeTimeMock).not.toHaveBeenCalled();
await userEvent.click(screen.getByTestId('olderLogsButton'));
await userEvent.click(screen.getByTestId('newerLogsButton'));
expect(scrollToTopLogs).not.toHaveBeenCalled();
expect(changeTimeMock).not.toHaveBeenCalled();
});
it('should not add results to cache unless pagination is used', async () => {
const addResultsToCache = jest.fn();
setup({ addResultsToCache });
expect(addResultsToCache).not.toHaveBeenCalled();
expect(screen.getByTestId('olderLogsButton')).not.toBeDisabled();
expect(screen.getByTestId('newerLogsButton')).toBeDisabled();
await userEvent.click(screen.getByTestId('olderLogsButton'));
await userEvent.click(screen.getByTestId('newerLogsButton'));
expect(addResultsToCache).toHaveBeenCalledTimes(1);
});
});
@@ -82,21 +82,15 @@ function LogsNavigation({
return newPages;
});
}
addResultsToCache();
}, [visibleRange, absoluteRange, logsSortOrder, queries, clearCache, addResultsToCache]);
useEffect(() => {
clearCache();
// We can't enforce the eslint rule here because we only want to run when component is mounted.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
const changeTime = useCallback(
({ from, to }: AbsoluteTimeRange) => {
addResultsToCache();
expectedRangeRef.current = { from, to };
onChangeTime({ from, to });
},
[onChangeTime]
[onChangeTime, addResultsToCache]
);
const sortPages = (a: LogsPage, b: LogsPage, logsSortOrder?: LogsSortOrder | null) => {
@@ -173,10 +167,10 @@ function LogsNavigation({
pageType: 'page',
pageNumber,
});
!loading && changeTime({ from: page.queryRange.from, to: page.queryRange.to });
changeTime({ from: page.queryRange.from, to: page.queryRange.to });
scrollToTopLogs();
},
[changeTime, loading, scrollToTopLogs]
[changeTime, scrollToTopLogs]
);
return (
@@ -49,6 +49,7 @@ export function LogsNavigationPages({ pages, currentPageIndex, oldestLogsFirst,
onClick={() => {
onClick(page, index + 1);
}}
disabled={loading}
>
<div className={cx(styles.line, { selectedBg: currentPageIndex === index })} />
<div className={cx(styles.time, { selectedText: currentPageIndex === index })}>