From e6e4e343aa700f1b489915f8df233a0abf2ea058 Mon Sep 17 00:00:00 2001 From: Elena Stoeva <59341489+ElenaStoeva@users.noreply.github.com> Date: Mon, 21 Oct 2024 20:20:30 +0100 Subject: [PATCH] [Console] Fix load_from param (#196836) Fixes https://github.com/elastic/kibana/issues/195877 Addresses https://github.com/elastic/kibana/issues/179658 ## Summary This PR fixes the bug in Console where using the `load_from` param in the URL made Console uneditable because every re-render reset the initial value in the editor. This is fixed by restricting the hook to only set the initial value once. This PR also adds some unit tests for the hook, as I realized that this was a long-standing improvement. ### How to test: Try loading the following URL (making the necessary replacement in the URL) and verify that the data is correctly loaded into the editor and value can be edited: `http://localhost:5601//app/dev_tools#/console?load_from=data:text/plain,AoeQygKgBA9A+gRwK4FMBOBPGBDAzhgOwGMB+AEzQHsAHOApAGwbiMoaQFsDcAoAbx5QoAImToMwgFwiAZgCVKAWShoUHSgBcUAWgBUkgJYEyKAB4pcwgDSCRDSkWwMUUkSgLXbwmQYZa0rgJCQsIARpRsgbbBIhxIuBquANoAujYxIT5+6Mlp0cHCuAAWlIxkuekZwnEJdJq5+QC+ts2NQA` `http://localhost:5601//app/dev_tools#/console?load_from=https://www.elastic.co/guide/en/elasticsearch/reference/current/snippets/86.console` Co-authored-by: Matthew Kime --- .../hooks/use_set_initial_value.test.ts | 178 ++++++++++++++++++ .../editor/hooks/use_set_initial_value.ts | 17 +- 2 files changed, 189 insertions(+), 6 deletions(-) create mode 100644 src/plugins/console/public/application/containers/editor/hooks/use_set_initial_value.test.ts diff --git a/src/plugins/console/public/application/containers/editor/hooks/use_set_initial_value.test.ts b/src/plugins/console/public/application/containers/editor/hooks/use_set_initial_value.test.ts new file mode 100644 index 000000000000..0605a0c903ee --- /dev/null +++ b/src/plugins/console/public/application/containers/editor/hooks/use_set_initial_value.test.ts @@ -0,0 +1,178 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { renderHook, act } from '@testing-library/react-hooks'; +import { useSetInitialValue } from './use_set_initial_value'; +import { IToasts } from '@kbn/core-notifications-browser'; +import { decompressFromEncodedURIComponent } from 'lz-string'; +import { DEFAULT_INPUT_VALUE } from '../../../../../common/constants'; + +jest.mock('lz-string', () => ({ + decompressFromEncodedURIComponent: jest.fn(), +})); + +jest.mock('./use_set_initial_value', () => ({ + ...jest.requireActual('./use_set_initial_value'), +})); + +describe('useSetInitialValue', () => { + const setValueMock = jest.fn(); + const addWarningMock = jest.fn(); + const toastsMock: IToasts = { addWarning: addWarningMock } as any; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should set the initial value only once', async () => { + const { rerender } = renderHook(() => + useSetInitialValue({ + localStorageValue: 'initial value', + setValue: setValueMock, + toasts: toastsMock, + }) + ); + + // Verify initial value is set on first render + expect(setValueMock).toHaveBeenCalledTimes(1); + expect(setValueMock).toHaveBeenCalledWith('initial value'); + + // Re-render the hook to simulate a component update + rerender(); + + // Verify that setValue is not called again after rerender + expect(setValueMock).toHaveBeenCalledTimes(1); // Still 1, no additional calls + }); + + it('should set value from localStorage if no load_from param is present', () => { + renderHook(() => + useSetInitialValue({ + localStorageValue: 'saved value', + setValue: setValueMock, + toasts: toastsMock, + }) + ); + + expect(setValueMock).toHaveBeenCalledWith('saved value'); + }); + + it('should set default value if localStorage is undefined and no load_from param is present', () => { + renderHook(() => + useSetInitialValue({ + localStorageValue: undefined, + setValue: setValueMock, + toasts: toastsMock, + }) + ); + + expect(setValueMock).toHaveBeenCalledWith(DEFAULT_INPUT_VALUE); + }); + + it('should load data from load_from param if it is a valid Elastic URL', async () => { + Object.defineProperty(window, 'location', { + writable: true, + value: { + hash: '?load_from=https://www.elastic.co/some-data', + }, + }); + + // Mock fetch to return "remote data" + global.fetch = jest.fn(() => + Promise.resolve({ + text: () => Promise.resolve('remote data'), + }) + ) as jest.Mock; + + await act(async () => { + renderHook(() => + useSetInitialValue({ + localStorageValue: 'initial value', + setValue: setValueMock, + toasts: toastsMock, + }) + ); + }); + + expect(fetch).toHaveBeenCalledWith(new URL('https://www.elastic.co/some-data')); + // The remote data should be appended to the initial value in the editor + expect(setValueMock).toHaveBeenCalledWith('initial value\n\nremote data'); + }); + + it('should show a warning if the load_from param is not an Elastic domain', async () => { + Object.defineProperty(window, 'location', { + writable: true, + value: { + hash: '?load_from=https://not.elastic.com/some-data', + }, + }); + + await act(async () => { + renderHook(() => + useSetInitialValue({ + localStorageValue: 'initial value', + setValue: setValueMock, + toasts: toastsMock, + }) + ); + }); + + expect(fetch).not.toHaveBeenCalled(); + expect(addWarningMock).toHaveBeenCalledWith( + 'Only URLs with the Elastic domain (www.elastic.co) can be loaded in Console.' + ); + }); + + it('should load and decompress data from a data URI', async () => { + Object.defineProperty(window, 'location', { + writable: true, + value: { + hash: '?load_from=data:text/plain,compressed-data', + }, + }); + (decompressFromEncodedURIComponent as jest.Mock).mockReturnValue('decompressed data'); + + await act(async () => { + renderHook(() => + useSetInitialValue({ + localStorageValue: 'initial value', + setValue: setValueMock, + toasts: toastsMock, + }) + ); + }); + + expect(decompressFromEncodedURIComponent).toHaveBeenCalledWith('compressed-data'); + // The initial value in the editor should be replaces with the decompressed data + expect(setValueMock).toHaveBeenCalledWith('decompressed data'); + }); + + it('should show a warning if decompressing a data URI fails', async () => { + Object.defineProperty(window, 'location', { + writable: true, + value: { + hash: '?load_from=data:text/plain,invalid-data', + }, + }); + (decompressFromEncodedURIComponent as jest.Mock).mockReturnValue(null); + + await act(async () => { + renderHook(() => + useSetInitialValue({ + localStorageValue: 'initial value', + setValue: setValueMock, + toasts: toastsMock, + }) + ); + }); + + expect(addWarningMock).toHaveBeenCalledWith( + 'Unable to load data from the load_from query parameter in the URL' + ); + }); +}); diff --git a/src/plugins/console/public/application/containers/editor/hooks/use_set_initial_value.ts b/src/plugins/console/public/application/containers/editor/hooks/use_set_initial_value.ts index 9e96c1af2734..11e9c4878227 100644 --- a/src/plugins/console/public/application/containers/editor/hooks/use_set_initial_value.ts +++ b/src/plugins/console/public/application/containers/editor/hooks/use_set_initial_value.ts @@ -12,7 +12,7 @@ import { parse } from 'query-string'; import { IToasts } from '@kbn/core-notifications-browser'; import { decompressFromEncodedURIComponent } from 'lz-string'; import { i18n } from '@kbn/i18n'; -import { useEffect } from 'react'; +import { useEffect, useRef } from 'react'; import { DEFAULT_INPUT_VALUE } from '../../../../../common/constants'; interface QueryParams { @@ -46,6 +46,7 @@ export const readLoadFromParam = () => { */ export const useSetInitialValue = (params: SetInitialValueParams) => { const { localStorageValue, setValue, toasts } = params; + const isInitialValueSet = useRef(false); useEffect(() => { const loadBufferFromRemote = async (url: string) => { @@ -104,11 +105,15 @@ export const useSetInitialValue = (params: SetInitialValueParams) => { const loadFromParam = readLoadFromParam(); - if (loadFromParam) { - loadBufferFromRemote(loadFromParam); - } else { - // Only set to default input value if the localstorage value is undefined - setValue(localStorageValue ?? DEFAULT_INPUT_VALUE); + // Only set the value in the editor if an initial value hasn't been set yet + if (!isInitialValueSet.current) { + if (loadFromParam) { + loadBufferFromRemote(loadFromParam); + } else { + // Only set to default input value if the localstorage value is undefined + setValue(localStorageValue ?? DEFAULT_INPUT_VALUE); + } + isInitialValueSet.current = true; } return () => {