Skip to content

Commit

Permalink
Avoid excessive reexecution of useSendTelemetry. (#19896)
Browse files Browse the repository at this point in the history
* Avoid excessive reexecution of `useSendTelemetry`.

* Cleaning up.

* Introducing reusable function.

* Adding tests.

* Fixing up.

* Making hook throw error when context is missing.

* Fixing test filename.

* Fixing state handling in `SetProfileModal` test uncovered by change.

* Removing console statement.

* Making `useSendTelemetry` resilient against missing context.

* Handling absent context as previously.

* Disabling linter hint.
  • Loading branch information
dennisoelkers authored Jul 16, 2024
1 parent 6c10c75 commit 2d20aea
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,13 @@ 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]);

useEffect(() => {
setProfile(currentProfile);
}, [currentProfile]);

const onChangeProfile = (newProfile: string) => setProfile(newProfile);

return (
Expand Down
30 changes: 24 additions & 6 deletions graylog2-web-interface/src/logic/telemetry/useSendTelemetry.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,37 @@
*/
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 useRoutePattern from 'routing/useRoutePattern';
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 route = useRoutePattern();
const dataRouterContext = useContext(DataRouterContext);

return useCallback((eventType: TelemetryEventType, event: Optional<TelemetryEvent, 'app_path_pattern'>) => {
const route = retrieveCurrentRoute(dataRouterContext);

return useCallback((eventType: TelemetryEventType, event: Optional<TelemetryEvent, 'app_path_pattern'>) => sendTelemetry(
eventType,
{ app_path_pattern: route, ...event },
), [sendTelemetry, route]);
return sendTelemetry(
eventType,
{ app_path_pattern: route, ...event },
);
}, [dataRouterContext, sendTelemetry]);
};

export default useSendTelemetry;
39 changes: 0 additions & 39 deletions graylog2-web-interface/src/routing/useRoutePattern.ts

This file was deleted.

44 changes: 43 additions & 1 deletion graylog2-web-interface/src/util/URLUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
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');

const oldLocation = window.location;

// eslint-disable-next-line compat/compat
const mockLocation = (url: string): Location => new URL(url) as unknown as Location;

describe('qualifyUrl', () => {
Expand Down Expand Up @@ -50,4 +51,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);
});
});
});
8 changes: 8 additions & 0 deletions graylog2-web-interface/src/util/URLUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -127,4 +134,5 @@ export const {
concatURLPath,
isValidURL,
hasAcceptedProtocol,
currentPathnameWithoutPrefix,
} = URLUtils;

0 comments on commit 2d20aea

Please sign in to comment.