Chore: removes props mutation (#2056)
While investigating some potential mutations [here](https://ops.grafana-ops.net/d/83f4951f-2ef3-4260-91a0-39a031992b75/getmutationobserverproxy-logs) I was able to find these mutating [lines](https://github.com/grafana/grafana-zabbix/blob/main/src/datasource/components/ConfigEditor.tsx#L55-L57) Although this works right now, this might not work in future Grafana versions. This PR makes sure we don't mutate the props. I haven't been able to test this manually so I could use some help to make sure the plugin works as expected. --------- Co-authored-by: ivanahuckova <ivana.huckova@gmail.com>
This commit is contained in:
5
.changeset/metal-bottles-sin.md
Normal file
5
.changeset/metal-bottles-sin.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
'grafana-zabbix': patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix: Remove props mutation in config editor
|
||||||
102
src/datasource/components/ConfigEditor.test.tsx
Normal file
102
src/datasource/components/ConfigEditor.test.tsx
Normal file
@@ -0,0 +1,102 @@
|
|||||||
|
import React from 'react';
|
||||||
|
import { render } from '@testing-library/react';
|
||||||
|
import { ConfigEditor, Props } from './ConfigEditor';
|
||||||
|
|
||||||
|
jest.mock('@grafana/runtime', () => ({
|
||||||
|
...jest.requireActual('@grafana/runtime'),
|
||||||
|
config: {},
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('ConfigEditor', () => {
|
||||||
|
describe('on initial render', () => {
|
||||||
|
it('should not mutate the options object', () => {
|
||||||
|
const options = Object.freeze({ ...getDefaultOptions() }); // freezing the options to prevent mutations
|
||||||
|
Object.freeze(options.jsonData);
|
||||||
|
Object.freeze(options.secureJsonData);
|
||||||
|
Object.freeze(options.secureJsonFields);
|
||||||
|
const onOptionsChangeSpy = jest.fn();
|
||||||
|
|
||||||
|
expect(() => render(<ConfigEditor options={options} onOptionsChange={onOptionsChangeSpy} />)).not.toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should call onOptionsChange with the correct values', () => {
|
||||||
|
const options = Object.freeze({ ...getDefaultOptions() }); // freezing the options to prevent mutations
|
||||||
|
Object.freeze(options.jsonData);
|
||||||
|
Object.freeze(options.secureJsonData);
|
||||||
|
Object.freeze(options.secureJsonFields);
|
||||||
|
const onOptionsChangeSpy = jest.fn();
|
||||||
|
|
||||||
|
expect(() => render(<ConfigEditor options={options} onOptionsChange={onOptionsChangeSpy} />)).not.toThrow();
|
||||||
|
expect(onOptionsChangeSpy).toBeCalledTimes(1);
|
||||||
|
expect(onOptionsChangeSpy).toHaveBeenCalledWith({
|
||||||
|
...getDefaultOptions(),
|
||||||
|
jsonData: {
|
||||||
|
...getDefaultOptions().jsonData,
|
||||||
|
authType: 'userLogin',
|
||||||
|
timeout: undefined,
|
||||||
|
password: undefined, // password should be missing from jsonData
|
||||||
|
},
|
||||||
|
secureJsonData: {
|
||||||
|
...getDefaultOptions().secureJsonData,
|
||||||
|
password: 'a password', // password should be present in secureJsonData
|
||||||
|
},
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not update password in secureJsonData if the field already exists in secureJsonFields', () => {
|
||||||
|
const options = Object.freeze({ ...getDefaultOptions(), secureJsonFields: { password: true } }); // freezing the options to prevent mutations
|
||||||
|
Object.freeze(options.jsonData);
|
||||||
|
Object.freeze(options.secureJsonData);
|
||||||
|
Object.freeze(options.secureJsonFields);
|
||||||
|
const onOptionsChangeSpy = jest.fn();
|
||||||
|
|
||||||
|
expect(() => render(<ConfigEditor options={options} onOptionsChange={onOptionsChangeSpy} />)).not.toThrow();
|
||||||
|
expect(onOptionsChangeSpy).toBeCalledTimes(1);
|
||||||
|
expect(onOptionsChangeSpy).toHaveBeenCalledWith({
|
||||||
|
...getDefaultOptions(),
|
||||||
|
jsonData: {
|
||||||
|
...getDefaultOptions().jsonData,
|
||||||
|
authType: 'userLogin',
|
||||||
|
timeout: undefined,
|
||||||
|
password: undefined, // password should be missing from jsonData
|
||||||
|
},
|
||||||
|
secureJsonData: {}, // password should be missing from secureJsonData
|
||||||
|
secureJsonFields: { ...getDefaultOptions().secureJsonFields, password: true },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
function getDefaultOptions(): Props['options'] {
|
||||||
|
return {
|
||||||
|
id: 1,
|
||||||
|
orgId: 1,
|
||||||
|
uid: '',
|
||||||
|
name: '',
|
||||||
|
typeLogoUrl: '',
|
||||||
|
type: '',
|
||||||
|
typeName: '',
|
||||||
|
access: '',
|
||||||
|
url: '',
|
||||||
|
user: '',
|
||||||
|
database: '',
|
||||||
|
basicAuth: false,
|
||||||
|
basicAuthUser: '',
|
||||||
|
isDefault: false,
|
||||||
|
jsonData: {
|
||||||
|
cacheTTL: '',
|
||||||
|
dbConnectionEnable: false,
|
||||||
|
disableDataAlignment: false,
|
||||||
|
disableReadOnlyUsersAck: false,
|
||||||
|
trends: false,
|
||||||
|
trendsFrom: '',
|
||||||
|
trendsRange: '',
|
||||||
|
username: '',
|
||||||
|
password: 'a password',
|
||||||
|
},
|
||||||
|
readOnly: false,
|
||||||
|
secureJsonData: {},
|
||||||
|
secureJsonFields: {},
|
||||||
|
withCredentials: false,
|
||||||
|
};
|
||||||
|
}
|
||||||
@@ -50,11 +50,11 @@ export const ConfigEditor = (props: Props) => {
|
|||||||
|
|
||||||
// Set secureJsonFields.password to password and then remove it from config
|
// Set secureJsonFields.password to password and then remove it from config
|
||||||
const { password, ...restJsonData } = jsonData;
|
const { password, ...restJsonData } = jsonData;
|
||||||
|
|
||||||
|
// Create new secureJsonData object
|
||||||
|
const newSecureJsonData = { ...options.secureJsonData };
|
||||||
if (!secureJsonFields?.password) {
|
if (!secureJsonFields?.password) {
|
||||||
if (!options.secureJsonData) {
|
newSecureJsonData.password = password;
|
||||||
options.secureJsonData = {};
|
|
||||||
}
|
|
||||||
options.secureJsonData.password = password;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
onOptionsChange({
|
onOptionsChange({
|
||||||
@@ -69,6 +69,7 @@ export const ConfigEditor = (props: Props) => {
|
|||||||
disableDataAlignment: false,
|
disableDataAlignment: false,
|
||||||
...restJsonData,
|
...restJsonData,
|
||||||
},
|
},
|
||||||
|
secureJsonData: { ...newSecureJsonData },
|
||||||
});
|
});
|
||||||
|
|
||||||
if (options.jsonData.dbConnectionEnable) {
|
if (options.jsonData.dbConnectionEnable) {
|
||||||
|
|||||||
@@ -1,4 +1,7 @@
|
|||||||
import { PanelCtrl, MetricsPanelCtrl } from './panelStub';
|
import { PanelCtrl, MetricsPanelCtrl } from './panelStub';
|
||||||
|
import { TextEncoder, TextDecoder } from 'util';
|
||||||
|
|
||||||
|
Object.assign(global, { TextDecoder, TextEncoder });
|
||||||
|
|
||||||
jest.mock(
|
jest.mock(
|
||||||
'grafana/app/features/templating/template_srv',
|
'grafana/app/features/templating/template_srv',
|
||||||
|
|||||||
Reference in New Issue
Block a user