diff --git a/src/views/config-v2/AdditionalSettingsSection.tsx b/src/views/config-v2/AdditionalSettingsSection.tsx index 3a36b98e0..d5b153290 100644 --- a/src/views/config-v2/AdditionalSettingsSection.tsx +++ b/src/views/config-v2/AdditionalSettingsSection.tsx @@ -1,7 +1,11 @@ import { ConfigSubSection } from 'components/experimental/ConfigSection'; import allLabels from './labelsV2'; import React, { ChangeEvent, useState } from 'react'; -import { DataSourcePluginOptionsEditorProps, onUpdateDatasourceJsonDataOption } from '@grafana/data'; +import { + DataSourcePluginOptionsEditorProps, + onUpdateDatasourceJsonDataOption, + onUpdateDatasourceJsonDataOptionChecked, +} from '@grafana/data'; import { AliasTableEntry, CHConfig, CHCustomSetting, CHLogsConfig, CHSecureConfig, CHTracesConfig } from 'types/config'; import { AliasTableConfig } from 'components/configEditor/AliasTableConfig'; import { DefaultDatabaseTableConfig } from 'components/configEditor/DefaultDatabaseTableConfig'; @@ -119,7 +123,7 @@ export const AdditionalSettingsSection = (props: Props) => { label={ <> 4. {CONFIG_SECTION_HEADERS[3].label} - + } isOpen={!!CONFIG_SECTION_HEADERS[3].isOpen} @@ -166,7 +170,7 @@ export const AdditionalSettingsSection = (props: Props) => { }} onValidateSqlChange={(e) => { trackClickhouseConfigV2QuerySettings({ validateSql: e.currentTarget.checked }); - onUpdateDatasourceJsonDataOption(props, 'validateSql')(e); + onUpdateDatasourceJsonDataOptionChecked(props, 'validateSql')(e); }} /> @@ -221,7 +225,7 @@ export const AdditionalSettingsSection = (props: Props) => { data-testid={labels.enableRowLimit.testid} onChange={(e) => { trackClickhouseConfigV2EnableRowLimitToggle({ rowLimitEnabled: e.currentTarget.checked }); - onUpdateDatasourceJsonDataOption(props, 'enableRowLimit')(e); + onUpdateDatasourceJsonDataOptionChecked(props, 'enableRowLimit')(e); }} /> diff --git a/src/views/config-v2/CHConfigEditor.tsx b/src/views/config-v2/CHConfigEditor.tsx index 3a3ebfd37..19e661ca6 100644 --- a/src/views/config-v2/CHConfigEditor.tsx +++ b/src/views/config-v2/CHConfigEditor.tsx @@ -71,7 +71,7 @@ const getStyles = (theme: GrafanaTheme2) => ({ top: '100px', alignSelf: 'flex-start', maxHeight: 'calc(100vh - 100px)', - overflowY: 'auto', + overflow: 'hidden', }), requiredFields: css({ marginBottom: theme.spacing(2), diff --git a/src/views/config-v2/HttpHeadersConfigV2.test.tsx b/src/views/config-v2/HttpHeadersConfigV2.test.tsx index a891a9bf4..2027808a7 100644 --- a/src/views/config-v2/HttpHeadersConfigV2.test.tsx +++ b/src/views/config-v2/HttpHeadersConfigV2.test.tsx @@ -2,19 +2,38 @@ import React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; import { HttpHeadersConfigV2 } from './HttpHeadersConfigV2'; import { selectors } from 'selectors'; +import { Protocol } from 'types/config'; +import { createTestProps } from './helpers'; describe('HttpHeadersConfigV2', () => { const onHttpHeadersChange = jest.fn(); - const onForwardGrafanaHeadersChange = jest.fn(); + const onOptionsChangeMock = jest.fn(); let consoleSpy: jest.SpyInstance; + const defaultProps = createTestProps({ + options: { + jsonData: { + host: '', + secure: false, + protocol: Protocol.Native, + port: undefined, + pdcInjected: false, + }, + secureJsonData: {}, + secureJsonFields: {}, + }, + mocks: { + onOptionsChange: onOptionsChangeMock, + }, + }); + const renderWith = (overrides?: Partial>) => { const props: React.ComponentProps = { + ...defaultProps, headers: [], forwardGrafanaHeaders: false, secureFields: {}, onHttpHeadersChange, - onForwardGrafanaHeadersChange, ...(overrides || {}), }; return render(); @@ -89,6 +108,123 @@ describe('HttpHeadersConfigV2', () => { const forwardCb = screen.getByLabelText(/forward grafana http headers to data source/i) as HTMLInputElement; fireEvent.click(forwardCb); - expect(onForwardGrafanaHeadersChange).toHaveBeenCalledWith(true); + expect(onOptionsChangeMock).toHaveBeenCalled(); + expect(onOptionsChangeMock).toHaveBeenLastCalledWith( + expect.objectContaining({ + jsonData: expect.objectContaining({ forwardGrafanaHeaders: true }), + }) + ); + }); + + describe('HttpHeadersConfigV2', () => { + const onHttpHeadersChange = jest.fn(); + const onOptionsChangeMock = jest.fn(); + let consoleSpy: jest.SpyInstance; + + const defaultProps = createTestProps({ + options: { + jsonData: { + host: '', + secure: false, + protocol: Protocol.Native, + port: undefined, + pdcInjected: false, + }, + secureJsonData: {}, + secureJsonFields: {}, + }, + mocks: { + onOptionsChange: onOptionsChangeMock, + }, + }); + + const renderWith = (overrides?: Partial>) => { + const props: React.ComponentProps = { + ...defaultProps, + headers: [], + forwardGrafanaHeaders: false, + secureFields: {}, + onHttpHeadersChange, + ...(overrides || {}), + }; + return render(); + }; + + beforeEach(() => { + // Mock console.error to suppress React act() warnings + consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + jest.clearAllMocks(); + }); + + afterEach(() => { + consoleSpy.mockRestore(); + }); + + it('renders top label, Add header button, and forward checkbox', () => { + renderWith(); + + expect(screen.getByText(/custom http headers/i)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /add header/i })).toBeInTheDocument(); + expect(screen.getByLabelText(/forward grafana http headers to data source/i)).toBeInTheDocument(); + }); + + it('adds a new header editor when Add header is clicked', () => { + renderWith(); + + const before = screen.queryAllByTestId(selectors.components.Config.HttpHeaderConfig.headerEditor).length; + fireEvent.click(screen.getByTestId(selectors.components.Config.HttpHeaderConfig.addHeaderButton)); + const after = screen.getAllByTestId(selectors.components.Config.HttpHeaderConfig.headerEditor).length; + + expect(after).toBe(before + 1); + expect(onHttpHeadersChange).not.toHaveBeenCalled(); + }); + + it('renders any initial headers passed in', () => { + renderWith({ + headers: [ + { name: 'X-Auth', value: 'abc', secure: false }, + { name: 'Foo', value: 'bar', secure: true }, + ], + }); + + const editors = screen.getAllByTestId(selectors.components.Config.HttpHeaderConfig.headerEditor); + expect(editors.length).toBe(2); + expect(screen.getAllByTestId(selectors.components.Config.HttpHeaderConfig.headerNameInput)[0]).toHaveValue( + 'X-Auth' + ); + expect(screen.getAllByTestId(selectors.components.Config.HttpHeaderConfig.headerNameInput)[1]).toHaveValue('Foo'); + }); + + it('removes a header and calls onHttpHeadersChange when Remove is clicked', () => { + renderWith({ + headers: [ + { name: 'A', value: '1', secure: false }, + { name: 'B', value: '2', secure: false }, + ], + }); + + const before = screen.getAllByTestId(selectors.components.Config.HttpHeaderConfig.headerEditor).length; + const removeButtons = screen.getAllByTestId('trash-alt'); + fireEvent.click(removeButtons[0]); + + expect(onHttpHeadersChange).toHaveBeenCalled(); + const next = onHttpHeadersChange.mock.lastCall?.[0]; + expect(next.length).toBe(before - 1); + expect(next.find((h: any) => h.name === 'A')).toBeUndefined(); + }); + + it('toggles "Forward Grafana headers" and updated forwardGrafanaHeaders value to true', () => { + renderWith({ forwardGrafanaHeaders: false }); + + const forwardCb = screen.getByLabelText(/forward grafana http headers to data source/i) as HTMLInputElement; + fireEvent.click(forwardCb); + + expect(onOptionsChangeMock).toHaveBeenCalled(); + expect(onOptionsChangeMock).toHaveBeenLastCalledWith( + expect.objectContaining({ + jsonData: expect.objectContaining({ forwardGrafanaHeaders: true }), + }) + ); + }); }); }); diff --git a/src/views/config-v2/HttpHeadersConfigV2.tsx b/src/views/config-v2/HttpHeadersConfigV2.tsx index 455b4a0fb..5fbd392bd 100644 --- a/src/views/config-v2/HttpHeadersConfigV2.tsx +++ b/src/views/config-v2/HttpHeadersConfigV2.tsx @@ -1,17 +1,16 @@ import React, { ChangeEvent, useMemo, useState } from 'react'; import { Input, Field, SecretInput, Button, Stack, Checkbox, Box } from '@grafana/ui'; -import { CHHttpHeader } from 'types/config'; +import { CHConfig, CHHttpHeader, CHSecureConfig } from 'types/config'; import allLabels from './labelsV2'; import { styles } from 'styles'; import { selectors as allSelectors } from 'selectors'; -import { KeyValue } from '@grafana/data'; +import { DataSourcePluginOptionsEditorProps, KeyValue, onUpdateDatasourceJsonDataOptionChecked } from '@grafana/data'; -interface HttpHeadersConfigProps { +interface HttpHeadersConfigProps extends DataSourcePluginOptionsEditorProps { headers?: CHHttpHeader[]; forwardGrafanaHeaders?: boolean; secureFields: KeyValue; onHttpHeadersChange: (v: CHHttpHeader[]) => void; - onForwardGrafanaHeadersChange: (v: boolean) => void; } export const HttpHeadersConfigV2 = (props: HttpHeadersConfigProps) => { @@ -40,9 +39,9 @@ export const HttpHeadersConfigV2 = (props: HttpHeadersConfigProps) => { onHttpHeadersChange(nextHeaders); }; - const updateForwardGrafanaHeaders = (value: boolean) => { - setForwardGrafanaHeaders(value); - props.onForwardGrafanaHeadersChange(value); + const updateForwardGrafanaHeaders = (e: React.SyntheticEvent) => { + setForwardGrafanaHeaders(e.currentTarget.checked); + onUpdateDatasourceJsonDataOptionChecked(props, 'forwardGrafanaHeaders')(e); }; return ( @@ -73,11 +72,10 @@ export const HttpHeadersConfigV2 = (props: HttpHeadersConfigProps) => { - {/* Use 'checked' instead of 'value' */} updateForwardGrafanaHeaders(e.currentTarget.checked)} + onChange={(e) => updateForwardGrafanaHeaders(e)} /> ); @@ -122,7 +120,6 @@ const HttpHeaderEditorV2 = (props: HttpHeaderEditorProps) => { /> - {/* Avoid 'grow'; use flex prop that Box supports */} {secure ? ( @@ -148,7 +145,6 @@ const HttpHeaderEditorV2 = (props: HttpHeaderEditorProps) => { {!isSecureConfigured && ( - // Use 'checked' instead of 'value' { ...defaultProps.options, jsonData: { ...defaultProps.options.jsonData, protocol: Protocol.Native }, }} - onSwitchToggle={jest.fn()} /> ); @@ -52,7 +51,7 @@ describe('HttpProtocolSettingsSection', () => { }); it('calls onOptionsChange when HTTP path is changed', () => { - render(); + render(); const pathInput = screen.getByLabelText(/path/i); fireEvent.change(pathInput, { target: { value: '/api' } }); @@ -61,7 +60,7 @@ describe('HttpProtocolSettingsSection', () => { }); it('toggles Optional HTTP settings open/closed via the button (icon changes)', () => { - render(); + render(); expect(screen.getByTestId('angle-right')).toBeInTheDocument(); diff --git a/src/views/config-v2/HttpProtocolSettingsSection.tsx b/src/views/config-v2/HttpProtocolSettingsSection.tsx index b62367113..8c1105a93 100644 --- a/src/views/config-v2/HttpProtocolSettingsSection.tsx +++ b/src/views/config-v2/HttpProtocolSettingsSection.tsx @@ -7,18 +7,11 @@ import { css } from '@emotion/css'; import { onHttpHeadersChange } from 'views/CHConfigEditorHooks'; import { HttpHeadersConfigV2 } from './HttpHeadersConfigV2'; -export interface HttpProtocolSettingsSectionProps extends DataSourcePluginOptionsEditorProps { - onSwitchToggle: ( - key: keyof Pick< - CHConfig, - 'secure' | 'validateSql' | 'enableSecureSocksProxy' | 'forwardGrafanaHeaders' | 'enableRowLimit' - >, - value: boolean - ) => void; -} +export interface HttpProtocolSettingsSectionProps + extends DataSourcePluginOptionsEditorProps {} export const HttpProtocolSettingsSection = (props: HttpProtocolSettingsSectionProps) => { - const { options, onOptionsChange, onSwitchToggle } = props; + const { options, onOptionsChange } = props; const { jsonData, secureJsonFields } = options; const labels = allLabels.components.Config.ConfigEditor; @@ -54,11 +47,12 @@ export const HttpProtocolSettingsSection = (props: HttpProtocolSettingsSectionPr {optionalHttpIsOpen && ( onHttpHeadersChange(headers, options, onOptionsChange)} - onForwardGrafanaHeadersChange={(forward) => onSwitchToggle('forwardGrafanaHeaders', forward)} /> )} diff --git a/src/views/config-v2/ServerAndEncryptionSection.tsx b/src/views/config-v2/ServerAndEncryptionSection.tsx index 30a8e9ee1..3bb9ce8d6 100644 --- a/src/views/config-v2/ServerAndEncryptionSection.tsx +++ b/src/views/config-v2/ServerAndEncryptionSection.tsx @@ -1,5 +1,10 @@ import React from 'react'; -import { DataSourcePluginOptionsEditorProps, GrafanaTheme2, onUpdateDatasourceJsonDataOption } from '@grafana/data'; +import { + DataSourcePluginOptionsEditorProps, + GrafanaTheme2, + onUpdateDatasourceJsonDataOption, + onUpdateDatasourceJsonDataOptionChecked, +} from '@grafana/data'; import { Box, CollapsableSection, @@ -61,22 +66,6 @@ export const ServerAndEncryptionSection = (props: Props) => { }); }; - const onSwitchToggle = ( - key: keyof Pick< - CHConfig, - 'secure' | 'validateSql' | 'enableSecureSocksProxy' | 'forwardGrafanaHeaders' | 'enableRowLimit' - >, - value: boolean - ) => { - onOptionsChange({ - ...options, - jsonData: { - ...options.jsonData, - [key]: value, - }, - }); - }; - return ( { checked={jsonData.secure || false} onChange={(e) => { trackClickhouseConfigV2SecureConnectionChecked({ secureConnection: e.currentTarget.checked }); - onSwitchToggle('secure', e.currentTarget.checked); + onUpdateDatasourceJsonDataOptionChecked(props, 'secure')(e); }} /> - + ); diff --git a/src/views/config-v2/TLSSSLSettingsSection.tsx b/src/views/config-v2/TLSSSLSettingsSection.tsx index 493763e9f..6b02610a7 100644 --- a/src/views/config-v2/TLSSSLSettingsSection.tsx +++ b/src/views/config-v2/TLSSSLSettingsSection.tsx @@ -1,4 +1,4 @@ -import { DataSourcePluginOptionsEditorProps, onUpdateDatasourceJsonDataOption } from '@grafana/data'; +import { DataSourcePluginOptionsEditorProps, onUpdateDatasourceJsonDataOptionChecked } from '@grafana/data'; import { Box, CollapsableSection, CertificationKey, Text, useStyles2, Checkbox, Stack, Badge } from '@grafana/ui'; import React from 'react'; import { CHConfig, CHSecureConfig } from 'types/config'; @@ -59,7 +59,7 @@ export const TLSSSLSettingsSection = (props: Props) => { label={ <> 3. {CONFIG_SECTION_HEADERS[2].label} - + } isOpen={!!(jsonData.tlsSkipVerify || jsonData.tlsAuth || jsonData.tlsAuthWithCACert)} @@ -79,7 +79,7 @@ export const TLSSSLSettingsSection = (props: Props) => { value={jsonData.tlsSkipVerify || false} onChange={(e) => { trackClickhouseConfigV2SkipTLSVerifyToggleClicked({ skipTlsVerifyToggle: e.currentTarget.checked }); - onUpdateDatasourceJsonDataOption(props, 'tlsSkipVerify')(e); + onUpdateDatasourceJsonDataOptionChecked(props, 'tlsSkipVerify')(e); }} /> { value={jsonData.tlsAuth || false} onChange={(e) => { trackClickhouseConfigV2TLSClientAuthToggleClicked({ clientAuthToggle: e.currentTarget.checked }); - onUpdateDatasourceJsonDataOption(props, 'tlsAuth')(e); + onUpdateDatasourceJsonDataOptionChecked(props, 'tlsAuth')(e); }} /> {jsonData.tlsAuth && ( @@ -116,7 +116,7 @@ export const TLSSSLSettingsSection = (props: Props) => { value={jsonData.tlsAuthWithCACert || false} onChange={(e) => { trackClickhouseConfigV2WithCACertToggleClicked({ caCertToggle: e.currentTarget.checked }); - onUpdateDatasourceJsonDataOption(props, 'tlsAuthWithCACert')(e); + onUpdateDatasourceJsonDataOptionChecked(props, 'tlsAuthWithCACert')(e); }} />