Fix overflow and toggle bugs

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={
           <>
             <Text variant="h3">4. {CONFIG_SECTION_HEADERS[3].label}</Text>
-            <Badge text="optional" color="blue" className={styles.badge} />
+            <Badge text="optional" color="darkgrey" className={styles.badge} />
           </>
         }
         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);
           }}
         />
         <Divider />
@@ -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);
             }}
           />
         </Field>

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),

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<React.ComponentProps<typeof HttpHeadersConfigV2>>) => {
     const props: React.ComponentProps<typeof HttpHeadersConfigV2> = {
+      ...defaultProps,
       headers: [],
       forwardGrafanaHeaders: false,
       secureFields: {},
       onHttpHeadersChange,
-      onForwardGrafanaHeadersChange,
       ...(overrides || {}),
     };
     return render(<HttpHeadersConfigV2 {...props} />);
@@ -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<React.ComponentProps<typeof HttpHeadersConfigV2>>) => {
+      const props: React.ComponentProps<typeof HttpHeadersConfigV2> = {
+        ...defaultProps,
+        headers: [],
+        forwardGrafanaHeaders: false,
+        secureFields: {},
+        onHttpHeadersChange,
+        ...(overrides || {}),
+      };
+      return render(<HttpHeadersConfigV2 {...props} />);
+    };
+
+    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 }),
+        })
+      );
+    });
   });
 });

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<CHConfig, CHSecureConfig> {
   headers?: CHHttpHeader[];
   forwardGrafanaHeaders?: boolean;
   secureFields: KeyValue<boolean>;
   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<HTMLInputElement, Event>) => {
+    setForwardGrafanaHeaders(e.currentTarget.checked);
+    onUpdateDatasourceJsonDataOptionChecked(props, 'forwardGrafanaHeaders')(e);
   };
 
   return (
@@ -73,11 +72,10 @@ export const HttpHeadersConfigV2 = (props: HttpHeadersConfigProps) => {
         </>
       </Field>
 
-      {/* Use 'checked' instead of 'value' */}
       <Checkbox
         label={labels.forwardGrafanaHeaders.label}
         checked={forwardGrafanaHeaders}
-        onChange={(e) => updateForwardGrafanaHeaders(e.currentTarget.checked)}
+        onChange={(e) => updateForwardGrafanaHeaders(e)}
       />
     </div>
   );
@@ -122,7 +120,6 @@ const HttpHeaderEditorV2 = (props: HttpHeaderEditorProps) => {
           />
         </Field>
 
-        {/* Avoid 'grow'; use flex prop that Box supports */}
         <Box flex="1 1 auto">
           <Field label={headerValueLabel} aria-label={headerValueLabel}>
             {secure ? (
@@ -148,7 +145,6 @@ const HttpHeaderEditorV2 = (props: HttpHeaderEditorProps) => {
         </Box>
 
         {!isSecureConfigured && (
-          // Use 'checked' instead of 'value'
           <Checkbox
             label={labels.secureLabel}
             checked={secure}

src/views/config-v2/HttpProtocolSettingsSection.test.tsx

@@ -43,7 +43,6 @@ describe('HttpProtocolSettingsSection', () => {
           ...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(<HttpProtocolSettingsSection {...defaultProps} onSwitchToggle={jest.fn()} />);
+    render(<HttpProtocolSettingsSection {...defaultProps} />);
 
     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(<HttpProtocolSettingsSection {...defaultProps} onSwitchToggle={jest.fn()} />);
+    render(<HttpProtocolSettingsSection {...defaultProps} />);
 
     expect(screen.getByTestId('angle-right')).toBeInTheDocument();
 

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<CHConfig, CHSecureConfig> {
-  onSwitchToggle: (
-    key: keyof Pick<
-      CHConfig,
-      'secure' | 'validateSql' | 'enableSecureSocksProxy' | 'forwardGrafanaHeaders' | 'enableRowLimit'
-    >,
-    value: boolean
-  ) => void;
-}
+export interface HttpProtocolSettingsSectionProps
+  extends DataSourcePluginOptionsEditorProps<CHConfig, CHSecureConfig> {}
 
 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
       </Button>
       {optionalHttpIsOpen && (
         <HttpHeadersConfigV2
+          options={options}
+          onOptionsChange={onOptionsChange}
           headers={jsonData.httpHeaders}
           forwardGrafanaHeaders={jsonData.forwardGrafanaHeaders}
           secureFields={secureJsonFields}
           onHttpHeadersChange={(headers) => onHttpHeadersChange(headers, options, onOptionsChange)}
-          onForwardGrafanaHeadersChange={(forward) => onSwitchToggle('forwardGrafanaHeaders', forward)}
         />
       )}
     </div>

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 (
     <Box
       borderStyle="solid"
@@ -184,11 +173,11 @@ export const ServerAndEncryptionSection = (props: Props) => {
             checked={jsonData.secure || false}
             onChange={(e) => {
               trackClickhouseConfigV2SecureConnectionChecked({ secureConnection: e.currentTarget.checked });
-              onSwitchToggle('secure', e.currentTarget.checked);
+              onUpdateDatasourceJsonDataOptionChecked(props, 'secure')(e);
             }}
           />
         </div>
-        <HttpProtocolSettingsSection onSwitchToggle={onSwitchToggle} {...props} />
+        <HttpProtocolSettingsSection {...props} />
       </CollapsableSection>
     </Box>
   );