Explore: Change Logs.tsx to a functional component (#87808)

* First pass * WIP * why is my cpu angy * Avoid triggering onHiddenSeriesChanged too often onHiddenSeriesChanged should be called only when dataWithConfig changes, not when the callback changes itself. This is actually causing an infinite loop because onHiddenSeriesChanged may not be memoized in the parent and passed as a new callback function on each render. * fix tests * Remove store mock and clear store between tests instead * Fix filtering * First pass adding callbacks and dependencies * Add useRef hook for toggleLegend and topLogsRef * Remove unwanted reactivity register(...) changes outlineItems. A function that calls register(...) behaves like a setter for the state - it cannot react to the state itself. * Separate unmount clean-ups * Ensure unmount is not reactive to props * Make memoized functions stats * Wrap functions passed down to components with useCallback * Fix reporting interaction when context is closed * Adjust several variables to use refs * Post-merge fixes * Ensure scrollIntoView is called with a ref to the container * Update docs * Ensure scrollIntoView is updated only if the scroll element changes * Fix the default pinned log tooltip * Small clean up * Remove console.log() --------- Co-authored-by: Piotr Jamroz <pm.jamroz@gmail.com> Co-authored-by: harisrozajac <haris.rozajac12@gmail.com>
2024-11-25 02:10:45 -06:00 · 2024-06-20 08:04:44 -05:00 · 2024-06-20 08:04:44 -05:00 · b7df121294
commit b7df121294
parent 2a714601a7
5 changed files with 902 additions and 921 deletions
--- a/public/app/features/explore/ContentOutline/ContentOutlineContext.tsx
+++ b/public/app/features/explore/ContentOutline/ContentOutlineContext.tsx
@ -16,7 +16,10 @@ export interface ContentOutlineContextProps {
  outlineItems: ContentOutlineItemContextProps[];
  register: RegisterFunction;
  unregister: (id: string) => void;
-  unregisterAllChildren: (parentId: string, childType: ITEM_TYPES) => void;
+  unregisterAllChildren: (
+    parentIdGetter: (items: ContentOutlineItemContextProps[]) => string | undefined,
+    childType: ITEM_TYPES
+  ) => void;
  updateOutlineItems: (newItems: ContentOutlineItemContextProps[]) => void;
  updateItem: (id: string, properties: Partial<Omit<ContentOutlineItemContextProps, 'id'>>) => void;
 }
@ -193,16 +196,23 @@ export function ContentOutlineContextProvider({ children, refreshDependencies }:
    );
  }, []);

