Skip to content

Commit

Permalink
Feat: back navigation support throughout the app (#6701)
Browse files Browse the repository at this point in the history
* feat: custom hook to prevent redundant navigation and handle default params with URL comparison

* feat: implement useSafeNavigation to QB, to ensure that the back navigation works properly

* fix: handle syncing the relativeTime param with the time picker selected relative time

* feat: add support for absolute and relative time sync with time picker component

* refactor: integrate safeNavigate in LogsExplorerChart and deprecate the existing back navigation

* feat: update pagination query params on pressing next/prev page

* fix: fix the issue of NOOP getting converted to Count on coming back from alert creation page

* refactor: replace history navigation with safeNavigate in DateTimeSelectionV2 component

it also fixes the issue of relativeTime not being added to the url on mounting

* feat: integrate useSafeNavigate across service details tabs

* fix: fix duplicate redirections by converting the timestamp to milliseconds

* fix: replace history navigation with useSafeNavigate in LogsExplorerViews and useUrlQueryData

* fix: replace history navigation with useSafeNavigate across dashboard components

* fix: use safeNavigate in alert components

* fix: fix the issue of back navigation in alert table and sync the pagination with url param

* fix: handle back navigation for resource filter and sync the state with url query

* fix: fix the issue of double redirection from top operations to traces

* fix: replace history.push with safeNavigate in TracesExplorer's updateDashboard

* fix: prevent unnecessary query re-runs by checking stagedQuery before redirecting in NewWidget

* chore: cleanup

* fix: fix the failing tests

* fix: fix the documentation redirection failing tests

* test: mock useSafeNavigate hook in WidgetGraphComponent test

* test: mock useSafeNavigate hook in ExplorerCard test
  • Loading branch information
ahmadshaheer authored Feb 14, 2025
1 parent ef635b6 commit 82d84c0
Show file tree
Hide file tree
Showing 50 changed files with 564 additions and 132 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ jest.mock('react-router-dom', () => ({
}),
}));

jest.mock('hooks/useSafeNavigate', () => ({
useSafeNavigate: (): any => ({
safeNavigate: jest.fn(),
}),
}));

jest.mock('hooks/queryBuilder/useGetPanelTypesQueryParam', () => ({
useGetPanelTypesQueryParam: jest.fn(() => 'mockedPanelType'),
}));
Expand Down
29 changes: 29 additions & 0 deletions frontend/src/components/ResizeTable/DynamicColumnTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { ColumnGroupType, ColumnType } from 'antd/es/table';
import { ColumnsType } from 'antd/lib/table';
import logEvent from 'api/common/logEvent';
import LaunchChatSupport from 'components/LaunchChatSupport/LaunchChatSupport';
import { useSafeNavigate } from 'hooks/useSafeNavigate';
import useUrlQuery from 'hooks/useUrlQuery';
import { SlidersHorizontal } from 'lucide-react';
import { memo, useEffect, useState } from 'react';
import { popupContainer } from 'utils/selectPopupContainer';
Expand All @@ -25,8 +27,12 @@ function DynamicColumnTable({
onDragColumn,
facingIssueBtn,
shouldSendAlertsLogEvent,
pagination,
...restProps
}: DynamicColumnTableProps): JSX.Element {
const { safeNavigate } = useSafeNavigate();
const urlQuery = useUrlQuery();

const [columnsData, setColumnsData] = useState<ColumnsType | undefined>(
columns,
);
Expand Down Expand Up @@ -93,6 +99,28 @@ function DynamicColumnTable({
type: 'checkbox',
})) || [];

// Get current page from URL or default to 1
const currentPage = Number(urlQuery.get('page')) || 1;

const handlePaginationChange = (page: number, pageSize?: number): void => {
// Update URL with new page number while preserving other params
urlQuery.set('page', page.toString());

const newUrl = `${window.location.pathname}?${urlQuery.toString()}`;
safeNavigate(newUrl);

// Call original pagination handler if provided
if (pagination?.onChange && !!pageSize) {
pagination.onChange(page, pageSize);
}
};

const enhancedPagination = {
...pagination,
current: currentPage, // Ensure the pagination component shows the correct page
onChange: handlePaginationChange,
};

