From 3a4f3280beb71c8467ec1af40b2ff5f723cd42c9 Mon Sep 17 00:00:00 2001 From: Jocelyn Collado-Kuri Date: Wed, 14 Jan 2026 11:50:17 -0800 Subject: [PATCH] Fix proxies dropdown in Problems query editor (#2200) Part 2 of #2197 Zabbix has two ways of processing and returning proxies depending on the zabbix version being used: 1. Before 7.0.0 it uses `host` 2. After 7.0.0 it uses `name` when we made the call to the backend, we accounted for this [difference](https://github.com/grafana/grafana-zabbix/blob/592380851cd5c10752d331176f6b5e9b98063f88/src/datasource/zabbix/connectors/zabbix_api/zabbixAPIConnector.ts#L940C5-L944C6). However, in the frontend, we always populated the dropdown using `proxy.host` regardless of the version customers were using. So for customers that had proxies in their zabbix set up AND were using a zabbix version `>-7.0.0`, the query editor would crash because we ended up with a list of undefined options. This PR changes it so that when `host` is not present, it uses `name` or otherwise defaults to `''` to ensure that we never have and array of options with undefined values. --- .../QueryEditor/ProblemsQueryEditor.test.tsx | 132 ++++++++++++++++++ .../QueryEditor/ProblemsQueryEditor.tsx | 16 +-- 2 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 src/datasource/components/QueryEditor/ProblemsQueryEditor.test.tsx diff --git a/src/datasource/components/QueryEditor/ProblemsQueryEditor.test.tsx b/src/datasource/components/QueryEditor/ProblemsQueryEditor.test.tsx new file mode 100644 index 0000000..8a0a09b --- /dev/null +++ b/src/datasource/components/QueryEditor/ProblemsQueryEditor.test.tsx @@ -0,0 +1,132 @@ +import React from 'react'; +import { render, waitFor } from '@testing-library/react'; +import { ProblemsQueryEditor } from './ProblemsQueryEditor'; +import { ShowProblemTypes, ZabbixTagEvalType } from '../../types/query'; + +const metricPickerSpy = jest.fn(); + +jest.mock('../../../components', () => ({ + MetricPicker: (props: any) => { + metricPickerSpy(props); + return null; + }, +})); + +jest.mock('@grafana/runtime', () => ({ + getTemplateSrv: jest.fn(() => ({ + getVariables: jest.fn(() => []), + })), +})); + +jest.mock('@grafana/ui', () => ({ + Combobox: (props: any) =>
, + InlineField: ({ children }: any) =>
{children}
, + InlineFieldRow: ({ children }: any) =>
{children}
, + InlineFormLabel: ({ children }: any) =>
{children}
, + Input: (props: any) => , + MultiSelect: (props: any) =>
, +})); + +const baseQuery: any = { + group: { filter: '' }, + host: { filter: '' }, + proxy: { filter: '' }, + application: { filter: '' }, + trigger: { filter: '' }, + tags: { filter: '' }, + evaltype: ZabbixTagEvalType.AndOr, + showProblems: ShowProblemTypes.Problems, + options: { severities: [] }, +}; + +const buildDatasource = (overrides: Partial = {}) => { + const zabbix = { + getAllGroups: jest.fn().mockResolvedValue([]), + getAllHosts: jest.fn().mockResolvedValue([]), + getAllApps: jest.fn().mockResolvedValue([]), + getProxies: jest.fn().mockResolvedValue([]), + supportsApplications: jest.fn(() => true), + ...overrides, + }; + + return { + zabbix, + interpolateVariablesInQueries: jest.fn((queries: any[]) => queries), + }; +}; + +describe('ProblemsQueryEditor', () => { + beforeEach(() => { + metricPickerSpy.mockClear(); + }); + + it('uses proxy name when host is missing', async () => { + const datasource = buildDatasource({ + getProxies: jest.fn().mockResolvedValue([{ name: 'proxy-a' }]), + }); + + render(); + + await waitFor(() => { + const proxyCall = metricPickerSpy.mock.calls + .map((call) => call[0]) + .find((props) => props?.placeholder === 'Proxy name' && props?.options); + + expect(proxyCall).toBeTruthy(); + expect(proxyCall.options).toEqual([{ value: 'proxy-a', label: 'proxy-a' }]); + }); + }); + + it('uses proxy host when present', async () => { + const datasource = buildDatasource({ + getProxies: jest.fn().mockResolvedValue([{ host: 'legacy-proxy' }]), + }); + + render(); + + await waitFor(() => { + const proxyCall = metricPickerSpy.mock.calls + .map((call) => call[0]) + .find((props) => props?.placeholder === 'Proxy name' && props?.options); + + expect(proxyCall).toBeTruthy(); + expect(proxyCall.options).toEqual([{ value: 'legacy-proxy', label: 'legacy-proxy' }]); + }); + }); + + it('defaults missing option values to empty strings', async () => { + const datasource = buildDatasource({ + getAllGroups: jest.fn().mockResolvedValue([{ name: 'group-a' }, { name: '' }, {}]), + getAllHosts: jest.fn().mockResolvedValue([{ name: 'host-a' }, { name: '' }, {}]), + getAllApps: jest.fn().mockResolvedValue([{ name: 'app-a' }, { name: '' }, {}]), + getProxies: jest.fn().mockResolvedValue([{ name: '' }, { host: '' }, { name: 'proxy-a' }]), + }); + + render(); + + await waitFor(() => { + const groupCall = metricPickerSpy.mock.calls + .map((call) => call[0]) + .find((props) => props?.placeholder === 'Group name' && props?.options); + const hostCall = metricPickerSpy.mock.calls + .map((call) => call[0]) + .find((props) => props?.placeholder === 'Host name' && props?.options); + const appCall = metricPickerSpy.mock.calls + .map((call) => call[0]) + .find((props) => props?.placeholder === 'Application name' && props?.options); + const proxyCall = metricPickerSpy.mock.calls + .map((call) => call[0]) + .find((props) => props?.placeholder === 'Proxy name' && props?.options); + + const hasValidValues = (options: any[]) => + options.every( + (option) => option.value !== undefined && (option.label !== undefined || option.value === '/.*/') + ); + + expect(hasValidValues(groupCall.options)).toBe(true); + expect(hasValidValues(hostCall.options)).toBe(true); + expect(hasValidValues(appCall.options)).toBe(true); + expect(hasValidValues(proxyCall.options)).toBe(true); + }); + }); +}); diff --git a/src/datasource/components/QueryEditor/ProblemsQueryEditor.tsx b/src/datasource/components/QueryEditor/ProblemsQueryEditor.tsx index 916cfa4..8b9d2e6 100644 --- a/src/datasource/components/QueryEditor/ProblemsQueryEditor.tsx +++ b/src/datasource/components/QueryEditor/ProblemsQueryEditor.tsx @@ -43,8 +43,8 @@ export const ProblemsQueryEditor = ({ query, datasource, onChange }: Props) => { const loadGroupOptions = async () => { const groups = await datasource.zabbix.getAllGroups(); const options = groups?.map((group) => ({ - value: group.name, - label: group.name, + value: group.name ?? '', + label: group.name ?? '', })); options.unshift(...getVariableOptions()); return options; @@ -58,8 +58,8 @@ export const ProblemsQueryEditor = ({ query, datasource, onChange }: Props) => { const loadHostOptions = async (group: string) => { const hosts = await datasource.zabbix.getAllHosts(group); let options: Array> = hosts?.map((host) => ({ - value: host.name, - label: host.name, + value: host.name ?? '', + label: host.name ?? '', })); options = _.uniqBy(options, (o) => o.value); options.unshift({ value: '/.*/' }); @@ -75,8 +75,8 @@ export const ProblemsQueryEditor = ({ query, datasource, onChange }: Props) => { const loadAppOptions = async (group: string, host: string) => { const apps = await datasource.zabbix.getAllApps(group, host); let options: Array> = apps?.map((app) => ({ - value: app.name, - label: app.name, + value: app.name ?? '', + label: app.name ?? '', })); options = _.uniqBy(options, (o) => o.value); options.unshift(...getVariableOptions()); @@ -91,8 +91,8 @@ export const ProblemsQueryEditor = ({ query, datasource, onChange }: Props) => { const loadProxyOptions = async () => { const proxies = await datasource.zabbix.getProxies(); const options = proxies?.map((proxy) => ({ - value: proxy.host, - label: proxy.host, + value: (!!proxy.host ? proxy.host : proxy.name) ?? '', + label: (!!proxy.host ? proxy.host : proxy.name) ?? '', })); options.unshift(...getVariableOptions()); return options;