diff --git a/public/app/features/correlations/CorrelationsPage.test.tsx b/public/app/features/correlations/CorrelationsPage.test.tsx index b3ac6243f94..81827aee913 100644 --- a/public/app/features/correlations/CorrelationsPage.test.tsx +++ b/public/app/features/correlations/CorrelationsPage.test.tsx @@ -1,9 +1,10 @@ -import { render, waitFor, screen, fireEvent } from '@testing-library/react'; +import { render, waitFor, screen, fireEvent, waitForElementToBeRemoved, within, Matcher } from '@testing-library/react'; import { merge, uniqueId } from 'lodash'; import React from 'react'; import { DeepPartial } from 'react-hook-form'; import { Provider } from 'react-redux'; import { Observable } from 'rxjs'; +import { MockDataSourceApi } from 'test/mocks/datasource_srv'; import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; import { DataSourcePluginMeta } from '@grafana/data'; @@ -127,19 +128,93 @@ const renderWithContext = async ( } as unknown as BackendSrv; const grafanaContext = getGrafanaContextMock({ backend }); - setDataSourceSrv(new MockDataSourceSrv(datasources)); + const dsServer = new MockDataSourceSrv(datasources); + dsServer.get = (name: string) => { + const dsApi = new MockDataSourceApi(name); + dsApi.components = { + QueryEditor: () => <>{name} query editor, + }; + return Promise.resolve(dsApi); + }; - render( + setDataSourceSrv(dsServer); + + const renderResult = render( - + , + { + queries: { + /** + * Gets all the rows in the table having the given text in the given column + */ + queryRowsByCellValue: ( + container: HTMLElement, + columnName: Matcher, + textValue: Matcher + ): HTMLTableRowElement[] => { + const table = within(container).getByRole('table'); + const headers = within(table).getAllByRole('columnheader'); + const headerIndex = headers.findIndex((h) => { + return within(h).queryByText(columnName); + }); + + // the first rowgroup is the header + const tableBody = within(table).getAllByRole('rowgroup')[1]; + + return within(tableBody) + .getAllByRole('row') + .filter((row) => { + const rowCells = within(row).getAllByRole('cell'); + const cell = rowCells[headerIndex]; + return within(cell).queryByText(textValue); + }); + }, + /** + * Gets all the cells in the table for the given column name + */ + queryCellsByColumnName: (container: HTMLElement, columnName: Matcher) => { + const table = within(container).getByRole('table'); + const headers = within(table).getAllByRole('columnheader'); + const headerIndex = headers.findIndex((h) => { + return within(h).queryByText(columnName); + }); + const tbody = table.querySelector('tbody'); + if (!tbody) { + return []; + } + return within(tbody) + .getAllByRole('row') + .map((r) => { + const cells = within(r).getAllByRole('cell'); + return cells[headerIndex]; + }); + }, + /** + * Gets the table header cell matching the given name + */ + getHeaderByName: (container: HTMLElement, columnName: Matcher): HTMLTableCellElement => { + const table = within(container).getByRole('table'); + const headers = within(table).getAllByRole('columnheader'); + const header = headers.find((h) => { + return within(h).queryByText(columnName); + }); + if (!header) { + throw new Error(`Could not find header with name ${columnName}`); + } + return header; + }, + }, + } ); await waitFor(() => { expect(screen.queryByText('Loading')).not.toBeInTheDocument(); }); + + return renderResult; }; beforeAll(() => { @@ -194,6 +269,9 @@ describe('CorrelationsPage', () => { fireEvent.click(CTAButton); + // wait for the form to be rendered and query editor to be mounted + await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i)); + // form's submit button expect(screen.getByRole('button', { name: /add$/i })).toBeInTheDocument(); }); @@ -218,12 +296,14 @@ describe('CorrelationsPage', () => { fireEvent.keyDown(screen.getByLabelText(/^target$/i), { keyCode: 40 }); fireEvent.click(screen.getByText('prometheus')); + fireEvent.change(screen.getByRole('textbox', { name: /target field/i }), { target: { value: 'Line' } }); + + await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i)); + fireEvent.click(screen.getByRole('button', { name: /add$/i })); // Waits for the form to be removed, meaning the correlation got successfully saved - await waitFor(() => { - expect(screen.queryByRole('button', { name: /add$/i })).not.toBeInTheDocument(); - }); + await waitForElementToBeRemoved(() => screen.queryByRole('button', { name: /add$/i })); // the table showing correlations should have appeared expect(screen.getByRole('table')).toBeInTheDocument(); @@ -231,8 +311,12 @@ describe('CorrelationsPage', () => { }); describe('With correlations', () => { + let queryRowsByCellValue: (columnName: Matcher, textValue: Matcher) => HTMLTableRowElement[]; + let getHeaderByName: (columnName: Matcher) => HTMLTableCellElement; + let queryCellsByColumnName: (columnName: Matcher) => HTMLTableCellElement[]; + beforeEach(async () => { - await renderWithContext( + const renderResult = await renderWithContext( { loki: mockDataSource( { @@ -275,16 +359,53 @@ describe('CorrelationsPage', () => { } ), }, - [{ sourceUID: 'loki', targetUID: 'loki', uid: '1', label: 'Some label' }] + [ + { + sourceUID: 'loki', + targetUID: 'loki', + uid: '1', + label: 'Some label', + config: { field: 'line', target: {}, type: 'query' }, + }, + { + sourceUID: 'prometheus', + targetUID: 'loki', + uid: '2', + label: 'Prometheus to Loki', + config: { field: 'label', target: {}, type: 'query' }, + }, + ] ); + queryRowsByCellValue = renderResult.queryRowsByCellValue; + queryCellsByColumnName = renderResult.queryCellsByColumnName; + getHeaderByName = renderResult.getHeaderByName; }); it('shows a table with correlations', async () => { - await renderWithContext(); - expect(screen.getByRole('table')).toBeInTheDocument(); }); + it('correctly sorts by source', async () => { + const sourceHeader = getHeaderByName('Source'); + fireEvent.click(sourceHeader); + let cells = queryCellsByColumnName('Source'); + cells.forEach((cell, i, allCells) => { + const prevCell = allCells[i - 1]; + if (prevCell && prevCell.textContent) { + expect(cell.textContent?.localeCompare(prevCell.textContent)).toBeGreaterThanOrEqual(0); + } + }); + + fireEvent.click(sourceHeader); + cells = queryCellsByColumnName('Source'); + cells.forEach((cell, i, allCells) => { + const prevCell = allCells[i - 1]; + if (prevCell && prevCell.textContent) { + expect(cell.textContent?.localeCompare(prevCell.textContent)).toBeLessThanOrEqual(0); + } + }); + }); + it('correctly adds correlations', async () => { const addNewButton = screen.getByRole('button', { name: /add new/i }); expect(addNewButton).toBeInTheDocument(); @@ -295,18 +416,20 @@ describe('CorrelationsPage', () => { // set source datasource picker value fireEvent.keyDown(screen.getByLabelText(/^source$/i), { keyCode: 40 }); - fireEvent.click(screen.getByText('prometheus')); + fireEvent.click(within(screen.getByLabelText('Select options menu')).getByText('prometheus')); // set target datasource picker value fireEvent.keyDown(screen.getByLabelText(/^target$/i), { keyCode: 40 }); fireEvent.click(screen.getByText('elastic')); + fireEvent.change(screen.getByRole('textbox', { name: /target field/i }), { target: { value: 'Line' } }); + + await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i)); + fireEvent.click(screen.getByRole('button', { name: /add$/i })); // the form should get removed after successful submissions - await waitFor(() => { - expect(screen.queryByRole('button', { name: /add$/i })).not.toBeInTheDocument(); - }); + await waitForElementToBeRemoved(() => screen.queryByRole('button', { name: /add$/i })); }); it('correctly closes the form when clicking on the close icon', async () => { @@ -316,35 +439,37 @@ describe('CorrelationsPage', () => { fireEvent.click(screen.getByRole('button', { name: /close$/i })); - await waitFor(() => { - expect(screen.queryByRole('button', { name: /add$/i })).not.toBeInTheDocument(); - }); + expect(screen.queryByRole('button', { name: /add$/i })).not.toBeInTheDocument(); }); it('correctly deletes correlations', async () => { // A row with the correlation should exist expect(screen.getByRole('cell', { name: /some label/i })).toBeInTheDocument(); - const deleteButton = screen.getByRole('button', { name: /delete correlation/i }); + const tableRows = queryRowsByCellValue('Source', 'loki'); + + const deleteButton = within(tableRows[0]).getByRole('button', { name: /delete correlation/i }); expect(deleteButton).toBeInTheDocument(); fireEvent.click(deleteButton); - const confirmButton = screen.getByRole('button', { name: /delete$/i }); + const confirmButton = within(tableRows[0]).getByRole('button', { name: /delete$/i }); expect(confirmButton).toBeInTheDocument(); fireEvent.click(confirmButton); - await waitFor(() => { - expect(screen.queryByRole('cell', { name: /some label/i })).not.toBeInTheDocument(); - }); + await waitForElementToBeRemoved(() => screen.queryByRole('cell', { name: /some label$/i })); }); it('correctly edits correlations', async () => { - const rowExpanderButton = screen.getByRole('button', { name: /toggle row expanded/i }); + const tableRows = queryRowsByCellValue('Source', 'loki'); + + const rowExpanderButton = within(tableRows[0]).getByRole('button', { name: /toggle row expanded/i }); fireEvent.click(rowExpanderButton); + await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i)); + fireEvent.change(screen.getByRole('textbox', { name: /label/i }), { target: { value: 'edited label' } }); fireEvent.change(screen.getByRole('textbox', { name: /description/i }), { target: { value: 'edited description' }, @@ -361,7 +486,15 @@ describe('CorrelationsPage', () => { }); describe('Read only correlations', () => { - const correlations = [{ sourceUID: 'loki', targetUID: 'loki', uid: '1', label: 'Some label' }]; + const correlations: Correlation[] = [ + { + sourceUID: 'loki', + targetUID: 'loki', + uid: '1', + label: 'Some label', + config: { field: 'line', target: {}, type: 'query' }, + }, + ]; beforeEach(async () => { await renderWithContext( @@ -393,6 +526,9 @@ describe('CorrelationsPage', () => { fireEvent.click(rowExpanderButton); + // wait for the form to be rendered and query editor to be mounted + await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i)); + // form elements should be readonly const labelInput = screen.getByRole('textbox', { name: /label/i }); expect(labelInput).toBeInTheDocument(); diff --git a/public/app/features/correlations/CorrelationsPage.tsx b/public/app/features/correlations/CorrelationsPage.tsx index d1b8824c13d..5ee331be916 100644 --- a/public/app/features/correlations/CorrelationsPage.tsx +++ b/public/app/features/correlations/CorrelationsPage.tsx @@ -148,7 +148,7 @@ export default function CorrelationsPage() { ( diff --git a/public/app/features/correlations/Forms/AddCorrelationForm.tsx b/public/app/features/correlations/Forms/AddCorrelationForm.tsx index 48c60350362..3be1bae60ed 100644 --- a/public/app/features/correlations/Forms/AddCorrelationForm.tsx +++ b/public/app/features/correlations/Forms/AddCorrelationForm.tsx @@ -1,6 +1,6 @@ import { css } from '@emotion/css'; import React, { useEffect } from 'react'; -import { Controller } from 'react-hook-form'; +import { Controller, FormProvider, useForm } from 'react-hook-form'; import { DataSourceInstanceSettings, GrafanaTheme2 } from '@grafana/data'; import { DataSourcePicker } from '@grafana/runtime'; @@ -12,7 +12,6 @@ import { useCorrelations } from '../useCorrelations'; import { CorrelationDetailsFormPart } from './CorrelationDetailsFormPart'; import { FormDTO } from './types'; -import { useCorrelationForm } from './useCorrelationForm'; const getStyles = (theme: GrafanaTheme2) => ({ panelContainer: css` @@ -57,62 +56,75 @@ export const AddCorrelationForm = ({ onClose, onCreated }: Props) => { } }, [error, loading, value, onCreated]); - const { control, handleSubmit, register, errors } = useCorrelationForm({ onSubmit: execute }); + const methods = useForm({ defaultValues: { config: { type: 'query', target: {} } } }); return ( -
-
- - !getDatasourceSrv().getInstanceSettings(uid)?.readOnly || "Source can't be a read-only data source.", - }, - }} - render={({ field: { onChange, value } }) => ( - - - - )} - /> -
Links to
- ( - - - - )} - /> -
+ + +
+ + !getDatasourceSrv().getInstanceSettings(uid)?.readOnly || + "Source can't be a read-only data source.", + }, + }} + render={({ field: { onChange, value } }) => ( + + + + )} + /> +
Links to
+ ( + + + + )} + /> +
- + - - - - + + + + + ); }; diff --git a/public/app/features/correlations/Forms/CorrelationDetailsFormPart.tsx b/public/app/features/correlations/Forms/CorrelationDetailsFormPart.tsx index dccb18a4148..58de7a44c13 100644 --- a/public/app/features/correlations/Forms/CorrelationDetailsFormPart.tsx +++ b/public/app/features/correlations/Forms/CorrelationDetailsFormPart.tsx @@ -1,13 +1,16 @@ import { css, cx } from '@emotion/css'; import React from 'react'; -import { RegisterOptions, UseFormRegisterReturn } from 'react-hook-form'; +import { useFormContext, useWatch } from 'react-hook-form'; import { GrafanaTheme2 } from '@grafana/data'; import { Field, Input, TextArea, useStyles2 } from '@grafana/ui'; -import { EditFormDTO } from './types'; +import { Correlation } from '../types'; -const getInputId = (inputName: string, correlation?: EditFormDTO) => { +import { QueryEditorField } from './QueryEditorField'; +import { FormDTO } from './types'; + +const getInputId = (inputName: string, correlation?: CorrelationBaseData) => { if (!correlation) { return inputName; } @@ -16,9 +19,6 @@ const getInputId = (inputName: string, correlation?: EditFormDTO) => { }; const getStyles = (theme: GrafanaTheme2) => ({ - marginless: css` - margin: 0; - `, label: css` max-width: ${theme.spacing(32)}; `, @@ -27,17 +27,24 @@ const getStyles = (theme: GrafanaTheme2) => ({ `, }); +type CorrelationBaseData = Pick; interface Props { - register: (path: 'label' | 'description', options?: RegisterOptions) => UseFormRegisterReturn; readOnly?: boolean; - correlation?: EditFormDTO; + correlation?: CorrelationBaseData; } -export function CorrelationDetailsFormPart({ register, readOnly = false, correlation }: Props) { +export function CorrelationDetailsFormPart({ readOnly = false, correlation }: Props) { const styles = useStyles2(getStyles); + const { + register, + formState: { errors }, + } = useFormContext(); + const targetUID: string | undefined = useWatch({ name: 'targetUID' }) || correlation?.targetUID; return ( <> + +