return (
<div className="DynamicColumnTable">
<Flex justify="flex-end" align="center" gap={8}>
Expand All @@ -116,6 +144,7 @@ function DynamicColumnTable({
<ResizeTable
columns={columnsData}
onDragColumn={onDragColumn}
pagination={enhancedPagination}
{...restProps}
/>
</div>
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/components/ResizeTable/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { TableProps } from 'antd';
import { ColumnsType } from 'antd/es/table';
import { PaginationProps } from 'antd/lib';
import { ColumnGroupType, ColumnType } from 'antd/lib/table';
import { LaunchChatSupportProps } from 'components/LaunchChatSupport/LaunchChatSupport';

Expand All @@ -15,6 +16,7 @@ export interface DynamicColumnTableProps extends TableProps<any> {
onDragColumn?: (fromIndex: number, toIndex: number) => void;
facingIssueBtn?: LaunchChatSupportProps;
shouldSendAlertsLogEvent?: boolean;
pagination?: PaginationProps;
}

export type GetVisibleColumnsFunction = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ jest.mock('uplot', () => {
};
});

jest.mock('hooks/useSafeNavigate', () => ({
useSafeNavigate: (): any => ({
safeNavigate: jest.fn(),
}),
}));

let mockWindowOpen: jest.Mock;

window.ResizeObserver =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ window.ResizeObserver =
unobserve: jest.fn(),
}));

