fix: make default values consistent for all check types

grafana · Sep 24, 2020 · 56866bb · 56866bb
1 parent c6d302e
commit 56866bb
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 111 deletions.
diff --git a/src/components/CheckEditor/CheckEditor.test.tsx b/src/components/CheckEditor/CheckEditor.test.tsx
@@ -136,6 +136,7 @@ describe('PING', () => {
 
   it('correctly populates default values', async () => {
     const check = {
+      id: 32,
       job: 'carne asada',
       target: 'target.com',
       enabled: true,
@@ -182,9 +183,10 @@ describe('HTTP', () => {
     expect(advanced).toBeInTheDocument();
   });
 
-  it('correctly populates default values', async () => {
+  it('correctly populates default values for preexisting check', async () => {
     const check = {
       job: 'carne asada',
+      id: 32,
       target: 'https://target.com',
       enabled: true,
       labels: [{ name: 'a great label', value: 'totally awesome label' }],
@@ -262,6 +264,7 @@ describe('HTTP', () => {
     expect(await within(advancedOptions).findByPlaceholderText('name')).toHaveValue('a great label');
     expect(await within(advancedOptions).findByPlaceholderText('value')).toHaveValue('totally awesome label');
     expect(await within(advancedOptions).findByText('V6')).toBeInTheDocument();
+    // Follow redirect field
     expect(await within(advancedOptions).findByRole('checkbox')).not.toBeChecked();
     expect(
       await within(advancedOptions).findByLabelText('Cache busting query parameter name', { exact: false })

diff --git a/src/components/CheckEditor/CheckEditor.tsx b/src/components/CheckEditor/CheckEditor.tsx
@@ -27,7 +27,7 @@ import { CHECK_TYPE_OPTIONS } from 'components/constants';
 import { useForm, FormContext, Controller } from 'react-hook-form';
 
 interface Props {
-  check: Check;
+  check?: Check;
   instance: SMDataSource;
   onReturn: (reload: boolean) => void;
 }
@@ -44,7 +44,7 @@ export const CheckEditor: FC<Props> = ({ check, instance, onReturn }) => {
 
   const { execute: onSubmit, error, loading: submitting } = useAsyncCallback(async (values: CheckFormValues) => {
     const updatedCheck = getCheckFromFormValues(values, defaultValues);
-    if (check.id) {
+    if (check?.id) {
       await instance.updateCheck({
         id: check.id,
         tenantId: check.tenantId,
@@ -59,7 +59,7 @@ export const CheckEditor: FC<Props> = ({ check, instance, onReturn }) => {
   const submissionError = error as SubmissionError;
 
   const onRemoveCheck = async () => {
-    const id = check.id;
+    const id = check?.id;
     if (!id) {
       return;
     }

diff --git a/src/components/CheckEditor/checkFormTransformations.ts b/src/components/CheckEditor/checkFormTransformations.ts
@@ -25,6 +25,7 @@ import {
   HttpSslOption,
   HttpRegexValidationType,
   HeaderMatch,
+  DnsResponseCodes,
 } from 'types';
 
 import {
@@ -36,6 +37,22 @@ import {
 } from 'components/constants';
 import { checkType } from 'utils';
 
+const fallbackCheck = {
+  job: '',
+  target: '',
+  frequency: 60000,
+  timeout: 3000,
+  enabled: true,
+  labels: [],
+  probes: [],
+  settings: {
+    ping: {
+      ipVersion: IpVersion.V4,
+      dontFragment: false,
+    },
+  },
+} as Check;
+
 export function selectableValueFrom<T>(value: T, label?: string): SelectableValue<T> {
   const labelValue = String(value);
   return { label: label ?? labelValue, value };
@@ -68,6 +85,7 @@ export function fallbackSettings(t: CheckType): Settings {
           ipVersion: IpVersion.V4,
           protocol: DnsProtocol.UDP,
           port: 53,
+          validRCodes: [DnsResponseCodes.NOERROR],
         },
       };
     }
@@ -248,16 +266,27 @@ const getFormSettingsForCheck = (settings: Settings): SettingsFormValues => {
   }
 };
 
-export const getDefaultValuesFromCheck = (check: Check): CheckFormValues => {
+const getAllFormSettingsForCheck = (): SettingsFormValues => {
+  return {
+    http: getHttpSettingsFormValues(fallbackSettings(CheckType.HTTP)),
+    tcp: getTcpSettingsFormValues(fallbackSettings(CheckType.TCP)),
+    dns: getDnsSettingsFormValues(fallbackSettings(CheckType.DNS)),
+    ping: getPingSettingsFormValues(fallbackSettings(CheckType.PING)),
+  };
+};
+
+export const getDefaultValuesFromCheck = (check: Check = fallbackCheck): CheckFormValues => {
   const defaultCheckType = checkType(check.settings);
+  const settings = check.id ? getFormSettingsForCheck(check.settings) : getAllFormSettingsForCheck();
+
   return {
     ...check,
     timeout: check.timeout / 1000,
     frequency: check.frequency / 1000,
     probes: check.probes,
     checkType:
       CHECK_TYPE_OPTIONS.find(checkTypeOption => checkTypeOption.value === defaultCheckType) ?? CHECK_TYPE_OPTIONS[1],
-    settings: getFormSettingsForCheck(check.settings),
+    settings,
   };
 };
 

diff --git a/src/components/DnsSettings.test.tsx b/src/components/DnsSettings.test.tsx
diff --git a/src/components/DnsSettings.tsx b/src/components/DnsSettings.tsx
@@ -14,11 +14,10 @@ import {
   useTheme,
 } from '@grafana/ui';
 import { useFormContext, Controller, useFieldArray } from 'react-hook-form';
-import { CheckType, ResponseMatchType } from 'types';
+import { ResponseMatchType } from 'types';
 import { Collapse } from 'components/Collapse';
 import { LabelField } from './LabelField';
 import { DNS_RESPONSE_CODES, DNS_RECORD_TYPES, DNS_PROTOCOLS, IP_OPTIONS } from './constants';
-import { fallbackSettings } from './CheckEditor/checkFormTransformations';
 
 interface Props {
   isEditor: boolean;
@@ -30,8 +29,6 @@ const RESPONSE_MATCH_OPTIONS = [
   { label: `Validate ${ResponseMatchType.Additional} matches`, value: ResponseMatchType.Additional },
 ];
 
-const fallbackValues = fallbackSettings(CheckType.DNS);
-
 const DnsSettingsForm: FC<Props> = ({ isEditor }) => {
   const { spacing } = useTheme();
 
@@ -58,12 +55,7 @@ const DnsSettingsForm: FC<Props> = ({ isEditor }) => {
           `}
         >
           <Field label="Record type" disabled={!isEditor}>
-            <Controller
-              as={Select}
-              name="settings.dns.recordType"
-              options={DNS_RECORD_TYPES}
-              defaultValue={fallbackValues.dns?.recordType}
-            />
+            <Controller as={Select} name="settings.dns.recordType" options={DNS_RECORD_TYPES} />
           </Field>
           <Field label="Server" disabled={!isEditor}>
             <Input
@@ -72,26 +64,13 @@ const DnsSettingsForm: FC<Props> = ({ isEditor }) => {
               name="settings.dns.server"
               type="text"
               placeholder="server"
-              defaultValue={fallbackValues.dns?.server}
             />
           </Field>
           <Field label="Protocol" disabled={!isEditor}>
-            <Controller
-              as={Select}
-              name="settings.dns.protocol"
-              options={DNS_PROTOCOLS}
-              defaultValue={fallbackValues.dns?.protocol}
-            />
+            <Controller as={Select} name="settings.dns.protocol" options={DNS_PROTOCOLS} />
           </Field>
           <Field label="Port" disabled={!isEditor}>
-            <Input
-              id="dns-settings-port"
-              ref={register}
-              name="settings.dns.port"
-              type="number"
-              placeholder="port"
-              defaultValue={fallbackValues.dns?.port}
-            />
+            <Input id="dns-settings-port" ref={register} name="settings.dns.port" type="number" placeholder="port" />
           </Field>
         </div>
       </Collapse>
@@ -178,7 +157,7 @@ const DnsSettingsForm: FC<Props> = ({ isEditor }) => {
         <LabelField isEditor={isEditor} />
         <HorizontalGroup>
           <Field label="IP version" description="The IP protocol of the ICMP request" disabled={!isEditor}>
-            <Controller name="settings.dns.ipVersion" as={Select} options={IP_OPTIONS} defaultValue={IP_OPTIONS[1]} />
+            <Controller name="settings.dns.ipVersion" as={Select} options={IP_OPTIONS} />
           </Field>
         </HorizontalGroup>
       </Collapse>

diff --git a/src/components/http/HttpSettings.tsx b/src/components/http/HttpSettings.tsx
@@ -201,13 +201,7 @@ export const HttpSettingsForm: FC<Props> = ({ isEditor }) => {
             invalid={Boolean(errors?.settings?.http?.method)}
             error={errors?.settings?.http?.method}
           >
-            <Controller
-              as={Select}
-              rules={{ required: true }}
-              name="settings.http.method"
-              options={methodOptions}
-              defaultValue={methodOptions[0]}
-            />
+            <Controller as={Select} rules={{ required: true }} name="settings.http.method" options={methodOptions} />
           </Field>
         </HorizontalGroup>
         <Container>

diff --git a/src/page/ChecksPage.tsx b/src/page/ChecksPage.tsx
@@ -2,7 +2,7 @@
 import React, { PureComponent } from 'react';
 
 // Types
-import { Check, GrafanaInstances, IpVersion } from 'types';
+import { Check, GrafanaInstances } from 'types';
 import { getLocationSrv } from '@grafana/runtime';
 import { CheckEditor } from 'components/CheckEditor';
 import { CheckList } from 'components/CheckList';
@@ -91,22 +91,7 @@ export class ChecksPage extends PureComponent<Props, State> {
       return <CheckEditor check={check} instance={instance.api} onReturn={this.onGoBack} />;
     }
     if (addNew) {
-      const template = {
-        job: '',
-        target: '',
-        frequency: 60000,
-        timeout: 3000,
-        enabled: true,
-        labels: [],
-        probes: [],
-        settings: {
-          ping: {
-            ipVersion: IpVersion.V4,
-            dontFragment: false,
-          },
-        },
-      } as Check;
-      return <CheckEditor check={template} instance={instance.api} onReturn={this.onGoBack} />;
+      return <CheckEditor instance={instance.api} onReturn={this.onGoBack} />;
     }
     return <CheckList instance={instance} onAddNewClick={this.onAddNew} checks={checks} />;
   }