From 0582f0eead80d3b6ec95a317e595c8f4993a6d24 Mon Sep 17 00:00:00 2001 From: Nathan Marrs Date: Fri, 4 Oct 2024 08:09:54 -0600 Subject: [PATCH] Query Editor: Fix field options not loading in (#437) --- src/QueryEditorForm.test.tsx | 30 ++++++---- src/QueryEditorForm.tsx | 103 ++++++++++++++++++++++------------- src/types.ts | 8 +++ 3 files changed, 92 insertions(+), 49 deletions(-) diff --git a/src/QueryEditorForm.test.tsx b/src/QueryEditorForm.test.tsx index 153304c2..99899f7a 100644 --- a/src/QueryEditorForm.test.tsx +++ b/src/QueryEditorForm.test.tsx @@ -6,7 +6,7 @@ import { mockDatasource, mockQuery } from './__mocks__/datasource'; import '@testing-library/jest-dom'; import { select } from 'react-select-event'; import { selectors } from 'tests/selectors'; -import { defaultKey, defaultQuery } from 'types'; +import { defaultKey, defaultQuery, QueryEditorFieldType } from 'types'; import * as experimental from '@grafana/experimental'; const ds = mockDatasource; @@ -39,7 +39,7 @@ describe('QueryEditor', () => { it('should request regions and use a new one', async () => { const onChange = jest.fn(); ds.getResource = jest.fn().mockResolvedValue([ds.defaultRegion, 'foo']); - ds.getRegions = jest.fn(() => ds.getResource('regions')); + ds.getRegions = jest.fn(() => ds.getResource(QueryEditorFieldType.Regions)); render(); const selectEl = screen.getByLabelText(selectors.components.ConfigEditor.region.input); @@ -48,7 +48,7 @@ describe('QueryEditor', () => { await select(selectEl, 'foo', { container: document.body }); - expect(ds.getResource).toHaveBeenCalledWith('regions'); + expect(ds.getResource).toHaveBeenCalledWith(QueryEditorFieldType.Regions); expect(onChange).toHaveBeenCalledWith({ ...q, connectionArgs: { ...q.connectionArgs, region: 'foo' }, @@ -58,7 +58,9 @@ describe('QueryEditor', () => { it('should request catalogs and use a new one', async () => { const onChange = jest.fn(); ds.postResource = jest.fn().mockResolvedValue([ds.defaultCatalog, 'foo']); - ds.getCatalogs = jest.fn((query) => ds.postResource('catalogs', { region: query.connectionArgs.region })); + ds.getCatalogs = jest.fn((query) => + ds.postResource(QueryEditorFieldType.Catalogs, { region: query.connectionArgs.region }) + ); render(); const selectEl = screen.getByLabelText(selectors.components.ConfigEditor.catalog.input); @@ -67,7 +69,7 @@ describe('QueryEditor', () => { await select(selectEl, 'foo', { container: document.body }); - expect(ds.postResource).toHaveBeenCalledWith('catalogs', { region: defaultKey }); + expect(ds.postResource).toHaveBeenCalledWith(QueryEditorFieldType.Catalogs, { region: defaultKey }); expect(onChange).toHaveBeenCalledWith({ ...q, connectionArgs: { ...q.connectionArgs, catalog: 'foo' }, @@ -79,7 +81,10 @@ describe('QueryEditor', () => { const onRunQuery = jest.fn(); ds.postResource = jest.fn().mockResolvedValue([ds.defaultDatabase, 'foo']); ds.getDatabases = jest.fn((query) => - ds.postResource('databases', { region: query.connectionArgs.region, catalog: query.connectionArgs.catalog }) + ds.postResource(QueryEditorFieldType.Databases, { + region: query.connectionArgs.region, + catalog: query.connectionArgs.catalog, + }) ); render(); @@ -89,7 +94,10 @@ describe('QueryEditor', () => { await select(selectEl, 'foo', { container: document.body }); - expect(ds.postResource).toHaveBeenCalledWith('databases', { region: defaultKey, catalog: defaultKey }); + expect(ds.postResource).toHaveBeenCalledWith(QueryEditorFieldType.Databases, { + region: defaultKey, + catalog: defaultKey, + }); expect(onChange).toHaveBeenCalledWith({ ...q, connectionArgs: { ...q.connectionArgs, database: 'foo' }, @@ -102,7 +110,7 @@ describe('QueryEditor', () => { const onRunQuery = jest.fn(); ds.postResource = jest.fn().mockResolvedValue(['foo']); ds.getTables = jest.fn((query) => - ds.postResource('tables', { + ds.postResource(QueryEditorFieldType.Tables, { region: query.connectionArgs.region, catalog: query.connectionArgs.catalog, database: query.connectionArgs.database, @@ -116,7 +124,7 @@ describe('QueryEditor', () => { await select(selectEl, 'foo', { container: document.body }); expect(ds.postResource).toHaveBeenCalledWith( - 'tables', + QueryEditorFieldType.Tables, expect.objectContaining({ region: defaultKey, catalog: defaultKey, database: defaultKey }) ); expect(onChange).toHaveBeenCalledWith({ @@ -131,7 +139,7 @@ describe('QueryEditor', () => { const onRunQuery = jest.fn(); ds.postResource = jest.fn().mockResolvedValue(['columnName']); ds.getColumns = jest.fn((query) => - ds.postResource('columns', { + ds.postResource(QueryEditorFieldType.Columns, { region: query.connectionArgs.region, catalog: query.connectionArgs.catalog, database: query.connectionArgs.database, @@ -153,7 +161,7 @@ describe('QueryEditor', () => { await select(selectEl, 'columnName', { container: document.body }); expect(ds.postResource).toHaveBeenCalledWith( - 'columns', + QueryEditorFieldType.Columns, expect.objectContaining({ region: defaultKey, catalog: defaultKey, database: defaultKey, table: 'tableName' }) ); expect(onChange).toHaveBeenCalledWith({ diff --git a/src/QueryEditorForm.tsx b/src/QueryEditorForm.tsx index aab8251b..048bcd44 100644 --- a/src/QueryEditorForm.tsx +++ b/src/QueryEditorForm.tsx @@ -2,7 +2,13 @@ import React, { useEffect, useState } from 'react'; import { css } from '@emotion/css'; import { GrafanaTheme2, QueryEditorProps, SelectableValue } from '@grafana/data'; import { DataSource } from './datasource'; -import { AthenaDataSourceOptions, AthenaQuery, defaultQuery, SelectableFormatOptions } from './types'; +import { + AthenaDataSourceOptions, + AthenaQuery, + defaultQuery, + QueryEditorFieldType, + SelectableFormatOptions, +} from './types'; import { CollapsableSection, useStyles2 } from '@grafana/ui'; import { FormatSelect, ResourceSelector } from '@grafana/aws-sdk'; import { selectors } from 'tests/selectors'; @@ -15,8 +21,6 @@ type Props = QueryEditorProps hideOptions?: boolean; }; -type QueryProperties = 'regions' | 'catalogs' | 'databases' | 'tables' | 'columns'; - export function QueryEditorForm(props: Props) { const [resultReuseSupported, setResultReuseSupported] = useState(false); const styles = useStyles2(getStyles); @@ -36,6 +40,17 @@ export function QueryEditorForm(props: Props) { }, }; + /* + State used force the component to re-render when fetching + editor field options so they show up in the select component + */ + const [needsUpdate, setNeedsUpdate] = useState(false); + useEffect(() => { + if (needsUpdate) { + setNeedsUpdate(false); + } + }, [needsUpdate]); + // Populate the props.query with defaults on mount useEffect(() => { props.onChange(queryWithDefaults); @@ -45,40 +60,49 @@ export function QueryEditorForm(props: Props) { const templateVariables = props.datasource.getVariables(); - const fetchRegions = () => - props.datasource.getRegions().then((regions) => appendTemplateVariables(templateVariables, regions)); - const fetchCatalogs = () => - props.datasource + const fetchRegions = async () => + await props.datasource + .getRegions() + .then((regions) => appendTemplateVariables(templateVariables, regions)) + .finally(() => setNeedsUpdate(true)); + const fetchCatalogs = async () => + await props.datasource .getCatalogs(queryWithDefaults) - .then((catalogs) => appendTemplateVariables(templateVariables, catalogs)); - const fetchDatabases = () => - props.datasource + .then((catalogs) => appendTemplateVariables(templateVariables, catalogs)) + .finally(() => setNeedsUpdate(true)); + const fetchDatabases = async () => + await props.datasource .getDatabases(queryWithDefaults) - .then((databases) => appendTemplateVariables(templateVariables, databases)); - const fetchTables = () => - props.datasource.getTables(queryWithDefaults).then((tables) => appendTemplateVariables(templateVariables, tables)); - const fetchColumns = () => - props.datasource + .then((databases) => appendTemplateVariables(templateVariables, databases)) + .finally(() => setNeedsUpdate(true)); + const fetchTables = async () => + await props.datasource + .getTables(queryWithDefaults) + .then((tables) => appendTemplateVariables(templateVariables, tables)) + .finally(() => setNeedsUpdate(true)); + const fetchColumns = async () => + await props.datasource .getColumns(queryWithDefaults) - .then((columns) => appendTemplateVariables(templateVariables, columns)); + .then((columns) => appendTemplateVariables(templateVariables, columns)) + .finally(() => setNeedsUpdate(true)); - const onChange = (prop: QueryProperties) => (e: SelectableValue | null) => { + const onChange = (prop: QueryEditorFieldType) => (e: SelectableValue | null) => { const newQuery = { ...props.query }; const value = e?.value; switch (prop) { - case 'regions': + case QueryEditorFieldType.Regions: newQuery.connectionArgs = { ...newQuery.connectionArgs, region: value }; break; - case 'catalogs': + case QueryEditorFieldType.Catalogs: newQuery.connectionArgs = { ...newQuery.connectionArgs, catalog: value }; break; - case 'databases': + case QueryEditorFieldType.Databases: newQuery.connectionArgs = { ...newQuery.connectionArgs, database: value }; break; - case 'tables': + case QueryEditorFieldType.Tables: newQuery.table = value; break; - case 'columns': + case QueryEditorFieldType.Columns: newQuery.column = value; break; } @@ -93,11 +117,11 @@ export function QueryEditorForm(props: Props) { width={15} label={selectors.components.ConfigEditor.region.input} data-testid={selectors.components.ConfigEditor.region.wrapper} - htmlFor="regions" + htmlFor={QueryEditorFieldType.Regions} > -
+
@@ -236,4 +260,7 @@ const getStyles = (theme: GrafanaTheme2) => ({ padding: 'unset', }, }), + sqlEditor: css({ + width: '100%', + }), }); diff --git a/src/types.ts b/src/types.ts index 98e73792..c6d76a38 100644 --- a/src/types.ts +++ b/src/types.ts @@ -7,6 +7,14 @@ export enum FormatOptions { Logs, } +export enum QueryEditorFieldType { + Regions = 'regions', + Catalogs = 'catalogs', + Databases = 'databases', + Tables = 'tables', + Columns = 'columns', +} + export const SelectableFormatOptions: Array> = [ { label: 'Time Series',