From 5d120402976de585ddc5e03d3edc7c087217bcce Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Fri, 12 Jul 2024 11:17:07 +0200 Subject: [PATCH 01/12] Avoid excessive reexecution of `useSendTelemetry`. --- .../src/logic/telemetry/useSendTelemetry.tsx | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx index f5fa6ad4981b..1b23bad22679 100644 --- a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx +++ b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx @@ -14,21 +14,34 @@ * along with this program. If not, see * . */ -import { useCallback, useContext } from 'react'; +import { useCallback, useContext, useMemo } from 'react'; import type { Optional } from 'utility-types'; +import { UNSAFE_DataRouterContext as DataRouterContext, matchRoutes } from 'react-router-dom'; import type { TelemetryEventType, TelemetryEvent } from 'logic/telemetry/TelemetryContext'; import TelemetryContext from 'logic/telemetry/TelemetryContext'; -import useRoutePattern from 'routing/useRoutePattern'; +import AppConfig from 'util/AppConfig'; const useSendTelemetry = () => { const { sendTelemetry } = useContext(TelemetryContext); - const route = useRoutePattern(); + const dataRouterContext = useContext(DataRouterContext); + const pathPrefixLength = useMemo(() => { + const pathPrefix = AppConfig.gl2AppPathPrefix(); - return useCallback((eventType: TelemetryEventType, event: Optional) => sendTelemetry( - eventType, - { app_path_pattern: route, ...event }, - ), [sendTelemetry, route]); + return (!pathPrefix || pathPrefix === '' || pathPrefix === '/') ? 0 : pathPrefix.length; + }, []); + + return useCallback((eventType: TelemetryEventType, event: Optional) => { + const { router: { routes } } = dataRouterContext; + const pathname = window.location.pathname.slice(pathPrefixLength); + const matches = matchRoutes(routes, pathname); + const route = matches.at(-1).route.path; + + return sendTelemetry( + eventType, + { app_path_pattern: route, ...event }, + ); + }, [sendTelemetry, dataRouterContext]); }; export default useSendTelemetry; From e746631228f7dd0f26895e767895875a42057d6d Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Fri, 12 Jul 2024 13:54:57 +0200 Subject: [PATCH 02/12] Cleaning up. --- .../src/logic/telemetry/useSendTelemetry.tsx | 2 +- .../src/routing/useRoutePattern.ts | 39 ------------------- 2 files changed, 1 insertion(+), 40 deletions(-) delete mode 100644 graylog2-web-interface/src/routing/useRoutePattern.ts diff --git a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx index 1b23bad22679..d573f35b5d36 100644 --- a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx +++ b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx @@ -41,7 +41,7 @@ const useSendTelemetry = () => { eventType, { app_path_pattern: route, ...event }, ); - }, [sendTelemetry, dataRouterContext]); + }, [dataRouterContext, pathPrefixLength, sendTelemetry]); }; export default useSendTelemetry; diff --git a/graylog2-web-interface/src/routing/useRoutePattern.ts b/graylog2-web-interface/src/routing/useRoutePattern.ts deleted file mode 100644 index 9ec6e6382794..000000000000 --- a/graylog2-web-interface/src/routing/useRoutePattern.ts +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright (C) 2020 Graylog, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * . - */ -import { useContext, useMemo } from 'react'; -import { useLocation, UNSAFE_DataRouterContext as DataRouterContext, matchRoutes } from 'react-router-dom'; - -import { singleton } from 'logic/singleton'; - -const useRoutePattern = () => { - const location = useLocation(); - const dataRouterContext = useContext(DataRouterContext); - - return useMemo(() => { - if (dataRouterContext?.router?.routes) { - const { router: { routes } } = dataRouterContext; - const matches = matchRoutes(routes, location.pathname); - const { route } = matches.at(-1); - - return route.path; - } - - return undefined; - }, [location.pathname, dataRouterContext]); -}; - -export default singleton('hooks.useRoutePattern', () => useRoutePattern); From 7efd676be0d2ee774fc21be4374707a04e57032e Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Fri, 12 Jul 2024 13:58:54 +0200 Subject: [PATCH 03/12] Introducing reusable function. --- .../src/logic/telemetry/useSendTelemetry.tsx | 13 ++++--------- graylog2-web-interface/src/util/URLUtils.ts | 8 ++++++++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx index d573f35b5d36..ad75f221021f 100644 --- a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx +++ b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx @@ -14,26 +14,21 @@ * along with this program. If not, see * . */ -import { useCallback, useContext, useMemo } from 'react'; +import { useCallback, useContext } from 'react'; import type { Optional } from 'utility-types'; import { UNSAFE_DataRouterContext as DataRouterContext, matchRoutes } from 'react-router-dom'; import type { TelemetryEventType, TelemetryEvent } from 'logic/telemetry/TelemetryContext'; import TelemetryContext from 'logic/telemetry/TelemetryContext'; -import AppConfig from 'util/AppConfig'; +import { currentPathnameWithoutPrefix } from 'util/URLUtils'; const useSendTelemetry = () => { const { sendTelemetry } = useContext(TelemetryContext); const dataRouterContext = useContext(DataRouterContext); - const pathPrefixLength = useMemo(() => { - const pathPrefix = AppConfig.gl2AppPathPrefix(); - - return (!pathPrefix || pathPrefix === '' || pathPrefix === '/') ? 0 : pathPrefix.length; - }, []); return useCallback((eventType: TelemetryEventType, event: Optional) => { const { router: { routes } } = dataRouterContext; - const pathname = window.location.pathname.slice(pathPrefixLength); + const pathname = window.location.pathname.slice(currentPathnameWithoutPrefix()); const matches = matchRoutes(routes, pathname); const route = matches.at(-1).route.path; @@ -41,7 +36,7 @@ const useSendTelemetry = () => { eventType, { app_path_pattern: route, ...event }, ); - }, [dataRouterContext, pathPrefixLength, sendTelemetry]); + }, [dataRouterContext, sendTelemetry]); }; export default useSendTelemetry; diff --git a/graylog2-web-interface/src/util/URLUtils.ts b/graylog2-web-interface/src/util/URLUtils.ts index 96ecea0f5e8f..4fcb21e5692e 100644 --- a/graylog2-web-interface/src/util/URLUtils.ts +++ b/graylog2-web-interface/src/util/URLUtils.ts @@ -113,6 +113,13 @@ const URLUtils = { getPathnameWithoutId(pathname: string) { return pathname.replace(/\/[0-9a-fA-F]{24}/, '').slice(1); }, + currentPathnameWithoutPrefix() { + const pathPrefix = AppConfig.gl2AppPathPrefix(); + + const pathPrefixLength = (!pathPrefix || pathPrefix === '' || pathPrefix === '/') ? 0 : pathPrefix.length; + + return window.location.pathname.slice(pathPrefixLength); + }, }; export default URLUtils; @@ -127,4 +134,5 @@ export const { concatURLPath, isValidURL, hasAcceptedProtocol, + currentPathnameWithoutPrefix, } = URLUtils; From cfd86a605a6a2e352f9624bc25c36a0c2781a5b4 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Fri, 12 Jul 2024 14:07:27 +0200 Subject: [PATCH 04/12] Adding tests. --- .../src/util/URLUtils.test.ts | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/graylog2-web-interface/src/util/URLUtils.test.ts b/graylog2-web-interface/src/util/URLUtils.test.ts index 6b18285acf8e..c271f2ea5a51 100644 --- a/graylog2-web-interface/src/util/URLUtils.test.ts +++ b/graylog2-web-interface/src/util/URLUtils.test.ts @@ -15,7 +15,7 @@ * . */ import { asMock } from 'helpers/mocking'; -import { qualifyUrl } from 'util/URLUtils'; +import { qualifyUrl, currentPathnameWithoutPrefix } from 'util/URLUtils'; import AppConfig from 'util/AppConfig'; jest.mock('util/AppConfig'); @@ -50,4 +50,45 @@ describe('qualifyUrl', () => { expect(qualifyUrl(qualifiedUrl)).toEqual('http://something.graylog.cloud/api/foo'); }); + + describe('currentPathnameWithoutPrefix', () => { + const setLocation = (pathname: string) => Object.defineProperty(window, 'location', { + value: { + pathname, + }, + writable: true, + }); + + const mockPathPrefix = (pathPrefix: string | undefined | null) => asMock(AppConfig.gl2AppPathPrefix).mockReturnValue(pathPrefix); + + it('returns current path when prefix is undefined/null/empty/single slash', () => { + const pathname = '/welcome'; + setLocation(pathname); + + mockPathPrefix(null); + + expect(currentPathnameWithoutPrefix()).toBe(pathname); + + mockPathPrefix(undefined); + + expect(currentPathnameWithoutPrefix()).toBe(pathname); + + mockPathPrefix(''); + + expect(currentPathnameWithoutPrefix()).toBe(pathname); + + mockPathPrefix('/'); + + expect(currentPathnameWithoutPrefix()).toBe(pathname); + }); + + it('returns current path when prefix is defined', () => { + const pathname = '/welcome'; + setLocation(`/foo${pathname}`); + + mockPathPrefix('/foo'); + + expect(currentPathnameWithoutPrefix()).toBe(pathname); + }); + }); }); From fa7ba3b8f96849b2b3ab6703e3b89430727c8398 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Fri, 12 Jul 2024 14:32:38 +0200 Subject: [PATCH 05/12] Fixing up. --- graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx index ad75f221021f..1c27d36f76f3 100644 --- a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx +++ b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx @@ -28,7 +28,7 @@ const useSendTelemetry = () => { return useCallback((eventType: TelemetryEventType, event: Optional) => { const { router: { routes } } = dataRouterContext; - const pathname = window.location.pathname.slice(currentPathnameWithoutPrefix()); + const pathname = currentPathnameWithoutPrefix(); const matches = matchRoutes(routes, pathname); const route = matches.at(-1).route.path; From 55f9ddeb5e870eabec0711455e9b7019decf4ef2 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Mon, 15 Jul 2024 08:25:44 +0200 Subject: [PATCH 06/12] Making hook throw error when context is missing. --- .../src/logic/telemetry/useSendTelemetry.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx index 1c27d36f76f3..484110cef801 100644 --- a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx +++ b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx @@ -27,6 +27,10 @@ const useSendTelemetry = () => { const dataRouterContext = useContext(DataRouterContext); return useCallback((eventType: TelemetryEventType, event: Optional) => { + if (!dataRouterContext?.router?.routes) { + throw new Error('Data router context is missing!'); + } + const { router: { routes } } = dataRouterContext; const pathname = currentPathnameWithoutPrefix(); const matches = matchRoutes(routes, pathname); From 632cbec0d8bbe92ffa6e49f6469b118277fd29f5 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Mon, 15 Jul 2024 08:47:14 +0200 Subject: [PATCH 07/12] Fixing test filename. --- ...keysContianerModal.test.tsx => HotkeysModalContainer.test.tsx} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename graylog2-web-interface/src/components/hotkeys/{HotkeysContianerModal.test.tsx => HotkeysModalContainer.test.tsx} (100%) diff --git a/graylog2-web-interface/src/components/hotkeys/HotkeysContianerModal.test.tsx b/graylog2-web-interface/src/components/hotkeys/HotkeysModalContainer.test.tsx similarity index 100% rename from graylog2-web-interface/src/components/hotkeys/HotkeysContianerModal.test.tsx rename to graylog2-web-interface/src/components/hotkeys/HotkeysModalContainer.test.tsx From 763b47f91dc40eb5de5724f22545a9b1f5825306 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Mon, 15 Jul 2024 08:47:43 +0200 Subject: [PATCH 08/12] Fixing state handling in `SetProfileModal` test uncovered by change. --- .../indices/IndexSetFieldTypes/SetProfileModal.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/graylog2-web-interface/src/components/indices/IndexSetFieldTypes/SetProfileModal.tsx b/graylog2-web-interface/src/components/indices/IndexSetFieldTypes/SetProfileModal.tsx index 8ef32ab91776..e2709315a03a 100644 --- a/graylog2-web-interface/src/components/indices/IndexSetFieldTypes/SetProfileModal.tsx +++ b/graylog2-web-interface/src/components/indices/IndexSetFieldTypes/SetProfileModal.tsx @@ -94,11 +94,18 @@ const SetProfileModal = ({ show, onClose, currentProfile }: Props) => { }, [onClose, sendTelemetry, telemetryPathName]); useEffect(() => { - setProfile(currentProfile); sendTelemetry(TELEMETRY_EVENT_TYPE.INDEX_SET_FIELD_TYPE_PROFILE.CHANGE_FOR_INDEX_OPENED, { app_pathname: telemetryPathName, app_action_value: 'removed-custom-field-type-opened' }); }, [sendTelemetry, telemetryPathName, currentProfile]); - const onChangeProfile = (newProfile: string) => setProfile(newProfile); + useEffect(() => { + setProfile(currentProfile); + }, [currentProfile]); + + const onChangeProfile = (newProfile: string) => { + console.log('Changing profile to: ', newProfile); + + return setProfile(newProfile); + }; return ( Date: Mon, 15 Jul 2024 09:35:54 +0200 Subject: [PATCH 09/12] Removing console statement. --- .../indices/IndexSetFieldTypes/SetProfileModal.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/graylog2-web-interface/src/components/indices/IndexSetFieldTypes/SetProfileModal.tsx b/graylog2-web-interface/src/components/indices/IndexSetFieldTypes/SetProfileModal.tsx index e2709315a03a..0650584807c9 100644 --- a/graylog2-web-interface/src/components/indices/IndexSetFieldTypes/SetProfileModal.tsx +++ b/graylog2-web-interface/src/components/indices/IndexSetFieldTypes/SetProfileModal.tsx @@ -101,11 +101,7 @@ const SetProfileModal = ({ show, onClose, currentProfile }: Props) => { setProfile(currentProfile); }, [currentProfile]); - const onChangeProfile = (newProfile: string) => { - console.log('Changing profile to: ', newProfile); - - return setProfile(newProfile); - }; + const onChangeProfile = (newProfile: string) => setProfile(newProfile); return ( Date: Mon, 15 Jul 2024 13:30:33 +0200 Subject: [PATCH 10/12] Making `useSendTelemetry` resilient against missing context. --- graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx index 484110cef801..2ad5e3234f8c 100644 --- a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx +++ b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx @@ -28,7 +28,7 @@ const useSendTelemetry = () => { return useCallback((eventType: TelemetryEventType, event: Optional) => { if (!dataRouterContext?.router?.routes) { - throw new Error('Data router context is missing!'); + return () => {}; } const { router: { routes } } = dataRouterContext; From d022af299f25a897060b04a7af80e0640d6cbf38 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Mon, 15 Jul 2024 13:37:17 +0200 Subject: [PATCH 11/12] Handling absent context as previously. --- .../src/logic/telemetry/useSendTelemetry.tsx | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx index 2ad5e3234f8c..a2441ede2b53 100644 --- a/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx +++ b/graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx @@ -17,24 +17,30 @@ import { useCallback, useContext } from 'react'; import type { Optional } from 'utility-types'; import { UNSAFE_DataRouterContext as DataRouterContext, matchRoutes } from 'react-router-dom'; +import type { DataRouterContextObject } from 'react-router/dist/lib/context'; import type { TelemetryEventType, TelemetryEvent } from 'logic/telemetry/TelemetryContext'; import TelemetryContext from 'logic/telemetry/TelemetryContext'; import { currentPathnameWithoutPrefix } from 'util/URLUtils'; +const retrieveCurrentRoute = (dataRouterContext: DataRouterContextObject) => { + if (!dataRouterContext?.router?.routes) { + return undefined; + } + + const { router: { routes } } = dataRouterContext; + const pathname = currentPathnameWithoutPrefix(); + const matches = matchRoutes(routes, pathname); + + return matches.at(-1).route.path; +}; + const useSendTelemetry = () => { const { sendTelemetry } = useContext(TelemetryContext); const dataRouterContext = useContext(DataRouterContext); return useCallback((eventType: TelemetryEventType, event: Optional) => { - if (!dataRouterContext?.router?.routes) { - return () => {}; - } - - const { router: { routes } } = dataRouterContext; - const pathname = currentPathnameWithoutPrefix(); - const matches = matchRoutes(routes, pathname); - const route = matches.at(-1).route.path; + const route = retrieveCurrentRoute(dataRouterContext); return sendTelemetry( eventType, From 0577b07d8783cc719713eb32064c0c82f446c760 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Mon, 15 Jul 2024 15:17:57 +0200 Subject: [PATCH 12/12] Disabling linter hint. --- graylog2-web-interface/src/util/URLUtils.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/graylog2-web-interface/src/util/URLUtils.test.ts b/graylog2-web-interface/src/util/URLUtils.test.ts index c271f2ea5a51..8742a3c35986 100644 --- a/graylog2-web-interface/src/util/URLUtils.test.ts +++ b/graylog2-web-interface/src/util/URLUtils.test.ts @@ -22,6 +22,7 @@ jest.mock('util/AppConfig'); const oldLocation = window.location; +// eslint-disable-next-line compat/compat const mockLocation = (url: string): Location => new URL(url) as unknown as Location; describe('qualifyUrl', () => {