Update shouldbeopen logic for additional settings

alyssajoyner · alyssajoyner · commit eee86d991c06 · 2025-11-10T14:51:25.000-07:00
diff --git a/package.json b/package.json
@@ -88,6 +88,5 @@
     "semver": "7.7.3",
     "sql-formatter": "^15.6.10",
     "tslib": "2.8.1"
-  },
-  "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
+  }
 }
diff --git a/src/components/configEditor/LogsConfig.test.tsx b/src/components/configEditor/LogsConfig.test.tsx
@@ -3,7 +3,7 @@ import { render, fireEvent } from '@testing-library/react';
 import { LogsConfig } from './LogsConfig';
 import allLabels from 'labels';
 import { columnLabelToPlaceholder } from 'data/utils';
-import { defaultLogsTable } from 'otel';
+import { defaultCHAdditionalSettingsConfig } from 'types/config';
 
 describe('LogsConfig', () => {
   it('should render', () => {
@@ -68,7 +68,7 @@ describe('LogsConfig', () => {
     );
     expect(result.container.firstChild).not.toBeNull();
 
-    const input = result.getByPlaceholderText(defaultLogsTable);
+    const input = result.getByPlaceholderText(defaultCHAdditionalSettingsConfig.logs?.defaultTable!);
     expect(input).toBeInTheDocument();
     fireEvent.change(input, { target: { value: 'changed' } });
     fireEvent.blur(input);
diff --git a/src/components/configEditor/LogsConfig.tsx b/src/components/configEditor/LogsConfig.tsx
@@ -3,9 +3,9 @@ import { ConfigSection, ConfigSubSection } from 'components/experimental/ConfigS
 import { Input, Field, InlineFormLabel, TagsInput } from '@grafana/ui';
 import { OtelVersionSelect } from 'components/queryBuilder/OtelVersionSelect';
 import { ColumnHint } from 'types/queryBuilder';
-import otel, { defaultLogsTable } from 'otel';
+import otel from 'otel';
 import { LabeledInput } from './LabeledInput';
-import { CHLogsConfig } from 'types/config';
+import { CHLogsConfig, defaultCHAdditionalSettingsConfig } from 'types/config';
 import allLabels from 'labels';
 import { columnLabelToPlaceholder } from 'data/utils';
 import { Switch } from 'components/queryBuilder/Switch';
@@ -80,7 +80,7 @@ export const LogsConfig = (props: LogsConfigProps) => {
           onChange={(e) => onDefaultTableChange(e.currentTarget.value)}
           label={labels.defaultTable.label}
           aria-label={labels.defaultTable.label}
-          placeholder={defaultLogsTable}
+          placeholder={defaultCHAdditionalSettingsConfig.logs?.defaultTable!}
         />
       </Field>
       <ConfigSubSection title={labels.columns.title} description={labels.columns.description}>
diff --git a/src/components/configEditor/TracesConfig.test.tsx b/src/components/configEditor/TracesConfig.test.tsx
@@ -3,7 +3,7 @@ import { render, fireEvent } from '@testing-library/react';
 import { TracesConfig, TraceConfigProps } from './TracesConfig';
 import allLabels from 'labels';
 import { columnLabelToPlaceholder } from 'data/utils';
-import { defaultTraceTable } from 'otel';
+import { defaultCHAdditionalSettingsConfig } from 'types/config';
 
 function defaultTraceConfigProps(): TraceConfigProps {
   return {
@@ -60,7 +60,7 @@ describe('TracesConfig', () => {
     const result = render(<TracesConfig {...defaultTraceConfigProps()} onDefaultTableChange={onDefaultTableChange} />);
     expect(result.container.firstChild).not.toBeNull();
 
-    const input = result.getByPlaceholderText(defaultTraceTable);
+    const input = result.getByPlaceholderText(defaultCHAdditionalSettingsConfig.traces?.defaultTable!);
     expect(input).toBeInTheDocument();
     fireEvent.change(input, { target: { value: 'changed' } });
     fireEvent.blur(input);
diff --git a/src/components/configEditor/TracesConfig.tsx b/src/components/configEditor/TracesConfig.tsx
@@ -3,10 +3,10 @@ import { ConfigSection, ConfigSubSection } from 'components/experimental/ConfigS
 import { Input, Field } from '@grafana/ui';
 import { OtelVersionSelect } from 'components/queryBuilder/OtelVersionSelect';
 import { ColumnHint, TimeUnit } from 'types/queryBuilder';
-import otel, { defaultTraceTable } from 'otel';
+import otel from 'otel';
 import { LabeledInput } from './LabeledInput';
 import { DurationUnitSelect } from 'components/queryBuilder/DurationUnitSelect';
-import { CHTracesConfig } from 'types/config';
+import { CHTracesConfig, defaultCHAdditionalSettingsConfig } from 'types/config';
 import allLabels from 'labels';
 import { columnLabelToPlaceholder } from 'data/utils';
 import { Switch } from 'components/queryBuilder/Switch';
@@ -136,7 +136,7 @@ export const TracesConfig = (props: TraceConfigProps) => {
           onChange={(e) => onDefaultTableChange(e.currentTarget.value)}
           label={labels.defaultTable.label}
           aria-label={labels.defaultTable.label}
-          placeholder={defaultTraceTable}
+          placeholder={defaultCHAdditionalSettingsConfig.traces?.defaultTable!}
         />
       </Field>
       <ConfigSubSection title={labels.columns.title} description={labels.columns.description}>
@@ -197,7 +197,7 @@ export const TracesConfig = (props: TraceConfigProps) => {
         />
         <DurationUnitSelect
           disabled={otelEnabled}
-          unit={(durationUnit as TimeUnit) || TimeUnit.Nanoseconds}
+          unit={(durationUnit as TimeUnit) || defaultCHAdditionalSettingsConfig.traces?.durationUnit}
           onChange={onDurationUnitChange}
         />
         <LabeledInput
diff --git a/src/types/config.ts b/src/types/config.ts
@@ -1,4 +1,6 @@
 import { DataSourceJsonData, KeyValue } from '@grafana/data';
+import otel, { defaultLogsTable, defaultTraceTable } from 'otel';
+import { TimeUnit } from './queryBuilder';
 
 export interface CHConfig extends DataSourceJsonData {
   /**
@@ -118,3 +120,17 @@ export enum Protocol {
   Native = 'native',
   Http = 'http',
 }
+
+export const defaultCHAdditionalSettingsConfig: Partial<CHConfig> = {
+  logs: {
+    defaultTable: defaultLogsTable,
+    otelVersion: otel.getLatestVersion().version,
+    selectContextColumns: true,
+    contextColumns: [],
+  },
+  traces: {
+    defaultTable: defaultTraceTable,
+    otelVersion: otel.getLatestVersion().version,
+    durationUnit: TimeUnit.Nanoseconds,
+  },
+};
diff --git a/src/views/CHConfigEditorHooks.test.ts b/src/views/CHConfigEditorHooks.test.ts
@@ -1,9 +1,8 @@
 import { DataSourceSettings } from '@grafana/data';
 import { renderHook } from '@testing-library/react';
-import { CHConfig, CHHttpHeader, CHSecureConfig, Protocol } from 'types/config';
+import { CHConfig, CHHttpHeader, CHSecureConfig, defaultCHAdditionalSettingsConfig, Protocol } from 'types/config';
 import { onHttpHeadersChange, useConfigDefaults } from './CHConfigEditorHooks';
 import { pluginVersion } from 'utils/version';
-import { defaultLogsTable, defaultTraceTable } from 'otel';
 
 describe('onHttpHeadersChange', () => {
   it('should properly sort headers into secure/plain config fields', async () => {
@@ -95,12 +94,12 @@ describe('useConfigDefaults', () => {
     version: pluginVersion,
     protocol: Protocol.Native,
     logs: {
-      defaultTable: defaultLogsTable,
+      defaultTable: defaultCHAdditionalSettingsConfig.logs?.defaultTable,
       selectContextColumns: true,
       contextColumns: [],
     },
     traces: {
-      defaultTable: defaultTraceTable,
+      defaultTable: defaultCHAdditionalSettingsConfig.traces?.defaultTable,
     },
   };
 
diff --git a/src/views/CHConfigEditorHooks.ts b/src/views/CHConfigEditorHooks.ts
@@ -1,7 +1,6 @@
 import { DataSourceSettings, KeyValue } from '@grafana/data';
-import { defaultLogsTable, defaultTraceTable } from 'otel';
 import { useEffect, useRef } from 'react';
-import { CHConfig, CHHttpHeader, CHSecureConfig, Protocol } from 'types/config';
+import { CHConfig, CHHttpHeader, CHSecureConfig, defaultCHAdditionalSettingsConfig, Protocol } from 'types/config';
 import { pluginVersion } from 'utils/version';
 
 /**
@@ -103,7 +102,7 @@ export const useConfigDefaults = (
     if (!jsonData.logs || jsonData.logs.defaultTable === undefined) {
       jsonData.logs = {
         ...jsonData.logs,
-        defaultTable: defaultLogsTable,
+        defaultTable: defaultCHAdditionalSettingsConfig.logs?.defaultTable,
         selectContextColumns: true,
         contextColumns: [],
       };
@@ -112,7 +111,7 @@ export const useConfigDefaults = (
     if (!jsonData.traces || jsonData.traces.defaultTable === undefined) {
       jsonData.traces = {
         ...jsonData.traces,
-        defaultTable: defaultTraceTable,
+        defaultTable: defaultCHAdditionalSettingsConfig.traces?.defaultTable,
       };
     }
 
diff --git a/src/views/config-v2/AdditionalSettingsSection.tsx b/src/views/config-v2/AdditionalSettingsSection.tsx
@@ -1,12 +1,20 @@
 import { ConfigSubSection } from 'components/experimental/ConfigSection';
 import allLabels from './labelsV2';
-import React, { ChangeEvent, useState } from 'react';
+import React, { ChangeEvent, useMemo, useState } from 'react';
 import {
   DataSourcePluginOptionsEditorProps,
   onUpdateDatasourceJsonDataOption,
   onUpdateDatasourceJsonDataOptionChecked,
 } from '@grafana/data';
-import { AliasTableEntry, CHConfig, CHCustomSetting, CHLogsConfig, CHSecureConfig, CHTracesConfig } from 'types/config';
+import {
+  AliasTableEntry,
+  CHConfig,
+  CHCustomSetting,
+  CHLogsConfig,
+  CHSecureConfig,
+  CHTracesConfig,
+  defaultCHAdditionalSettingsConfig,
+} from 'types/config';
 import { AliasTableConfig } from 'components/configEditor/AliasTableConfig';
 import { DefaultDatabaseTableConfig } from 'components/configEditor/DefaultDatabaseTableConfig';
 import { LogsConfig } from 'components/configEditor/LogsConfig';
@@ -52,69 +60,6 @@ export const AdditionalSettingsSection = (props: Props) => {
   useConfigDefaults(options, onOptionsChange);
 
   const [customSettings, setCustomSettings] = useState(jsonData.customSettings || []);
-  const ADDITIONAL_SETTINGS_DEFAULTS = {
-    defaultDatabase: jsonData.defaultDatabase,
-    defaultTable: jsonData.defaultTable,
-    connMaxLifetime: jsonData.connMaxLifetime,
-    dialTimeout: jsonData.dialTimeout,
-    maxIdleConns: jsonData.maxIdleConns,
-    maxOpenConns: jsonData.maxOpenConns,
-    queryTimeout: jsonData.queryTimeout,
-    validateSql: jsonData.validateSql,
-    logs: {
-      defaultDatabase: jsonData.logs?.defaultDatabase,
-      defaultTable: jsonData.logs?.defaultTable !== 'otel_logs' ? jsonData.logs?.defaultTable : undefined,
-      otelEnabled: jsonData.logs?.otelEnabled,
-      otelVersion: jsonData.logs?.otelVersion !== 'latest' ? jsonData.logs?.otelVersion : undefined,
-      timeColumn: jsonData.logs?.timeColumn,
-      levelColumn: jsonData.logs?.levelColumn,
-      messageColumn: jsonData.logs?.messageColumn,
-      selectContextColumns: jsonData.logs?.selectContextColumns,
-      contextColumns:
-        jsonData.logs?.contextColumns && jsonData.logs?.contextColumns.length > 0
-          ? jsonData.logs?.contextColumns
-          : undefined,
-    },
-    traces: {
-      defaultDatabase: jsonData.traces?.defaultDatabase,
-      defaultTable: jsonData.traces?.defaultTable !== 'otel_traces' ? jsonData.logs?.defaultTable : undefined,
-      otelEnabled: jsonData.traces?.otelEnabled,
-      otelVersion: jsonData.traces?.otelVersion !== 'latest' ? jsonData.logs?.otelVersion : undefined,
-      traceIdColumn: jsonData.traces?.traceIdColumn,
-      spanIdColumn: jsonData.traces?.spanIdColumn,
-      operationNameColumn: jsonData.traces?.operationNameColumn,
-      parentSpanIdColumn: jsonData.traces?.parentSpanIdColumn,
-      serviceNameColumn: jsonData.traces?.serviceNameColumn,
-      durationColumn: jsonData.traces?.durationColumn,
-      startTimeColumn: jsonData.traces?.startTimeColumn,
-      tagsColumn: jsonData.traces?.tagsColumn,
-      serviceTagsColumn: jsonData.traces?.serviceTagsColumn,
-      kindColumn: jsonData.traces?.kindColumn,
-      statusCodeColumn: jsonData.traces?.statusCodeColumn,
-      statusMessageColumn: jsonData.traces?.statusMessageColumn,
-      stateColumn: jsonData.traces?.stateColumn,
-      instrumentationLibraryNameColumn: jsonData.traces?.instrumentationLibraryNameColumn,
-      instrumentationLibraryVersionColumn: jsonData.traces?.instrumentationLibraryVersionColumn,
-      flattenNested: jsonData.traces?.flattenNested,
-      traceEventsColumnPrefix: jsonData.traces?.traceEventsColumnPrefix,
-      traceLinksColumnPrefix: jsonData.traces?.traceLinksColumnPrefix,
-    },
-    aliasTables: jsonData.aliasTables && jsonData.aliasTables.length > 0 ? jsonData.aliasTables : undefined,
-    enableRowLimit: jsonData.enableRowLimit,
-    enableSecureSocksProxy: jsonData.enableSecureSocksProxy,
-  };
-
-  const hasDefinedValue = (obj: any): boolean => {
-    if (obj == null) {
-      return false;
-    }
-    if (typeof obj !== 'object') {
-      return obj !== '' && obj !== undefined && obj !== null;
-    }
-    return Object.values(obj).some((v) => hasDefinedValue(v));
-  };
-
-  const shouldBeOpen = hasDefinedValue(ADDITIONAL_SETTINGS_DEFAULTS);
 
   const onLogsConfigChange = (key: keyof CHLogsConfig, value: string | boolean | string[]) => {
     onOptionsChange({
@@ -173,6 +118,50 @@ export const AdditionalSettingsSection = (props: Props) => {
     });
   };
 
+  const shallowSettingsCompare = (currentSettings: any, defaultSettings: any): boolean => {
+    // needed for dealing with proxy object from currentSettings
+    currentSettings = Object.assign({}, currentSettings);
+
+    const currentSettingsKeys = Object.keys(currentSettings);
+    const defaultSettingsKeys = Object.keys(defaultSettings);
+
+    if (currentSettingsKeys.length !== defaultSettingsKeys.length) {
+      return false;
+    }
+
+    for (const key of currentSettingsKeys) {
+      if (!defaultSettingsKeys.includes(key)) {
+        return false;
+      }
+      if (currentSettings[key].length === 0 && defaultSettings[key].length === 0) {
+        continue;
+      }
+      if (currentSettings[key] !== defaultSettings[key]) {
+        return false;
+      }
+    }
+    return true;
+  };
+
+  const shouldBeOpen = useMemo(() => {
+    return (
+      jsonData.defaultDatabase ||
+      jsonData.defaultTable ||
+      jsonData.connMaxLifetime ||
+      jsonData.dialTimeout ||
+      jsonData.maxIdleConns ||
+      jsonData.maxOpenConns ||
+      jsonData.queryTimeout ||
+      jsonData.validateSql ||
+      !shallowSettingsCompare(jsonData.logs, defaultCHAdditionalSettingsConfig.logs) ||
+      !shallowSettingsCompare(jsonData.traces, defaultCHAdditionalSettingsConfig.traces) ||
+      (jsonData.aliasTables && jsonData.aliasTables.length > 0) ||
+      jsonData.enableRowLimit ||
+      jsonData.enableSecureSocksProxy ||
+      customSettings.length > 0
+    );
+  }, [jsonData, customSettings]);
+
   return (
     <Box
       borderStyle="solid"
@@ -296,7 +285,7 @@ export const AdditionalSettingsSection = (props: Props) => {
           <Field label={labels.secureSocksProxy.label} description={labels.secureSocksProxy.tooltip}>
             <Switch
               value={jsonData.enableSecureSocksProxy || false}
-              onChange={(e) => onUpdateDatasourceJsonDataOption(props, 'enableSecureSocksProxy')(e)}
+              onChange={(e) => onUpdateDatasourceJsonDataOptionChecked(props, 'enableSecureSocksProxy')(e)}
             />
           </Field>
         )}

Original file line number	Diff line number	Diff line change
`@@ -88,6 +88,5 @@`
`88`	`88`	`"semver": "7.7.3",`
`89`	`89`	`"sql-formatter": "^15.6.10",`
`90`	`90`	`"tslib": "2.8.1"`
`91`		`- },`
`92`		`- "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"`
	`91`	`+ }`
`93`	`92`	`}`