From 6b5ebf2b4b0d7e083f5766e35ba29480bd7ea3d3 Mon Sep 17 00:00:00 2001
From: Gareth Dawson <gareth.dawson@grafana.com>
Date: Tue, 29 Nov 2022 14:07:34 +0000
Subject: [PATCH] Loki: Add improvements to loki label browser (#59387)

* improvements

* refactor label browser modal

* feat(label-browser-modal): fetch labels on modal open

* apply suggestions

* check for log labels after languageProvider start

Co-authored-by: Matias Chomicki <matyax@gmail.com>
---
 .../loki/components/LokiQueryEditor.tsx       | 51 ++++-------------
 .../components/LabelBrowserModal.test.tsx     | 10 +++-
 .../components/LabelBrowserModal.tsx          | 57 ++++++++++++-------
 3 files changed, 55 insertions(+), 63 deletions(-)

diff --git a/public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx b/public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx
index e33a4c27359..04ca1cbba03 100644
--- a/public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx
+++ b/public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx
@@ -31,7 +31,6 @@ export const LokiQueryEditor = React.memo<LokiQueryEditorProps>((props) => {
   const [queryPatternsModalOpen, setQueryPatternsModalOpen] = useState(false);
   const [dataIsStale, setDataIsStale] = useState(false);
   const [labelBrowserVisible, setLabelBrowserVisible] = useState(false);
-  const [labelsLoaded, setLabelsLoaded] = useState(false);
   const { flag: explain, setFlag: setExplain } = useFlag(lokiQueryEditorExplainKey);
 
   const query = getQueryWithDefaults(props.query);
@@ -73,30 +72,10 @@ export const LokiQueryEditor = React.memo<LokiQueryEditorProps>((props) => {
     onChange(query);
   };
 
-  const onClickChooserButton = () => {
+  const onClickLabelBrowserButton = () => {
     setLabelBrowserVisible((visible) => !visible);
   };
 
-  const getChooserText = (logLabelsLoaded: boolean, hasLogLabels: boolean) => {
-    if (!logLabelsLoaded) {
-      return 'Loading labels...';
-    }
-    if (!hasLogLabels) {
-      return '(No labels found)';
-    }
-    return 'Label browser';
-  };
-
-  useEffect(() => {
-    datasource.languageProvider.start().then(() => {
-      setLabelsLoaded(true);
-    });
-  }, [datasource]);
-
-  const hasLogLabels = datasource.languageProvider.getLabelKeys().length > 0;
-  const labelBrowserText = getChooserText(labelsLoaded, hasLogLabels);
-  const buttonDisabled = !(labelsLoaded && hasLogLabels);
-
   return (
     <>
       <ConfirmModal
@@ -119,16 +98,16 @@ export const LokiQueryEditor = React.memo<LokiQueryEditorProps>((props) => {
         onChange={onChange}
         onAddQuery={onAddQuery}
       />
+      <LabelBrowserModal
+        isOpen={labelBrowserVisible}
+        datasource={datasource}
+        query={query}
+        app={app}
+        onClose={() => setLabelBrowserVisible(false)}
+        onChange={onChangeInternal}
+        onRunQuery={onRunQuery}
+      />
       <EditorHeader>
-        <LabelBrowserModal
-          isOpen={labelBrowserVisible}
-          languageProvider={datasource.languageProvider}
-          query={query}
-          app={app}
-          onClose={() => setLabelBrowserVisible(false)}
-          onChange={onChangeInternal}
-          onRunQuery={onRunQuery}
-        />
         <Stack gap={1}>
           <Button
             aria-label={selectors.components.QueryBuilder.queryPatterns}
@@ -149,14 +128,8 @@ export const LokiQueryEditor = React.memo<LokiQueryEditorProps>((props) => {
           >
             Kick start your query
           </Button>
-          <Button
-            variant="secondary"
-            size="sm"
-            onClick={onClickChooserButton}
-            disabled={buttonDisabled}
-            data-testid="label-browser-button"
-          >
-            {labelBrowserText}
+          <Button variant="secondary" size="sm" onClick={onClickLabelBrowserButton} data-testid="label-browser-button">
+            Label browser
           </Button>
         </Stack>
         <QueryHeaderSwitch label="Explain" value={explain} onChange={onExplainChange} />
diff --git a/public/app/plugins/datasource/loki/querybuilder/components/LabelBrowserModal.test.tsx b/public/app/plugins/datasource/loki/querybuilder/components/LabelBrowserModal.test.tsx
index 3f95c0b1bc6..e36470b7d36 100644
--- a/public/app/plugins/datasource/loki/querybuilder/components/LabelBrowserModal.test.tsx
+++ b/public/app/plugins/datasource/loki/querybuilder/components/LabelBrowserModal.test.tsx
@@ -20,7 +20,7 @@ describe('LabelBrowserModal', () => {
 
     props = {
       isOpen: true,
-      languageProvider: datasource.languageProvider,
+      datasource: datasource,
       query: {} as LokiQuery,
       onClose: jest.fn(),
       onChange: jest.fn(),
@@ -30,13 +30,17 @@ describe('LabelBrowserModal', () => {
     jest.spyOn(datasource, 'metadataRequest').mockResolvedValue({});
   });
 
-  it('renders the label browser modal when open', () => {
+  it('renders the label browser modal when open', async () => {
     render(<LabelBrowserModal {...props} />);
+
+    expect(await screen.findByText(/Loading/)).not.toBeInTheDocument();
+
     expect(screen.getByRole('heading', { name: /label browser/i })).toBeInTheDocument();
   });
 
-  it("doesn't render the label browser modal when closed", () => {
+  it("doesn't render the label browser modal when closed", async () => {
     render(<LabelBrowserModal {...props} isOpen={false} />);
+
     expect(screen.queryByRole('heading', { name: /label browser/i })).toBeNull();
   });
 });
diff --git a/public/app/plugins/datasource/loki/querybuilder/components/LabelBrowserModal.tsx b/public/app/plugins/datasource/loki/querybuilder/components/LabelBrowserModal.tsx
index 4a0b0ba6bdd..7e86717196d 100644
--- a/public/app/plugins/datasource/loki/querybuilder/components/LabelBrowserModal.tsx
+++ b/public/app/plugins/datasource/loki/querybuilder/components/LabelBrowserModal.tsx
@@ -1,16 +1,16 @@
-import React from 'react';
+import React, { useState, useEffect } from 'react';
 
 import { CoreApp } from '@grafana/data';
-import { Modal } from '@grafana/ui';
+import { LoadingPlaceholder, Modal } from '@grafana/ui';
 import { LocalStorageValueProvider } from 'app/core/components/LocalStorageValueProvider';
 
-import LanguageProvider from '../../LanguageProvider';
 import { LokiLabelBrowser } from '../../components/LokiLabelBrowser';
+import { LokiDatasource } from '../../datasource';
 import { LokiQuery } from '../../types';
 
 export interface Props {
   isOpen: boolean;
-  languageProvider: LanguageProvider;
+  datasource: LokiDatasource;
   query: LokiQuery;
   app?: CoreApp;
   onClose: () => void;
@@ -19,13 +19,24 @@ export interface Props {
 }
 
 export const LabelBrowserModal = (props: Props) => {
-  const { isOpen, onClose, languageProvider, app } = props;
-
+  const { isOpen, onClose, datasource, app } = props;
+  const [labelsLoaded, setLabelsLoaded] = useState(false);
+  const [hasLogLabels, setHasLogLabels] = useState(false);
   const LAST_USED_LABELS_KEY = 'grafana.datasources.loki.browser.labels';
 
+  useEffect(() => {
+    if (!isOpen) {
+      return;
+    }
+
+    datasource.languageProvider.start().then(() => {
+      setLabelsLoaded(true);
+      setHasLogLabels(datasource.languageProvider.getLabelKeys().length > 0);
+    });
+  }, [datasource, isOpen]);
+
   const changeQuery = (value: string) => {
     const { query, onChange, onRunQuery } = props;
-
     const nextQuery = { ...query, expr: value };
     onChange(nextQuery);
     onRunQuery();
@@ -38,20 +49,24 @@ export const LabelBrowserModal = (props: Props) => {
 
   return (
     <Modal isOpen={isOpen} title="Label browser" onDismiss={onClose}>
-      <LocalStorageValueProvider<string[]> storageKey={LAST_USED_LABELS_KEY} defaultValue={[]}>
-        {(lastUsedLabels, onLastUsedLabelsSave, onLastUsedLabelsDelete) => {
-          return (
-            <LokiLabelBrowser
-              languageProvider={languageProvider}
-              onChange={onChange}
-              lastUsedLabels={lastUsedLabels}
-              storeLastUsedLabels={onLastUsedLabelsSave}
-              deleteLastUsedLabels={onLastUsedLabelsDelete}
-              app={app}
-            />
-          );
-        }}
-      </LocalStorageValueProvider>
+      {!labelsLoaded && <LoadingPlaceholder text="Loading labels..." />}
+      {labelsLoaded && !hasLogLabels && <p>No labels found.</p>}
+      {labelsLoaded && hasLogLabels && (
+        <LocalStorageValueProvider<string[]> storageKey={LAST_USED_LABELS_KEY} defaultValue={[]}>
+          {(lastUsedLabels, onLastUsedLabelsSave, onLastUsedLabelsDelete) => {
+            return (
+              <LokiLabelBrowser
+                languageProvider={datasource.languageProvider}
+                onChange={onChange}
+                lastUsedLabels={lastUsedLabels}
+                storeLastUsedLabels={onLastUsedLabelsSave}
+                deleteLastUsedLabels={onLastUsedLabelsDelete}
+                app={app}
+              />
+            );
+          }}
+        </LocalStorageValueProvider>
+      )}
     </Modal>
   );
 };