fix(ui) Followups to recent changes to UI ingestion forms (#5602)

datahub-web-react/src/app/ingest/source/builder/DefineRecipeStep.tsx

@@ -7,7 +7,7 @@ import { YamlEditor } from './YamlEditor';
 import { ANTD_GRAY } from '../../../entity/shared/constants';
 import { IngestionSourceBuilderStep } from './steps';
 import RecipeBuilder from './RecipeBuilder';
-import { CONNECTORS_WITH_FORM } from './RecipeForm/utils';
+import { CONNECTORS_WITH_FORM } from './RecipeForm/constants';
 import { getRecipeJson } from './RecipeForm/TestConnection/TestConnectionButton';
 
 const LOOKML_DOC_LINK = 'https://datahubproject.io/docs/generated/ingestion/sources/looker#module-lookml';

datahub-web-react/src/app/ingest/source/builder/RecipeForm/FormField.tsx

@@ -5,7 +5,7 @@ import { MinusCircleOutlined, PlusOutlined, QuestionCircleOutlined } from '@ant-
 import { ANTD_GRAY } from '../../../../entity/shared/constants';
 import { RecipeField, FieldType } from './common';
 import { Secret } from '../../../../../types.generated';
-import SecretField from './SecretField/SecretField';
+import SecretField, { StyledFormItem } from './SecretField/SecretField';
 
 const Label = styled.div`
     font-weight: bold;
@@ -27,24 +27,6 @@ const StyledRemoveIcon = styled(MinusCircleOutlined)`
     margin-left: 10px;
 `;
 
-const StyledFormItem = styled(Form.Item)<{ alignLeft?: boolean; removeMargin: boolean }>`
-    margin-bottom: ${(props) => (props.removeMargin ? '0' : '16px')};
-
-    ${(props) =>
-        props.alignLeft &&
-        `
-        .ant-form-item {
-            flex-direction: row;
-
-        }
-
-        .ant-form-item-label {
-            padding: 0;
-            margin-right: 10px;
-        }
-    `}
-`;
-
 const ListWrapper = styled.div<{ removeMargin: boolean }>`
     margin-bottom: ${(props) => (props.removeMargin ? '0' : '16px')};
 `;
@@ -116,7 +98,9 @@ function FormField(props: Props) {
     if (field.type === FieldType.SELECT) return <SelectField field={field} removeMargin={removeMargin} />;
 
     if (field.type === FieldType.SECRET)
-        return <SecretField field={field} secrets={secrets} refetchSecrets={refetchSecrets} />;
+        return (
+            <SecretField field={field} secrets={secrets} removeMargin={removeMargin} refetchSecrets={refetchSecrets} />
+        );
 
     const isBoolean = field.type === FieldType.BOOLEAN;
     const input = isBoolean ? <Checkbox /> : <Input placeholder={field.placeholder} />;

datahub-web-react/src/app/ingest/source/builder/RecipeForm/RecipeForm.tsx

@@ -5,7 +5,7 @@ import YAML from 'yamljs';
 import { ApiOutlined, FilterOutlined, QuestionCircleOutlined, SettingOutlined } from '@ant-design/icons';
 import styled from 'styled-components/macro';
 import { jsonToYaml } from '../../utils';
-import { RECIPE_FIELDS } from './utils';
+import { RECIPE_FIELDS } from './constants';
 import FormField from './FormField';
 import TestConnectionButton from './TestConnection/TestConnectionButton';
 import { SNOWFLAKE } from '../../conf/snowflake/snowflake';

datahub-web-react/src/app/ingest/source/builder/RecipeForm/SecretField/SecretField.tsx

@@ -9,15 +9,34 @@ const StyledDivider = styled(Divider)`
     margin: 0;
 `;
 
+export const StyledFormItem = styled(Form.Item)<{ alignLeft?: boolean; removeMargin: boolean }>`
+    margin-bottom: ${(props) => (props.removeMargin ? '0' : '16px')};
+
+    ${(props) =>
+        props.alignLeft &&
+        `
+        .ant-form-item {
+            flex-direction: row;
+
+        }
+
+        .ant-form-item-label {
+            padding: 0;
+            margin-right: 10px;
+        }
+    `}
+`;
+
 interface SecretFieldProps {
     field: RecipeField;
     secrets: Secret[];
+    removeMargin?: boolean;
     refetchSecrets: () => void;
 }
 
-function SecretField({ field, secrets, refetchSecrets }: SecretFieldProps) {
+function SecretField({ field, secrets, removeMargin, refetchSecrets }: SecretFieldProps) {
     return (
-        <Form.Item name={field.name} label={field.label} tooltip={field.tooltip}>
+        <StyledFormItem name={field.name} label={field.label} tooltip={field.tooltip} removeMargin={!!removeMargin}>
             <Select
                 showSearch
                 filterOption={(input, option) => !!option?.children.toLowerCase().includes(input.toLowerCase())}
@@ -35,7 +54,7 @@ function SecretField({ field, secrets, refetchSecrets }: SecretFieldProps) {
                     </Select.Option>
                 ))}
             </Select>
-        </Form.Item>
+        </StyledFormItem>
     );
 }
 

datahub-web-react/src/app/ingest/source/builder/RecipeForm/__tests__/utils.test.ts → datahub-web-react/src/app/ingest/source/builder/RecipeForm/__tests__/common.test.ts

@@ -3,6 +3,7 @@ import { setFieldValueOnRecipe, setListValuesOnRecipe } from '../common';
 describe('setFieldValueOnRecipe', () => {
     const accountIdFieldPath = 'source.config.account_id';
     const profilingEnabledFieldPath = 'source.config.profiling.enabled';
+    const dottedFieldPath = ['source', 'config', 'profiling', 'enabled.test'];
 
     it('should set the field value on a recipe object when it was not defined', () => {
         const recipe = { source: { config: {} } };
@@ -63,6 +64,28 @@ describe('setFieldValueOnRecipe', () => {
         const updatedRecipe = setFieldValueOnRecipe(recipe, null, 'source.config.profiling.testing');
         expect(updatedRecipe).toMatchObject({ source: { config: { test: 'test', profiling: { enabled: true } } } });
     });
+
+    it('should set the field when the key is a dotted value', () => {
+        const recipe = { source: { config: { test: 'test', profiling: { 'enabled.test': true } } } };
+        const updatedRecipe = setFieldValueOnRecipe(recipe, false, dottedFieldPath);
+        expect(updatedRecipe).toMatchObject({
+            source: { config: { test: 'test', profiling: { 'enabled.test': false } } },
+        });
+    });
+
+    it('should clear the dotted field and its parent when passing in null and field is only child of parent', () => {
+        const recipe = { source: { config: { test: 'test', profiling: { 'enabled.test': true } } } };
+        const updatedRecipe = setFieldValueOnRecipe(recipe, null, dottedFieldPath);
+        expect(updatedRecipe).toMatchObject({
+            source: { config: { test: 'test' } },
+        });
+    });
+
+    it('should clear the dotted field but not its parent when passing in null and parent has other children', () => {
+        const recipe = { source: { config: { test: 'test', profiling: { 'enabled.test': true, testing: 'this' } } } };
+        const updatedRecipe = setFieldValueOnRecipe(recipe, null, dottedFieldPath);
+        expect(updatedRecipe).toMatchObject({ source: { config: { test: 'test', profiling: { testing: 'this' } } } });
+    });
 });
 
 describe('setListValuesOnRecipe', () => {

datahub-web-react/src/app/ingest/source/builder/RecipeForm/common.tsx

@@ -29,10 +29,14 @@ export interface RecipeField {
     placeholder?: string;
 }
 
-function clearFieldAndParents(recipe: any, fieldPath: string) {
+function clearFieldAndParents(recipe: any, fieldPath: string | string[]) {
     set(recipe, fieldPath, undefined);
 
-    const updatedFieldPath = fieldPath.split('.').slice(0, -1).join('.'); // remove last item from fieldPath
+    // remove last item from fieldPath
+    const updatedFieldPath = Array.isArray(fieldPath)
+        ? fieldPath.slice(0, -1).join('.')
+        : fieldPath.split('.').slice(0, -1).join('.');
+
     if (updatedFieldPath) {
         const parentKeys = Object.keys(get(recipe, updatedFieldPath));
 
@@ -43,8 +47,7 @@ function clearFieldAndParents(recipe: any, fieldPath: string) {
     }
     return recipe;
 }
-
-export function setFieldValueOnRecipe(recipe: any, value: any, fieldPath: string) {
+export function setFieldValueOnRecipe(recipe: any, value: any, fieldPath: string | string[]) {
     const updatedRecipe = { ...recipe };
     if (value !== undefined) {
         if (value === null) {
@@ -67,19 +70,6 @@ export function setListValuesOnRecipe(recipe: any, values: string[] | undefined,
     return updatedRecipe;
 }
 
-export function setDottedFieldValuesOnRecipe(recipe: any, value: any, fieldPath: string[]) {
-    const updatedRecipe = { ...recipe };
-    if (value !== undefined) {
-        if (value === null) {
-            set(recipe, fieldPath, undefined);
-            clearFieldAndParents(updatedRecipe, fieldPath.slice(0, -1).join('.'));
-            return updatedRecipe;
-        }
-        set(updatedRecipe, fieldPath, value);
-    }
-    return updatedRecipe;
-}
-
 /* ---------------------------------------------------- Filter Section ---------------------------------------------------- */
 const databaseAllowFieldPath = 'source.config.database_pattern.allow';
 export const DATABASE_ALLOW: RecipeField = {
@@ -254,7 +244,7 @@ export const PROFILING_ENABLED: RecipeField = {
 export const STATEFUL_INGESTION_ENABLED: RecipeField = {
     name: 'stateful_ingestion.enabled',
     label: 'Enable Stateful Ingestion',
-    tooltip: 'Enable the type of the ingestion state provider registered with datahub.',
+    tooltip: 'Remove stale datasets from datahub once they have been deleted in the source.',
     type: FieldType.BOOLEAN,
     fieldPath: 'source.config.stateful_ingestion.enabled',
     rules: null,
@@ -263,7 +253,7 @@ export const STATEFUL_INGESTION_ENABLED: RecipeField = {
 export const UPSTREAM_LINEAGE_IN_REPORT: RecipeField = {
     name: 'upstream_lineage_in_report',
     label: 'Include Upstream Lineage In Report.',
-    tooltip: 'Remove stale datasets from datahub once they have been deleted in the source.',
+    tooltip: 'Useful for debugging lineage information. Set to True to see the raw lineage created internally.',
     type: FieldType.BOOLEAN,
     fieldPath: 'source.config.upstream_lineage_in_report',
     rules: null,

datahub-web-react/src/app/ingest/source/builder/RecipeForm/utils.tsx → datahub-web-react/src/app/ingest/source/builder/RecipeForm/constants.ts

@@ -20,6 +20,7 @@ import {
     EXTRACT_USAGE_HISTORY,
     EXTRACT_OWNERS,
     SKIP_PERSONAL_FOLDERS,
+    RecipeField,
 } from './common';
 import {
     SNOWFLAKE_ACCOUNT_ID,
@@ -74,7 +75,18 @@ import {
     TOPIC_DENY,
 } from './kafka';
 
-export const RECIPE_FIELDS = {
+interface RecipeFields {
+    [key: string]: {
+        fields: RecipeField[];
+        filterFields: RecipeField[];
+        advancedFields: RecipeField[];
+        connectionSectionTooltip?: string;
+        filterSectionTooltip?: string;
+        advancedSectionTooltip?: string;
+    };
+}
+
+export const RECIPE_FIELDS: RecipeFields = {
     [SNOWFLAKE]: {
         fields: [SNOWFLAKE_ACCOUNT_ID, SNOWFLAKE_WAREHOUSE, SNOWFLAKE_USERNAME, SNOWFLAKE_PASSWORD, SNOWFLAKE_ROLE],
         advancedFields: [INCLUDE_LINEAGE, PROFILING_ENABLED, STATEFUL_INGESTION_ENABLED],

datahub-web-react/src/app/ingest/source/builder/RecipeForm/kafka.ts

@@ -1,4 +1,4 @@
-import { RecipeField, FieldType, setDottedFieldValuesOnRecipe, setListValuesOnRecipe } from './common';
+import { RecipeField, FieldType, setListValuesOnRecipe } from './common';
 
 const saslUsernameFieldPath = ['source', 'config', 'connection', 'consumer_config', 'sasl.username'];
 export const KAFKA_SASL_USERNAME: RecipeField = {
@@ -8,8 +8,6 @@ export const KAFKA_SASL_USERNAME: RecipeField = {
     type: FieldType.TEXT,
     fieldPath: saslUsernameFieldPath,
     rules: null,
-    setValueOnRecipeOverride: (recipe: any, values: string[]) =>
-        setDottedFieldValuesOnRecipe(recipe, values, saslUsernameFieldPath),
 };
 
 const saslPasswordFieldPath = ['source', 'config', 'connection', 'consumer_config', 'sasl.password'];
@@ -20,8 +18,6 @@ export const KAFKA_SASL_PASSWORD: RecipeField = {
     type: FieldType.TEXT,
     fieldPath: saslPasswordFieldPath,
     rules: null,
-    setValueOnRecipeOverride: (recipe: any, values: string[]) =>
-        setDottedFieldValuesOnRecipe(recipe, values, saslPasswordFieldPath),
 };
 
 export const KAFKA_BOOTSTRAP: RecipeField = {
@@ -61,8 +57,6 @@ export const KAFKA_SCHEMA_REGISTRY_USER_CREDENTIAL: RecipeField = {
     type: FieldType.TEXT,
     fieldPath: registryCredentialsFieldPath,
     rules: null,
-    setValueOnRecipeOverride: (recipe: any, values: string[]) =>
-        setDottedFieldValuesOnRecipe(recipe, values, registryCredentialsFieldPath),
 };
 
 const securityProtocolFieldPath = ['source', 'config', 'connection', 'consumer_config', 'security.protocol'];
@@ -77,8 +71,6 @@ export const KAFKA_SECURITY_PROTOCOL: RecipeField = {
         { label: 'SASL_SSL', value: 'SASL_SSL' },
         { label: 'SASL_PLAINTEXT', value: 'SASL_PLAINTEXT' },
     ],
-    setValueOnRecipeOverride: (recipe: any, values: string[]) =>
-        setDottedFieldValuesOnRecipe(recipe, values, securityProtocolFieldPath),
 };
 
 const saslMechanismFieldPath = ['source', 'config', 'connection', 'consumer_config', 'sasl.mechanism'];
@@ -94,8 +86,6 @@ export const KAFKA_SASL_MECHANISM: RecipeField = {
         { label: 'SCRAM-SHA-256', value: 'SCRAM-SHA-256' },
         { label: 'SCRAM-SHA-512', value: 'SCRAM-SHA-512' },
     ],
-    setValueOnRecipeOverride: (recipe: any, values: string[]) =>
-        setDottedFieldValuesOnRecipe(recipe, values, saslMechanismFieldPath),
 };
 
 const topicAllowFieldPath = 'source.config.topic_patterns.allow';