From 6a125fd59f9a0488c5400e3f818f3d14ee8529a3 Mon Sep 17 00:00:00 2001
From: Matias Chomicki <matyax@gmail.com>
Date: Thu, 13 Jun 2024 18:56:50 +0200
Subject: [PATCH] Logs panel: do not pass default handlers if context is not
 defined (#89174)

---
 .betterer.results                             |  3 ++
 .../app/plugins/panel/logs/LogsPanel.test.tsx | 49 ++++++++++++++++++-
 public/app/plugins/panel/logs/LogsPanel.tsx   |  8 ++-
 3 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/.betterer.results b/.betterer.results
index c5e1da0bb83..2cdb78262b7 100644
--- a/.betterer.results
+++ b/.betterer.results
@@ -7124,6 +7124,9 @@ exports[`better eslint`] = {
       [0, 0, 0, "Styles should be written using objects.", "6"],
       [0, 0, 0, "Styles should be written using objects.", "7"]
     ],
+    "public/app/plugins/panel/logs/LogsPanel.test.tsx:5381": [
+      [0, 0, 0, "* import is invalid because \'Layout,HorizontalGroup,VerticalGroup\' from \'@grafana/ui\' is restricted from being used by a pattern. Use Stack component instead.", "0"]
+    ],
     "public/app/plugins/panel/logs/types.ts:5381": [
       [0, 0, 0, "Do not re-export imported variable (\`./panelcfg.gen\`)", "0"]
     ],
diff --git a/public/app/plugins/panel/logs/LogsPanel.test.tsx b/public/app/plugins/panel/logs/LogsPanel.test.tsx
index 400423a8842..f462094b27b 100644
--- a/public/app/plugins/panel/logs/LogsPanel.test.tsx
+++ b/public/app/plugins/panel/logs/LogsPanel.test.tsx
@@ -13,6 +13,7 @@ import {
   LogsDedupStrategy,
   EventBusSrv,
 } from '@grafana/data';
+import * as grafanaUI from '@grafana/ui';
 import * as styles from 'app/features/logs/components/getLogRowStyles';
 import { LogRowContextModal } from 'app/features/logs/components/log-context/LogRowContextModal';
 
@@ -347,7 +348,7 @@ describe('LogsPanel', () => {
       }),
     ];
 
-    it('allow to filter for a value or filter out a value', async () => {
+    it('allows to filter for a value or filter out a value', async () => {
       const filterForMock = jest.fn();
       const filterOutMock = jest.fn();
       const isFilterLabelActiveMock = jest.fn();
@@ -380,6 +381,50 @@ describe('LogsPanel', () => {
 
       expect(isFilterLabelActiveMock).toHaveBeenCalledTimes(1);
     });
+
+    describe('invalid handlers', () => {
+      it('does not show the controls if onAddAdHocFilter is not defined', async () => {
+        jest.spyOn(grafanaUI, 'usePanelContext').mockReturnValue({
+          eventsScope: 'global',
+          eventBus: new EventBusSrv(),
+        });
+
+        setup({
+          data: {
+            series,
+          },
+        });
+
+        expect(await screen.findByRole('row')).toBeInTheDocument();
+
+        await userEvent.click(screen.getByText('logline text'));
+
+        expect(screen.queryByLabelText('Filter for value in query A')).not.toBeInTheDocument();
+        expect(screen.queryByLabelText('Filter out value in query A')).not.toBeInTheDocument();
+      });
+      it('shows the controls if onAddAdHocFilter is defined', async () => {
+        jest.spyOn(grafanaUI, 'usePanelContext').mockReturnValue({
+          eventsScope: 'global',
+          eventBus: new EventBusSrv(),
+          onAddAdHocFilter: jest.fn(),
+        });
+
+        setup({
+          data: {
+            series,
+          },
+        });
+
+        expect(await screen.findByRole('row')).toBeInTheDocument();
+
+        await userEvent.click(screen.getByText('logline text'));
+
+        expect(await screen.findByText('common_app')).toBeInTheDocument();
+
+        expect(screen.getByLabelText('Filter for value in query A')).toBeInTheDocument();
+        expect(screen.getByLabelText('Filter out value in query A')).toBeInTheDocument();
+      });
+    });
   });
 });
 
@@ -414,7 +459,7 @@ const setup = (propsOverrides?: {}) => {
       prettifyLogMessage: false,
       sortOrder: LogsSortOrder.Descending,
       dedupStrategy: LogsDedupStrategy.none,
-      enableLogDetails: false,
+      enableLogDetails: true,
       showLogContextToggle: false,
     },
     title: 'Logs panel',
diff --git a/public/app/plugins/panel/logs/LogsPanel.tsx b/public/app/plugins/panel/logs/LogsPanel.tsx
index 0bafecd0004..d2cefdae7ce 100644
--- a/public/app/plugins/panel/logs/LogsPanel.tsx
+++ b/public/app/plugins/panel/logs/LogsPanel.tsx
@@ -249,7 +249,7 @@ export const LogsPanel = ({
     [scrollElement]
   );
 
-  const defaultOnClickFilterLabel = useCallback(
+  const handleOnClickFilterLabel = useCallback(
     (key: string, value: string) => {
       onAddAdHocFilter?.({
         key,
@@ -260,7 +260,7 @@ export const LogsPanel = ({
     [onAddAdHocFilter]
   );
 
-  const defaultOnClickFilterOutLabel = useCallback(
+  const handleOnClickFilterOutLabel = useCallback(
     (key: string, value: string) => {
       onAddAdHocFilter?.({
         key,
@@ -285,6 +285,10 @@ export const LogsPanel = ({
     </div>
   );
 
+  // Passing callbacks control the display of the filtering buttons. We want to pass it only if onAddAdHocFilter is defined.
+  const defaultOnClickFilterLabel = onAddAdHocFilter ? handleOnClickFilterLabel : undefined;
+  const defaultOnClickFilterOutLabel = onAddAdHocFilter ? handleOnClickFilterOutLabel : undefined;
+
   return (
     <>
       {contextRow && (