jest.mock('hooks/useSafeNavigate', () => ({
useSafeNavigate: (): any => ({
safeNavigate: jest.fn(),
}),
}));
describe('Anomaly Alert Documentation Redirection', () => {
let mockWindowOpen: jest.Mock;

Expand Down
12 changes: 6 additions & 6 deletions frontend/src/container/FormAlertRules/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import { BuilderUnitsFilter } from 'container/QueryBuilder/filters';
import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder';
import { useShareBuilderUrl } from 'hooks/queryBuilder/useShareBuilderUrl';
import { useNotifications } from 'hooks/useNotifications';
import { useSafeNavigate } from 'hooks/useSafeNavigate';
import useUrlQuery from 'hooks/useUrlQuery';
import history from 'lib/history';
import { mapQueryDataFromApi } from 'lib/newQueryBuilder/queryBuilderMappers/mapQueryDataFromApi';
import { mapQueryDataToApi } from 'lib/newQueryBuilder/queryBuilderMappers/mapQueryDataToApi';
import { isEqual } from 'lodash-es';
Expand Down Expand Up @@ -87,7 +87,7 @@ function FormAlertRules({
// init namespace for translations
const { t } = useTranslation('alerts');
const { featureFlags } = useAppContext();

const { safeNavigate } = useSafeNavigate();
const { selectedTime: globalSelectedInterval } = useSelector<
AppState,
GlobalReducer
Expand Down Expand Up @@ -224,7 +224,7 @@ function FormAlertRules({

const generatedUrl = `${location.pathname}?${queryParams.toString()}`;

history.replace(generatedUrl);
safeNavigate(generatedUrl);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [detectionMethod]);

Expand Down Expand Up @@ -295,8 +295,8 @@ function FormAlertRules({
urlQuery.delete(QueryParams.panelTypes);
urlQuery.delete(QueryParams.ruleId);
urlQuery.delete(QueryParams.relativeTime);
history.replace(`${ROUTES.LIST_ALL_ALERT}?${urlQuery.toString()}`);
}, [urlQuery]);
safeNavigate(`${ROUTES.LIST_ALL_ALERT}?${urlQuery.toString()}`);
}, [safeNavigate, urlQuery]);

// onQueryCategoryChange handles changes to query category
// in state as well as sets additional defaults
Expand Down Expand Up @@ -515,7 +515,7 @@ function FormAlertRules({
urlQuery.delete(QueryParams.panelTypes);
urlQuery.delete(QueryParams.ruleId);
urlQuery.delete(QueryParams.relativeTime);
history.replace(`${ROUTES.LIST_ALL_ALERT}?${urlQuery.toString()}`);
safeNavigate(`${ROUTES.LIST_ALL_ALERT}?${urlQuery.toString()}`);
}, 2000);
} else {
logData = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import {
import PanelWrapper from 'container/PanelWrapper/PanelWrapper';
import { useGetQueryRange } from 'hooks/queryBuilder/useGetQueryRange';
import { useChartMutable } from 'hooks/useChartMutable';
import { useSafeNavigate } from 'hooks/useSafeNavigate';
import useUrlQuery from 'hooks/useUrlQuery';
import { getDashboardVariables } from 'lib/dashbaordVariables/getDashboardVariables';
import { GetQueryResultsProps } from 'lib/dashboard/getQueryResults';
import GetMinMax from 'lib/getMinMax';
import history from 'lib/history';
import { useDashboard } from 'providers/Dashboard/Dashboard';
import { useCallback, useEffect, useRef, useState } from 'react';
import { useDispatch, useSelector } from 'react-redux';
Expand All @@ -49,6 +49,7 @@ function FullView({
isDependedDataLoaded = false,
onToggleModelHandler,
}: FullViewProps): JSX.Element {
const { safeNavigate } = useSafeNavigate();
const { selectedTime: globalSelectedTime } = useSelector<
AppState,
GlobalReducer
Expand Down Expand Up @@ -137,9 +138,9 @@ function FullView({
urlQuery.set(QueryParams.startTime, minTime.toString());
urlQuery.set(QueryParams.endTime, maxTime.toString());
const generatedUrl = `${location.pathname}?${urlQuery.toString()}`;
history.push(generatedUrl);
safeNavigate(generatedUrl);
},
[dispatch, location.pathname, urlQuery],
[dispatch, location.pathname, safeNavigate, urlQuery],
);

const [graphsVisibilityStates, setGraphsVisibilityStates] = useState<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ jest.mock('react-router-dom', () => ({
}),
}));

jest.mock('hooks/useSafeNavigate', () => ({
useSafeNavigate: (): any => ({
safeNavigate: jest.fn(),
}),
}));

jest.mock('uplot', () => {
const paths = {
spline: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import { placeWidgetAtBottom } from 'container/NewWidget/utils';
import PanelWrapper from 'container/PanelWrapper/PanelWrapper';
import { useUpdateDashboard } from 'hooks/dashboard/useUpdateDashboard';
import { useNotifications } from 'hooks/useNotifications';
import { useSafeNavigate } from 'hooks/useSafeNavigate';
import useUrlQuery from 'hooks/useUrlQuery';
import createQueryParams from 'lib/createQueryParams';
import history from 'lib/history';
import { RowData } from 'lib/query/createTableColumnsFromQuery';
import { useDashboard } from 'providers/Dashboard/Dashboard';
import {
Expand Down Expand Up @@ -51,6 +51,7 @@ function WidgetGraphComponent({
customSeries,
customErrorMessage,
}: WidgetGraphComponentProps): JSX.Element {
const { safeNavigate } = useSafeNavigate();
const [deleteModal, setDeleteModal] = useState(false);
const [hovered, setHovered] = useState(false);
const { notifications } = useNotifications();
Expand Down Expand Up @@ -173,7 +174,7 @@ function WidgetGraphComponent({
graphType: widget?.panelTypes,
widgetId: uuid,
};
history.push(`${pathname}/new?${createQueryParams(queryParams)}`);
safeNavigate(`${pathname}/new?${createQueryParams(queryParams)}`);
},
},
);
Expand All @@ -194,7 +195,7 @@ function WidgetGraphComponent({
const separator = existingSearch.toString() ? '&' : '';
const newSearch = `${existingSearch}${separator}${updatedSearch}`;

history.push({
safeNavigate({
pathname,
search: newSearch,
});
Expand All @@ -221,7 +222,7 @@ function WidgetGraphComponent({
});
setGraphVisibility(localStoredVisibilityState);
}
history.push({
safeNavigate({
pathname,
search: createQueryParams(updatedQueryParams),
});
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/container/GridCardLayout/GridCardLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import { useUpdateDashboard } from 'hooks/dashboard/useUpdateDashboard';
import useComponentPermission from 'hooks/useComponentPermission';
import { useIsDarkMode } from 'hooks/useDarkMode';
import { useNotifications } from 'hooks/useNotifications';
import { useSafeNavigate } from 'hooks/useSafeNavigate';
import useUrlQuery from 'hooks/useUrlQuery';
import history from 'lib/history';
import { defaultTo, isUndefined } from 'lodash-es';
import isEqual from 'lodash-es/isEqual';
import {
Expand Down Expand Up @@ -55,6 +55,7 @@ interface GraphLayoutProps {
// eslint-disable-next-line sonarjs/cognitive-complexity
function GraphLayout(props: GraphLayoutProps): JSX.Element {
const { handle } = props;
const { safeNavigate } = useSafeNavigate();
const {
selectedDashboard,
layouts,
Expand Down Expand Up @@ -215,13 +216,13 @@ function GraphLayout(props: GraphLayoutProps): JSX.Element {
urlQuery.set(QueryParams.startTime, startTimestamp.toString());
urlQuery.set(QueryParams.endTime, endTimestamp.toString());
const generatedUrl = `${pathname}?${urlQuery.toString()}`;
history.push(generatedUrl);
safeNavigate(generatedUrl);

if (startTimestamp !== endTimestamp) {
dispatch(UpdateTimeInterval('custom', [startTimestamp, endTimestamp]));
}
},
[dispatch, pathname, urlQuery],
[dispatch, pathname, safeNavigate, urlQuery],
);

useEffect(() => {
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/container/GridCardLayout/WidgetHeader/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import { QueryParams } from 'constants/query';
import { PANEL_TYPES } from 'constants/queryBuilder';
import useCreateAlerts from 'hooks/queryBuilder/useCreateAlerts';
import useComponentPermission from 'hooks/useComponentPermission';
import { useSafeNavigate } from 'hooks/useSafeNavigate';
import useUrlQuery from 'hooks/useUrlQuery';
import history from 'lib/history';
import { RowData } from 'lib/query/createTableColumnsFromQuery';
import { isEmpty } from 'lodash-es';
import { CircleX, X } from 'lucide-react';
Expand Down Expand Up @@ -72,6 +72,7 @@ function WidgetHeader({
setSearchTerm,
}: IWidgetHeaderProps): JSX.Element | null {
const urlQuery = useUrlQuery();
const { safeNavigate } = useSafeNavigate();
const onEditHandler = useCallback((): void => {
const widgetId = widget.id;
urlQuery.set(QueryParams.widgetId, widgetId);
Expand All @@ -81,8 +82,8 @@ function WidgetHeader({
encodeURIComponent(JSON.stringify(widget.query)),
);
const generatedUrl = `${window.location.pathname}/new?${urlQuery}`;
history.push(generatedUrl);
}, [urlQuery, widget.id, widget.panelTypes, widget.query]);
safeNavigate(generatedUrl);
}, [safeNavigate, urlQuery, widget.id, widget.panelTypes, widget.query]);

const onCreateAlertsHandler = useCreateAlerts(widget, 'dashboardView');

Expand Down
22 changes: 12 additions & 10 deletions frontend/src/container/ListOfDashboard/DashboardsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import dayjs from 'dayjs';
import { useGetAllDashboard } from 'hooks/dashboard/useGetAllDashboard';
import useComponentPermission from 'hooks/useComponentPermission';
import { useNotifications } from 'hooks/useNotifications';
import history from 'lib/history';
import { useSafeNavigate } from 'hooks/useSafeNavigate';
import { get, isEmpty, isUndefined } from 'lodash-es';
import {
ArrowDownWideNarrow,
Expand Down Expand Up @@ -74,7 +74,7 @@ import {
} from 'react';
import { Layout } from 'react-grid-layout';
import { useTranslation } from 'react-i18next';
import { generatePath, Link } from 'react-router-dom';
import { generatePath } from 'react-router-dom';
import { useCopyToClipboard } from 'react-use';
import {
Dashboard,
Expand Down Expand Up @@ -105,7 +105,7 @@ function DashboardsList(): JSX.Element {
} = useGetAllDashboard();

const { user } = useAppContext();

const { safeNavigate } = useSafeNavigate();
const {
listSortOrder: sortOrder,
setListSortOrder: setSortOrder,
Expand Down Expand Up @@ -293,7 +293,7 @@ function DashboardsList(): JSX.Element {
});

if (response.statusCode === 200) {
history.push(
safeNavigate(
generatePath(ROUTES.DASHBOARD, {
dashboardId: response.payload.uuid,
}),
Expand All @@ -313,7 +313,7 @@ function DashboardsList(): JSX.Element {
errorMessage: (error as AxiosError).toString() || 'Something went Wrong',
});
}
}, [newDashboardState, t]);
}, [newDashboardState, safeNavigate, t]);

const onModalHandler = (uploadedGrafana: boolean): void => {
logEvent('Dashboard List: Import JSON clicked', {});
Expand Down Expand Up @@ -418,7 +418,7 @@ function DashboardsList(): JSX.Element {
if (event.metaKey || event.ctrlKey) {
window.open(getLink(), '_blank');
} else {
history.push(getLink());
safeNavigate(getLink());
}
logEvent('Dashboard List: Clicked on dashboard', {
dashboardId: dashboard.id,
Expand All @@ -444,10 +444,12 @@ function DashboardsList(): JSX.Element {
placement="left"
overlayClassName="title-toolip"
>
<Link
to={getLink()}
<div
className="title-link"
onClick={(e): void => e.stopPropagation()}
onClick={(e): void => {
e.stopPropagation();
safeNavigate(getLink());
}}
>
<img
src={dashboard?.image || Base64Icons[0]}
Expand All @@ -460,7 +462,7 @@ function DashboardsList(): JSX.Element {
>
{dashboard.name}
</Typography.Text>
</Link>
</div>
</Tooltip>
</div>

Expand Down
Loading

0 comments on commit 82d84c0

Please sign in to comment.