From 3850f7b334132fcbbd6e58a40aac2e1df1e87d72 Mon Sep 17 00:00:00 2001
From: Kristina <kristina.durivage@grafana.com>
Date: Wed, 14 Jun 2023 08:34:06 -0500
Subject: [PATCH] Correlations: UX updates (#69313)

* Round 1

* Add comma

* Remove validate query from editor

* Slightly change headings

* Change text
---
 .../correlations/CorrelationsPage.tsx         |  10 +-
 .../ConfigureCorrelationBasicInfoForm.tsx     |   6 +-
 .../Forms/ConfigureCorrelationSourceForm.tsx  |   9 +-
 .../Forms/ConfigureCorrelationTargetForm.tsx  |   6 +-
 .../Forms/CorrelationFormNavigation.tsx       |   4 +-
 .../Forms/QueryEditorField.test.tsx           | 102 +-----------------
 .../correlations/Forms/QueryEditorField.tsx   |  94 +---------------
 7 files changed, 29 insertions(+), 202 deletions(-)

diff --git a/public/app/features/correlations/CorrelationsPage.tsx b/public/app/features/correlations/CorrelationsPage.tsx
index db075c534ea..dd93e7884a6 100644
--- a/public/app/features/correlations/CorrelationsPage.tsx
+++ b/public/app/features/correlations/CorrelationsPage.tsx
@@ -15,6 +15,7 @@ import {
   type Column,
   type CellProps,
   type SortByFn,
+  Icon,
 } from '@grafana/ui';
 import { Page } from 'app/core/components/Page/Page';
 import { contextSrv } from 'app/core/core';
@@ -153,7 +154,14 @@ export default function CorrelationsPage() {
   return (
     <Page
       navModel={navModel}
-      subTitle="Define how data living in different data sources relates to each other."
+      subTitle={
+        <>
+          Define how data living in different data sources relates to each other. Read more in the{' '}
+          <a href="https://grafana.com/docs/grafana/next/administration/correlations/" target="_blank" rel="noreferrer">
+            documentation <Icon name="external-link-alt" />
+          </a>
+        </>
+      }
       actions={addButton}
     >
       <Page.Contents>
diff --git a/public/app/features/correlations/Forms/ConfigureCorrelationBasicInfoForm.tsx b/public/app/features/correlations/Forms/ConfigureCorrelationBasicInfoForm.tsx
index 00a120c5817..7cd8fd739de 100644
--- a/public/app/features/correlations/Forms/ConfigureCorrelationBasicInfoForm.tsx
+++ b/public/app/features/correlations/Forms/ConfigureCorrelationBasicInfoForm.tsx
@@ -25,12 +25,12 @@ export const ConfigureCorrelationBasicInfoForm = () => {
 
   return (
     <>
-      <FieldSet label="Define correlation name (1/3)">
-        <p>The name of the correlation is used as the label of the link.</p>
+      <FieldSet label="Define correlation label (Step 1 of 3)">
+        <p>Define text that will describe the correlation.</p>
         <input type="hidden" {...register('config.type')} />
         <Field
           label="Label"
-          description="This name is be used as the label of the link button"
+          description="This name will be used as the label for the correlation. This will show as button text, a menu item, or hover text on a link."
           className={styles.label}
           invalid={!!formState.errors.label}
           error={formState.errors.label?.message}
diff --git a/public/app/features/correlations/Forms/ConfigureCorrelationSourceForm.tsx b/public/app/features/correlations/Forms/ConfigureCorrelationSourceForm.tsx
index 4588e881a7b..e44fd529290 100644
--- a/public/app/features/correlations/Forms/ConfigureCorrelationSourceForm.tsx
+++ b/public/app/features/correlations/Forms/ConfigureCorrelationSourceForm.tsx
@@ -36,10 +36,13 @@ export const ConfigureCorrelationSourceForm = () => {
   );
   return (
     <>
-      <FieldSet label="Configure source data source (3/3)">
+      <FieldSet
+        label={`Configure the data source that will link to ${
+          getDatasourceSrv().getInstanceSettings(correlation?.targetUID)?.name
+        } (Step 3 of 3)`}
+      >
         <p>
-          Links are displayed with results of the selected origin source data. They show along with the value of the
-          provided <em>results field</em>.
+          Define what data source will display the correlation, and what data will replace previously defined variables.
         </p>
         <Controller
           control={control}
diff --git a/public/app/features/correlations/Forms/ConfigureCorrelationTargetForm.tsx b/public/app/features/correlations/Forms/ConfigureCorrelationTargetForm.tsx
index 58687a4b8a7..2643e7cfb49 100644
--- a/public/app/features/correlations/Forms/ConfigureCorrelationTargetForm.tsx
+++ b/public/app/features/correlations/Forms/ConfigureCorrelationTargetForm.tsx
@@ -16,8 +16,10 @@ export const ConfigureCorrelationTargetForm = () => {
 
   return (
     <>
-      <FieldSet label="Setup target query (2/3)">
-        <p>Clicking on a link runs a provided target query.</p>
+      <FieldSet label="Setup the target for the correlation (Step 2 of 3)">
+        <p>
+          Define what data source the correlation will link to, and what query will run when the correlation is clicked.
+        </p>
         <Controller
           control={control}
           name="targetUID"
diff --git a/public/app/features/correlations/Forms/CorrelationFormNavigation.tsx b/public/app/features/correlations/Forms/CorrelationFormNavigation.tsx
index d3ce7888e61..287fe2564ca 100644
--- a/public/app/features/correlations/Forms/CorrelationFormNavigation.tsx
+++ b/public/app/features/correlations/Forms/CorrelationFormNavigation.tsx
@@ -17,13 +17,13 @@ export const CorrelationFormNavigation = () => {
   );
 
   const NextPage = (
-    <Button variant="secondary" type="submit">
+    <Button variant="primary" type="submit">
       Next
     </Button>
   );
 
   return (
-    <HorizontalGroup justify="flex-end">
+    <HorizontalGroup justify="flex-start">
       {currentPage > 0 ? (
         <Button variant="secondary" onClick={prevPage}>
           Back
diff --git a/public/app/features/correlations/Forms/QueryEditorField.test.tsx b/public/app/features/correlations/Forms/QueryEditorField.test.tsx
index b5989a0f077..e9884198705 100644
--- a/public/app/features/correlations/Forms/QueryEditorField.test.tsx
+++ b/public/app/features/correlations/Forms/QueryEditorField.test.tsx
@@ -1,9 +1,8 @@
-import { fireEvent, render, screen, waitFor, waitForElementToBeRemoved } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import React, { ReactNode } from 'react';
 import { FormProvider, useForm } from 'react-hook-form';
 import { MockDataSourceApi } from 'test/mocks/datasource_srv';
 
-import { LoadingState } from '@grafana/data';
 import { setDataSourceSrv } from '@grafana/runtime';
 import { MockDataSourceSrv } from 'app/features/alerting/unified/mocks';
 
@@ -34,19 +33,6 @@ const renderWithContext = (
   render(<Wrapper>{children}</Wrapper>);
 };
 
-const initiateDsApi = () => {
-  const dsApi = new MockDataSourceApi('dsApiMock');
-  dsApi.components = {
-    QueryEditor: () => <>query editor</>,
-  };
-
-  renderWithContext(<QueryEditorField name="query" dsUid="randomDsUid" />, async () => {
-    return dsApi;
-  });
-
-  return dsApi;
-};
-
 describe('QueryEditorField', () => {
   afterAll(() => {
     jest.restoreAllMocks();
@@ -81,90 +67,4 @@ describe('QueryEditorField', () => {
       await screen.findByRole('alert', { name: 'Data source does not export a query editor.' })
     ).toBeInTheDocument();
   });
-
-  describe('Query validation', () => {
-    it('should result in succeeded validation if LoadingState.Done and data is available', async () => {
-      const dsApi = initiateDsApi();
-
-      await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i));
-
-      dsApi.result = {
-        data: [
-          {
-            name: 'test',
-            fields: [],
-            length: 1,
-          },
-        ],
-        state: LoadingState.Done,
-      };
-
-      fireEvent.click(screen.getByRole('button', { name: /Validate query$/i }));
-
-      await waitFor(() => {
-        expect(screen.getByText('This query is valid.')).toBeInTheDocument();
-      });
-    });
-
-    it('should result in failed validation if LoadingState.Error and data is not available', async () => {
-      const dsApi = initiateDsApi();
-
-      await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i));
-
-      dsApi.result = {
-        data: [],
-        state: LoadingState.Error,
-      };
-
-      fireEvent.click(screen.getByRole('button', { name: /Validate query$/i }));
-
-      await waitFor(() => {
-        const alertEl = screen.getByRole('alert');
-        expect(alertEl).toBeInTheDocument();
-        expect(alertEl).toHaveTextContent(/this query is not valid/i);
-      });
-    });
-
-    it('should result in failed validation if LoadingState.Error and data is available', async () => {
-      const dsApi = initiateDsApi();
-
-      await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i));
-
-      dsApi.result = {
-        data: [
-          {
-            name: 'test',
-            fields: [],
-            length: 1,
-          },
-        ],
-        state: LoadingState.Error,
-      };
-
-      fireEvent.click(screen.getByRole('button', { name: /Validate query$/i }));
-
-      await waitFor(() => {
-        const alertEl = screen.getByRole('alert');
-        expect(alertEl).toBeInTheDocument();
-        expect(alertEl).toHaveTextContent(/this query is not valid/i);
-      });
-    });
-
-    it('should result in failed validation if result with LoadingState.Done and data is not available', async () => {
-      const dsApi = initiateDsApi();
-
-      await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i));
-
-      dsApi.result = {
-        data: [],
-        state: LoadingState.Done,
-      };
-
-      fireEvent.click(screen.getByRole('button', { name: /Validate query$/i }));
-
-      await waitFor(() => {
-        expect(screen.getByText('This query is not valid.')).toBeInTheDocument();
-      });
-    });
-  });
 });
diff --git a/public/app/features/correlations/Forms/QueryEditorField.tsx b/public/app/features/correlations/Forms/QueryEditorField.tsx
index 938749a7c22..f246c4fc93e 100644
--- a/public/app/features/correlations/Forms/QueryEditorField.tsx
+++ b/public/app/features/correlations/Forms/QueryEditorField.tsx
@@ -1,24 +1,10 @@
-import { css } from '@emotion/css';
-import React, { useState } from 'react';
+import React from 'react';
 import { Controller } from 'react-hook-form';
 import { useAsync } from 'react-use';
 
-import { CoreApp, DataQuery, getDefaultTimeRange, GrafanaTheme2 } from '@grafana/data';
+import { CoreApp } from '@grafana/data';
 import { getDataSourceSrv } from '@grafana/runtime';
-import {
-  Field,
-  LoadingPlaceholder,
-  Alert,
-  Button,
-  HorizontalGroup,
-  Icon,
-  FieldValidationMessage,
-  useStyles2,
-} from '@grafana/ui';
-
-import { generateKey } from '../../../core/utils/explore';
-import { QueryTransaction } from '../../../types';
-import { runRequest } from '../../query/state/runRequest';
+import { Field, LoadingPlaceholder, Alert } from '@grafana/ui';
 
 interface Props {
   dsUid?: string;
@@ -27,19 +13,7 @@ interface Props {
   error?: string;
 }
 
-function getStyle(theme: GrafanaTheme2) {
-  return {
-    valid: css`
-      color: ${theme.colors.success.text};
-    `,
-  };
-}
-
 export const QueryEditorField = ({ dsUid, invalid, error, name }: Props) => {
-  const [isValidQuery, setIsValidQuery] = useState<boolean | undefined>(undefined);
-
-  const style = useStyles2(getStyle);
-
   const {
     value: datasource,
     loading: dsLoading,
@@ -53,53 +27,6 @@ export const QueryEditorField = ({ dsUid, invalid, error, name }: Props) => {
 
   const QueryEditor = datasource?.components?.QueryEditor;
 
-  const handleValidation = (value: DataQuery) => {
-    const interval = '1s';
-    const intervalMs = 1000;
-    const id = generateKey();
-    const queries = [{ ...value, refId: 'A' }];
-
-    const transaction: QueryTransaction = {
-      queries,
-      request: {
-        app: CoreApp.Correlations,
-        timezone: 'utc',
-        startTime: Date.now(),
-        interval,
-        intervalMs,
-        targets: queries,
-        range: getDefaultTimeRange(),
-        requestId: 'correlations_' + id,
-        scopedVars: {
-          __interval: { text: interval, value: interval },
-          __interval_ms: { text: intervalMs, value: intervalMs },
-        },
-      },
-      id,
-      done: false,
-    };
-
-    if (datasource) {
-      runRequest(datasource, transaction.request).subscribe((panelData) => {
-        if (
-          !panelData ||
-          panelData.state === 'Error' ||
-          (panelData.state === 'Done' && panelData.series.length === 0)
-        ) {
-          setIsValidQuery(false);
-        } else if (
-          panelData.state === 'Done' &&
-          panelData.series.length > 0 &&
-          Boolean(panelData.series.find((element) => element.length > 0))
-        ) {
-          setIsValidQuery(true);
-        } else {
-          setIsValidQuery(undefined);
-        }
-      });
-    }
-  };
-
   return (
     <Field
       label="Query"
@@ -147,27 +74,14 @@ export const QueryEditorField = ({ dsUid, invalid, error, name }: Props) => {
           return (
             <>
               <QueryEditor
+                onRunQuery={() => {}}
                 app={CoreApp.Correlations}
-                onRunQuery={() => handleValidation(value)}
                 onChange={(value) => {
-                  setIsValidQuery(undefined);
                   onChange(value);
                 }}
                 datasource={datasource}
                 query={value}
               />
-              <HorizontalGroup justify="flex-end">
-                {isValidQuery ? (
-                  <div className={style.valid}>
-                    <Icon name="check" /> This query is valid.
-                  </div>
-                ) : isValidQuery === false ? (
-                  <FieldValidationMessage>This query is not valid.</FieldValidationMessage>
-                ) : null}
-                <Button variant="secondary" icon={'check'} type="button" onClick={() => handleValidation(value)}>
-                  Validate query
-                </Button>
-              </HorizontalGroup>
             </>
           );
         }}