-  const unregisterAllChildren = useCallback((parentId: string, childType: ITEM_TYPES) => {
-    setOutlineItems((prevItems) =>
-      prevItems.map((item) => {
-        if (item.id === parentId) {
-          item.children = item.children?.filter((child) => child.type !== childType);
+  const unregisterAllChildren = useCallback(
+    (parentIdGetter: (items: ContentOutlineItemContextProps[]) => string | undefined, childType: ITEM_TYPES) => {
+      setOutlineItems((prevItems) => {
+        const parentId = parentIdGetter(prevItems);
+        if (!parentId) {
+          return prevItems;
        }
-        return item;
-      })
-    );
-  }, []);
+        return prevItems.map((item) => {
+          if (item.id === parentId) {
+            item.children = item.children?.filter((child) => child.type !== childType);
+          }
+          return item;
+        });
+      });
+    },
+    []
+  );

  useEffect(() => {
    setOutlineItems((prevItems) => {
--- a/public/app/features/explore/Graph/ExploreGraph.tsx
+++ b/public/app/features/explore/Graph/ExploreGraph.tsx
@ -1,5 +1,5 @@
 import { identity } from 'lodash';
-import React, { useEffect, useMemo, useState } from 'react';
+import React, { useEffect, useMemo, useRef, useState } from 'react';
 import { usePrevious } from 'react-use';

 import {
@ -154,8 +154,10 @@ export function ExploreGraph({

  const structureRev = useStructureRev(dataWithConfig);

+  const onHiddenSeriesChangedRef = useRef(onHiddenSeriesChanged);
+
  useEffect(() => {
-    if (onHiddenSeriesChanged) {
+    if (onHiddenSeriesChangedRef.current) {
      const hiddenFrames: string[] = [];
      dataWithConfig.forEach((frame) => {
        const allFieldsHidden = frame.fields.map((field) => field.config?.custom?.hideFrom?.viz).every(identity);
@ -163,9 +165,9 @@ export function ExploreGraph({
          hiddenFrames.push(getFrameDisplayName(frame));
        }
      });
-      onHiddenSeriesChanged(hiddenFrames);
+      onHiddenSeriesChangedRef.current(hiddenFrames);
    }
-  }, [dataWithConfig, onHiddenSeriesChanged]);
+  }, [dataWithConfig]);

  const panelContext: PanelContext = {
    eventsScope: 'explore',
--- a/public/app/features/explore/Logs/Logs.test.tsx
+++ b/public/app/features/explore/Logs/Logs.test.tsx
@ -1,11 +1,11 @@
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import React, { ComponentProps } from 'react';
+import { Provider } from 'react-redux';

 import {
  DataFrame,
  EventBusSrv,
-  ExploreLogsPanelState,
  ExplorePanelsState,
  LoadingState,
  LogLevel,
@ -13,25 +13,20 @@ import {
  standardTransformersRegistry,
  toUtc,
  createDataFrame,
+  ExploreLogsPanelState,
 } from '@grafana/data';
 import { organizeFieldsTransformer } from '@grafana/data/src/transformations/transformers/organize';
 import { config } from '@grafana/runtime';
-import store from 'app/core/store';
 import { extractFieldsTransformer } from 'app/features/transformers/extractFields/extractFields';
+import { configureStore } from 'app/store/configureStore';
+
+import { initialExploreState } from '../state/main';
+import { makeExplorePaneState } from '../state/utils';

 import { Logs } from './Logs';
 import { visualisationTypeKey } from './utils/logs';
 import { getMockElasticFrame, getMockLokiFrame } from './utils/testMocks.test';

-jest.mock('app/core/store', () => {
-  return {
-    getBool: jest.fn(),
-    getObject: jest.fn((_a, b) => b),
-    get: jest.fn(),
-    set: jest.fn(),
-  };
-});
-
 const reportInteraction = jest.fn();
 jest.mock('@grafana/runtime', () => ({
  ...jest.requireActual('@grafana/runtime'),
@ -45,26 +40,11 @@ jest.mock('app/core/utils/shortLinks', () => ({
  createAndCopyShortLink: (url: string) => createAndCopyShortLink(url),
 }));

-jest.mock('app/store/store', () => ({
-  getState: jest.fn().mockReturnValue({
-    explore: {
-      panes: {
-        left: {
-          datasource: 'id',
-          queries: [{ refId: 'A', expr: '', queryType: 'range', datasource: { type: 'loki', uid: 'id' } }],
-          range: { raw: { from: 'now-1h', to: 'now' } },
-        },
-      },
-    },
-  }),
-  dispatch: jest.fn(),
-}));
-
-const changePanelState = jest.fn();
+const fakeChangePanelState = jest.fn().mockReturnValue({ type: 'fakeAction' });
 jest.mock('../state/explorePane', () => ({
  ...jest.requireActual('../state/explorePane'),
  changePanelState: (exploreId: string, panel: 'logs', panelState: {} | ExploreLogsPanelState) => {
-    return changePanelState(exploreId, panel, panelState);
+    return fakeChangePanelState(exploreId, panel, panelState);
  },
 }));

@ -72,6 +52,7 @@ describe('Logs', () => {
  let originalHref = window.location.href;

  beforeEach(() => {
+    localStorage.clear();
    jest.clearAllMocks();
  });

@ -120,6 +101,7 @@ describe('Logs', () => {
    ];

    const testDataFrame = dataFrame ?? getMockLokiFrame();
+
    return (
      <Logs
        exploreId={'left'}
@ -157,8 +139,23 @@ describe('Logs', () => {
      />
    );
  };
+
  const setup = (partialProps?: Partial<ComponentProps<typeof Logs>>, dataFrame?: DataFrame, logs?: LogRowModel[]) => {
-    return render(getComponent(partialProps, dataFrame ? dataFrame : getMockLokiFrame(), logs));
+    const fakeStore = configureStore({
+      explore: {
+        ...initialExploreState,
+        panes: {
+          left: makeExplorePaneState(),
+        },
+      },
+    });
+
+    const { rerender } = render(
+      <Provider store={fakeStore}>
+        {getComponent(partialProps, dataFrame ? dataFrame : getMockLokiFrame(), logs)}
+      </Provider>
+    );
+    return { rerender, store: fakeStore };
  };

  describe('scrolling behavior', () => {
@ -216,40 +213,47 @@ describe('Logs', () => {

  it('should render a load more button', () => {
    const scanningStarted = jest.fn();
+    const store = configureStore({
+      explore: {
+        ...initialExploreState,
+      },
+    });
    render(
-      <Logs
-        exploreId={'left'}
-        splitOpen={() => undefined}
-        logsVolumeEnabled={true}
-        onSetLogsVolumeEnabled={() => null}
-        onClickFilterLabel={() => null}
-        onClickFilterOutLabel={() => null}
-        logsVolumeData={undefined}
-        loadLogsVolumeData={() => undefined}
-        logRows={[]}
-        onStartScanning={scanningStarted}
-        timeZone={'utc'}
-        width={50}
-        loading={false}
-        loadingState={LoadingState.Done}
-        absoluteRange={{
-          from: toUtc('2019-01-01 10:00:00').valueOf(),
-          to: toUtc('2019-01-01 16:00:00').valueOf(),
-        }}
-        range={{
-          from: toUtc('2019-01-01 10:00:00'),
-          to: toUtc('2019-01-01 16:00:00'),
-          raw: { from: 'now-1h', to: 'now' },
-        }}
-        addResultsToCache={() => {}}
-        onChangeTime={() => {}}
-        clearCache={() => {}}
-        getFieldLinks={() => {
-          return [];
-        }}
-        eventBus={new EventBusSrv()}
-        isFilterLabelActive={jest.fn()}
-      />
+      <Provider store={store}>
+        <Logs
+          exploreId={'left'}
+          splitOpen={() => undefined}
+          logsVolumeEnabled={true}
+          onSetLogsVolumeEnabled={() => null}
+          onClickFilterLabel={() => null}
+          onClickFilterOutLabel={() => null}
+          logsVolumeData={undefined}
+          loadLogsVolumeData={() => undefined}
+          logRows={[]}
+          onStartScanning={scanningStarted}
+          timeZone={'utc'}
+          width={50}
+          loading={false}
+          loadingState={LoadingState.Done}
+          absoluteRange={{
+            from: toUtc('2019-01-01 10:00:00').valueOf(),
+            to: toUtc('2019-01-01 16:00:00').valueOf(),
+          }}
+          range={{
+            from: toUtc('2019-01-01 10:00:00'),
+            to: toUtc('2019-01-01 16:00:00'),
+            raw: { from: 'now-1h', to: 'now' },
+          }}
+          addResultsToCache={() => {}}
+          onChangeTime={() => {}}
+          clearCache={() => {}}
+          getFieldLinks={() => {
+            return [];
+          }}
+          eventBus={new EventBusSrv()}
+          isFilterLabelActive={jest.fn()}
+        />
+      </Provider>
    );
    const button = screen.getByRole('button', {
      name: /scan for older logs/i,
@ -259,40 +263,47 @@ describe('Logs', () => {
  });

  it('should render a stop scanning button', () => {
+    const store = configureStore({
+      explore: {
+        ...initialExploreState,
+      },
+    });
    render(
-      <Logs
-        exploreId={'left'}
-        splitOpen={() => undefined}
-        logsVolumeEnabled={true}
-        onSetLogsVolumeEnabled={() => null}
-        onClickFilterLabel={() => null}
-        onClickFilterOutLabel={() => null}
-        logsVolumeData={undefined}
-        loadLogsVolumeData={() => undefined}
-        logRows={[]}
-        scanning={true}
-        timeZone={'utc'}
-        width={50}
-        loading={false}
-        loadingState={LoadingState.Done}
-        absoluteRange={{
-          from: toUtc('2019-01-01 10:00:00').valueOf(),
-          to: toUtc('2019-01-01 16:00:00').valueOf(),
-        }}
-        range={{
-          from: toUtc('2019-01-01 10:00:00'),
-          to: toUtc('2019-01-01 16:00:00'),
-          raw: { from: 'now-1h', to: 'now' },
-        }}
-        addResultsToCache={() => {}}
-        onChangeTime={() => {}}
-        clearCache={() => {}}
-        getFieldLinks={() => {
-          return [];
-        }}
-        eventBus={new EventBusSrv()}
-        isFilterLabelActive={jest.fn()}
-      />
+      <Provider store={store}>
+        <Logs
+          exploreId={'left'}
+          splitOpen={() => undefined}
+          logsVolumeEnabled={true}
+          onSetLogsVolumeEnabled={() => null}
+          onClickFilterLabel={() => null}
+          onClickFilterOutLabel={() => null}
+          logsVolumeData={undefined}
+          loadLogsVolumeData={() => undefined}
+          logRows={[]}
+          scanning={true}
+          timeZone={'utc'}
+          width={50}
+          loading={false}
+          loadingState={LoadingState.Done}
+          absoluteRange={{
+            from: toUtc('2019-01-01 10:00:00').valueOf(),
+            to: toUtc('2019-01-01 16:00:00').valueOf(),
+          }}
+          range={{
+            from: toUtc('2019-01-01 10:00:00'),
+            to: toUtc('2019-01-01 16:00:00'),
+            raw: { from: 'now-1h', to: 'now' },
+          }}
+          addResultsToCache={() => {}}
+          onChangeTime={() => {}}
+          clearCache={() => {}}
+          getFieldLinks={() => {
+            return [];
+          }}
+          eventBus={new EventBusSrv()}
+          isFilterLabelActive={jest.fn()}
+        />
+      </Provider>
    );

    expect(
@ -304,42 +315,48 @@ describe('Logs', () => {

  it('should render a stop scanning button', () => {
    const scanningStopped = jest.fn();
-
+    const store = configureStore({
+      explore: {
+        ...initialExploreState,
+      },
+    });
    render(
-      <Logs
-        exploreId={'left'}
-        splitOpen={() => undefined}
-        logsVolumeEnabled={true}
-        onSetLogsVolumeEnabled={() => null}
-        onClickFilterLabel={() => null}
-        onClickFilterOutLabel={() => null}
-        logsVolumeData={undefined}
-        loadLogsVolumeData={() => undefined}
-        logRows={[]}
-        scanning={true}
-        onStopScanning={scanningStopped}
-        timeZone={'utc'}
-        width={50}
-        loading={false}
-        loadingState={LoadingState.Done}
-        absoluteRange={{
-          from: toUtc('2019-01-01 10:00:00').valueOf(),
-          to: toUtc('2019-01-01 16:00:00').valueOf(),
-        }}
-        range={{
-          from: toUtc('2019-01-01 10:00:00'),
-          to: toUtc('2019-01-01 16:00:00'),
-          raw: { from: 'now-1h', to: 'now' },
-        }}
-        addResultsToCache={() => {}}
-        onChangeTime={() => {}}
-        clearCache={() => {}}
-        getFieldLinks={() => {
-          return [];
-        }}
-        eventBus={new EventBusSrv()}
-        isFilterLabelActive={jest.fn()}
-      />
+      <Provider store={store}>
+        <Logs
+          exploreId={'left'}
+          splitOpen={() => undefined}
+          logsVolumeEnabled={true}
+          onSetLogsVolumeEnabled={() => null}
+          onClickFilterLabel={() => null}
+          onClickFilterOutLabel={() => null}
+          logsVolumeData={undefined}
+          loadLogsVolumeData={() => undefined}
+          logRows={[]}
+          scanning={true}
+          onStopScanning={scanningStopped}
+          timeZone={'utc'}
+          width={50}
+          loading={false}
+          loadingState={LoadingState.Done}
+          absoluteRange={{
+            from: toUtc('2019-01-01 10:00:00').valueOf(),
+            to: toUtc('2019-01-01 16:00:00').valueOf(),
+          }}
+          range={{
+            from: toUtc('2019-01-01 10:00:00'),
+            to: toUtc('2019-01-01 16:00:00'),
+            raw: { from: 'now-1h', to: 'now' },
+          }}
+          addResultsToCache={() => {}}
+          onChangeTime={() => {}}
+          clearCache={() => {}}
+          getFieldLinks={() => {
+            return [];
+          }}
+          eventBus={new EventBusSrv()}
+          isFilterLabelActive={jest.fn()}
+        />
+      </Provider>
    );

    const button = screen.getByRole('button', {
@ -363,12 +380,12 @@ describe('Logs', () => {
  describe('for permalinking', () => {
    it('should dispatch a `changePanelState` event without the id', () => {
      const panelState = { logs: { id: '1' } };
-      const { rerender } = setup({ loading: false, panelState });
+      const { rerender, store } = setup({ loading: false, panelState });

-      rerender(getComponent({ loading: true, exploreId: 'right', panelState }));
-      rerender(getComponent({ loading: false, exploreId: 'right', panelState }));
+      rerender(<Provider store={store}>{getComponent({ loading: true, exploreId: 'right', panelState })}</Provider>);
+      rerender(<Provider store={store}>{getComponent({ loading: false, exploreId: 'right', panelState })}</Provider>);

-      expect(changePanelState).toHaveBeenCalledWith('right', 'logs', { logs: {} });
+      expect(fakeChangePanelState).toHaveBeenCalledWith('right', 'logs', { logs: {} });
    });

    it('should scroll the scrollElement into view if rows contain id', () => {
@ -491,23 +508,17 @@ describe('Logs', () => {
    });

    it('should use default state from localstorage - table', async () => {
-      const oldGet = store.get;
-      store.get = jest.fn().mockReturnValue('table');
      localStorage.setItem(visualisationTypeKey, 'table');
      setup({});
      const table = await screen.findByTestId('logRowsTable');
      expect(table).toBeInTheDocument();
-      store.get = oldGet;
    });

    it('should use default state from localstorage - logs', async () => {
-      const oldGet = store.get;
-      store.get = jest.fn().mockReturnValue('logs');
      localStorage.setItem(visualisationTypeKey, 'logs');
      setup({});
      const table = await screen.findByTestId('logRows');
      expect(table).toBeInTheDocument();
-      store.get = oldGet;
    });

    it('should change visualisation to table on toggle (elastic)', async () => {
--- a/public/app/features/explore/Logs/Logs.tsx
+++ b/public/app/features/explore/Logs/Logs.tsx
--- a/public/app/features/explore/state/main.ts
+++ b/public/app/features/explore/state/main.ts
@ -270,7 +270,7 @@ export const exploreReducer = (state = initialExploreState, action: AnyAction):
    };
  }

-  if (initializeExplore.pending.match(action)) {
+  if (initializeExplore?.pending.match(action)) {
    const initialPanes = Object.entries(state.panes);
    const before = initialPanes.slice(0, action.meta.arg.position);
    const after = initialPanes.slice(before.length);